Ocfs2 devel - Dec 2017 - [PATCH] ocfs2: fall back to buffer IO when append dio is disabled with file hole existing

On Tue, 19 Dec 2017 09:24:17 +0800 Joseph Qi <jiangqi903 at gmail.com>
wrote:
> > ---
a/fs/ocfs2/aops.c~ocfs2-fall-back-to-buffer-io-when-append-dio-is-disabled-with-file-hole-existing-fix
> > +++ a/fs/ocfs2/aops.c
> > @@ -2469,10 +2469,9 @@ static int ocfs2_dio_end_io(struct kiocb
> >   * Will look for holes and unwritten extents in the range starting at
> >   * pos for count bytes (inclusive).
> >   */
> > -static int ocfs2_check_range_for_holes(struct inode *inode, loff_t
pos,
> > -				       size_t count)
> > +static bool ocfs2_range_has_holes(struct inode *inode, loff_t pos,
size_t count)
> >  {
> > -	int ret = 0;
> > +	bool ret = false;
> 
> I have a different opinion here. If we change return value from int to
> bool, the error returned by ocfs2_get_clusters cannot be reflected. So
> I'd prefer the original version.
But that error code is not used?

On 17/12/19 09:27, Andrew Morton wrote:
> On Tue, 19 Dec 2017 09:24:17 +0800 Joseph Qi <jiangqi903 at
gmail.com> wrote:
> 
>>> ---
a/fs/ocfs2/aops.c~ocfs2-fall-back-to-buffer-io-when-append-dio-is-disabled-with-file-hole-existing-fix
>>> +++ a/fs/ocfs2/aops.c
>>> @@ -2469,10 +2469,9 @@ static int ocfs2_dio_end_io(struct kiocb
>>>   * Will look for holes and unwritten extents in the range starting
at
>>>   * pos for count bytes (inclusive).
>>>   */
>>> -static int ocfs2_check_range_for_holes(struct inode *inode, loff_t
pos,
>>> -				       size_t count)
>>> +static bool ocfs2_range_has_holes(struct inode *inode, loff_t pos,
size_t count)
>>>  {
>>> -	int ret = 0;
>>> +	bool ret = false;
>>
>> I have a different opinion here. If we change return value from int to
>> bool, the error returned by ocfs2_get_clusters cannot be reflected. So
>> I'd prefer the original version.
> 
> But that error code is not used?
> 
IMO, since ocfs2_get_clusters will get io involved, the caller shouldn't
ignore its error.

Something like:
ret = ocfs2_check_range_for_holes();
if (ret < 0) {
	mlog_errno(ret);
	goto out;
}

Then check if append dio feature has been enabled as well as write range
and hole.

Thanks,
Joseph