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@xxxxxxx>
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@xxxxxxx>
Signed-off-by: Bernd Schubert <bschubert@xxxxxxx>
---
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
