lkml.org 
[lkml]   [2022]   [May]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH v3 1/1] FUSE: Allow non-extending parallel direct writes on the same file.
From


On 5/25/22 20:41, Vivek Goyal wrote:
> On Fri, May 20, 2022 at 10:04:43AM +0530, Dharmendra Singh wrote:
>> From: Dharmendra Singh <dsingh@ddn.com>
>>
>> In general, as of now, in FUSE, direct writes on the same file are
>> serialized over inode lock i.e we hold inode lock for the full duration
>> of the write request. I could not found in fuse code a comment which
>> clearly explains why this exclusive lock is taken for direct writes.
>> Our guess is some USER space fuse implementations might be relying
>> on this lock for seralization and also it protects for the issues
>> arising due to file size assumption or write failures. This patch
>> relaxes this exclusive lock in some cases of direct writes.
>>
>> With these changes, we allows non-extending parallel direct writes
>> on the same file with the help of a flag called FOPEN_PARALLEL_WRITES.
>> If this flag is set on the file (flag is passed from libfuse to fuse
>> kernel as part of file open/create), we do not take exclusive lock instead
>> use shared lock so that all non-extending writes can run in parallel.
>>
>> Best practise would be to enable parallel direct writes of all kinds
>> including extending writes as well but we see some issues such as
>> when one write completes and other fails, how we should truncate(if
>> needed) the file if underlying file system does not support holes
>> (For file systems which supports holes, there might be a possibility
>> of enabling parallel writes for all cases).
>>
>> FUSE implementations which rely on this inode lock for serialisation
>> can continue to do so and this is default behaviour i.e no parallel
>> direct writes.
>>
>> Signed-off-by: Dharmendra Singh <dsingh@ddn.com>
>> Signed-off-by: Bernd Schubert <bschubert@ddn.com>
>> ---
>> fs/fuse/file.c | 33 ++++++++++++++++++++++++++++++---
>> include/uapi/linux/fuse.h | 2 ++
>> 2 files changed, 32 insertions(+), 3 deletions(-)
>>
>> diff --git a/fs/fuse/file.c b/fs/fuse/file.c
>> index 829094451774..1a93fd80a6ce 100644
>> --- a/fs/fuse/file.c
>> +++ b/fs/fuse/file.c
>> @@ -1541,14 +1541,37 @@ static ssize_t fuse_direct_read_iter(struct kiocb *iocb, struct iov_iter *to)
>> return res;
>> }
>>
>> +static bool fuse_direct_write_extending_i_size(struct kiocb *iocb,
>> + struct iov_iter *iter)
>> +{
>> + struct inode *inode = file_inode(iocb->ki_filp);
>> +
>> + return (iocb->ki_flags & IOCB_APPEND ||
>> + iocb->ki_pos + iov_iter_count(iter) > i_size_read(inode));
>> +}
>
> Hi Dharmendra,
>
> I have a question. What makes i_size stable. This is being read outside
> the inode_lock(). Can it race with truncate. I mean we checked
> i_size and decided to take shared lock. In the mean time another thread
> truncated the file and now our decision to take shared lock is wrong
> as file will be extended due to direct write?

Oh right, good catch! I guess we need to take a shared lock first, read
the size and if it is an extending lock need to unlock/switch to an
excluding lock. Theoretically could be a loop, but I guess that would be
overkill.


Thanks for your review!

Cheers,
Bernd

\
 
 \ /
  Last update: 2022-05-25 21:09    [W:0.201 / U:0.304 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site