Re: [PATCH 3/5] fuse: Allow parallel direct writes for O_DIRECT

[Bernd Schubert <bernd.schubert@fastmail.fm>]

On 8/28/23 12:42, Miklos Szeredi wrote:
> On Thu, 24 Aug 2023 at 17:08, Bernd Schubert <bschubert@ddn.com> wrote:
>>
>> Take a shared lock in fuse_cache_write_iter.
>>
>> Cc: Hao Xu <howeyxu@tencent.com>
>> Cc: Miklos Szeredi <miklos@szeredi.hu>
>> Cc: Dharmendra Singh <dsingh@ddn.com>
>> Cc: linux-fsdevel@vger.kernel.org
>> Signed-off-by: Bernd Schubert <bschubert@ddn.com>
>> ---
>>   fs/fuse/file.c | 21 ++++++++++++++++-----
>>   1 file changed, 16 insertions(+), 5 deletions(-)
>>
>> diff --git a/fs/fuse/file.c b/fs/fuse/file.c
>> index a16f9b6888de..905ce3bb0047 100644
>> --- a/fs/fuse/file.c
>> +++ b/fs/fuse/file.c
>> @@ -1314,9 +1314,10 @@ static bool fuse_dio_wr_exclusive_lock(struct kiocb *iocb, struct iov_iter *from
>>          struct file *file = iocb->ki_filp;
>>          struct fuse_file *ff = file->private_data;
>>
>> -       return  !(ff->open_flags & FOPEN_PARALLEL_DIRECT_WRITES) ||
>> -               iocb->ki_flags & IOCB_APPEND ||
>> -               fuse_direct_write_extending_i_size(iocb, from);
>> +       return ((!(iocb->ki_flags & IOCB_DIRECT)) ||
>> +               (!(ff->open_flags & FOPEN_PARALLEL_DIRECT_WRITES)) ||
> 
> Why the extra parenthesis around the negation in the above two conditions?
> 
> So this condition will always be true at this point when called from
> fuse_cache_write_iter()?  If so, you need to explain in the commit
> message why are you doing this at this point (e.g. future patches
> depend on this).

Oh, thanks for spotting, the double parenthesis were accidentally. 
Although I don't think it would have an effect, it just results in

return ((!(condition1)) || ...

I.e. does not change the condition itself?

Anyway, yeah, agreed on your comment in the patch before, with one 
condition per line it becomes easier to read and avoids parenthesis. I 
had just tried to keep the code as it is to make the patch easier to read.


Thanks,
Bernd