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

[Bernd Schubert <aakef@fastmail.fm>]

On 8/30/23 15:28, Miklos Szeredi wrote:
> On Tue, 29 Aug 2023 at 18:11, Bernd Schubert <bschubert@ddn.com> wrote:
>>
>> Take a shared lock in fuse_cache_write_iter. This was already
>> done for FOPEN_DIRECT_IO in
>>
>> commit 153524053bbb ("fuse: allow non-extending parallel direct
>> writes on the same file")
>>
>> but so far missing for plain O_DIRECT. Server side needs
>> to set FOPEN_PARALLEL_DIRECT_WRITES in order to signal that
>> it supports parallel dio writes.
> 
> Hmm, I think file_remove_privs() needs exclusive lock (due to calling
> notify_change()).   And the fallback case also.
> 
> Need to be careful with such locking changes...

Hrmm, yeah, I missed that :( Really sorry and thanks!

I guess I can fix it if by exporting dentry_needs_remove_privs. I hope 
that is acceptable. Interesting is that btrfs_direct_write seems to have 
the same issue.

btrfs_direct_write
	if (iocb->ki_pos + iov_iter_count(from) <= i_size_read(inode))
		ilock_flags |= BTRFS_ILOCK_SHARED;
...
	err = btrfs_inode_lock(BTRFS_I(inode), ilock_flags);
...	
	err = btrfs_write_check(iocb, from, err);
...
			ret = file_remove_privs(file);


I think that can be fixed as well after exporting 
dentry_needs_remove_privs().


Btw, why is fuse_direct_write_iter not dropping privileges? That is 
another change I need to document...


Another issue I just see is that it needs to check file size again after 
taking the lock.


Thanks a lot for your review,
Bernd