All of lore.kernel.org
 help / color / mirror / Atom feed
From: Bernd Schubert <bschubert@ddn.com>
To: linux-fsdevel@vger.kernel.org
Cc: bernd.schubert@fastmail.fm, miklos@szeredi.hu, dsingh@ddn.com,
	Bernd Schubert <bschubert@ddn.com>, Hao Xu <howeyxu@tencent.com>
Subject: [PATCH 5/6] fuse: Remove fuse_direct_write_iter code path / use IOCB_DIRECT
Date: Tue, 29 Aug 2023 18:11:15 +0200	[thread overview]
Message-ID: <20230829161116.2914040-6-bschubert@ddn.com> (raw)
In-Reply-To: <20230829161116.2914040-1-bschubert@ddn.com>

fuse_direct_write_iter is basically duplicating what is already
in fuse_cache_write_iter/generic_file_direct_write. That can be
avoided by setting IOCB_DIRECT in fuse_file_write_iter, after that
fuse_cache_write_iter can be used for the FOPEN_DIRECT_IO code path
and fuse_direct_write_iter can be removed.

Before it was using for FOPEN_DIRECT_IO

1) async (!is_sync_kiocb(iocb)) && IOCB_DIRECT

fuse_file_write_iter
    fuse_direct_write_iter
        fuse_direct_IO
            fuse_send_dio

2) sync (is_sync_kiocb(iocb)) or IOCB_DIRECT not being set

fuse_file_write_iter
    fuse_direct_write_iter
        fuse_send_dio

3) FOPEN_DIRECT_IO not set

Same as the consolidates path below

The new consolidated code path is always

fuse_file_write_iter
    fuse_cache_write_iter
        generic_file_write_iter
             __generic_file_write_iter
                 generic_file_direct_write
                     mapping->a_ops->direct_IO / fuse_direct_IO
                         fuse_send_dio

So in general for FOPEN_DIRECT_IO the code path gets longer. Additionally
fuse_direct_IO does an allocation of struct fuse_io_priv - might be a bit
slower in micro benchmarks.
Also, the IOCB_DIRECT information gets lost (as we now set it outselves).
But then IOCB_DIRECT is directly related to O_DIRECT set in
struct file::f_flags.

An additional change is for condition 2 above, which might will now do
async IO on the condition ff->fm->fc->async_dio. Given that async IO for
FOPEN_DIRECT_IO was especially introduced in commit
'commit 23c94e1cdcbf ("fuse: Switch to using async direct IO for
 FOPEN_DIRECT_IO")'
it should not matter. Especially as fuse_direct_IO is blocking for
is_sync_kiocb(), at worst it has another slight overhead.

Advantage is the removal of code paths and conditions and it is now also
possible to remove FOPEN_DIRECT_IO conditions in fuse_send_dio
(in a later patch).

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 | 54 ++++----------------------------------------------
 1 file changed, 4 insertions(+), 50 deletions(-)

diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index f9d21804d313..0b3363eec435 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -1602,52 +1602,6 @@ static ssize_t fuse_direct_read_iter(struct kiocb *iocb, struct iov_iter *to)
 	return res;
 }
 
-static ssize_t fuse_direct_write_iter(struct kiocb *iocb, struct iov_iter *from)
-{
-	struct inode *inode = file_inode(iocb->ki_filp);
-	struct fuse_io_priv io = FUSE_IO_PRIV_SYNC(iocb);
-	ssize_t res;
-	bool exclusive_lock = fuse_dio_wr_exclusive_lock(iocb, from);
-
-	/*
-	 * Take exclusive lock if
-	 * - Parallel direct writes are disabled - a user space decision
-	 * - Parallel direct writes are enabled and i_size is being extended.
-	 *   This might not be needed at all, but needs further investigation.
-	 */
-	if (exclusive_lock)
-		inode_lock(inode);
-	else {
-		inode_lock_shared(inode);
-
-		/* A race with truncate might have come up as the decision for
-		 * the lock type was done without holding the lock, check again.
-		 */
-		if (fuse_io_past_eof(iocb, from)) {
-			inode_unlock_shared(inode);
-			inode_lock(inode);
-			exclusive_lock = true;
-		}
-	}
-
-	res = generic_write_checks(iocb, from);
-	if (res > 0) {
-		if (!is_sync_kiocb(iocb) && iocb->ki_flags & IOCB_DIRECT) {
-			res = fuse_direct_IO(iocb, from);
-		} else {
-			res = fuse_send_dio(&io, from, &iocb->ki_pos,
-					    FUSE_DIO_WRITE);
-			fuse_write_update_attr(inode, iocb->ki_pos, res);
-		}
-	}
-	if (exclusive_lock)
-		inode_unlock(inode);
-	else
-		inode_unlock_shared(inode);
-
-	return res;
-}
-
 static ssize_t fuse_file_read_iter(struct kiocb *iocb, struct iov_iter *to)
 {
 	struct file *file = iocb->ki_filp;
@@ -1678,10 +1632,10 @@ static ssize_t fuse_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
 	if (FUSE_IS_DAX(inode))
 		return fuse_dax_write_iter(iocb, from);
 
-	if (!(ff->open_flags & FOPEN_DIRECT_IO))
-		return fuse_cache_write_iter(iocb, from);
-	else
-		return fuse_direct_write_iter(iocb, from);
+	if (ff->open_flags & FOPEN_DIRECT_IO)
+		iocb->ki_flags |= IOCB_DIRECT;
+
+	return fuse_cache_write_iter(iocb, from);
 }
 
 static void fuse_writepage_free(struct fuse_writepage_args *wpa)
-- 
2.39.2


  parent reply	other threads:[~2023-08-29 16:13 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-08-29 16:11 [PATCH 0/5 v3] fuse direct write consolidation and parallel IO Bernd Schubert
2023-08-29 16:11 ` [PATCH 1/6] fuse: direct IO can use the write-through code path Bernd Schubert
2023-08-29 16:11 ` [PATCH 2/6] fuse: Create helper function if DIO write needs exclusive lock Bernd Schubert
2023-08-30 10:57   ` Miklos Szeredi
2023-08-30 12:13     ` Bernd Schubert
2023-08-30 12:14       ` Miklos Szeredi
2023-08-29 16:11 ` [PATCH 3/6] fuse: Allow parallel direct writes for O_DIRECT Bernd Schubert
2023-08-30 13:28   ` Miklos Szeredi
2023-08-30 14:38     ` Bernd Schubert
2023-08-30 14:50       ` Miklos Szeredi
2023-08-31  8:30   ` Hao Xu
2023-08-31  8:33     ` Bernd Schubert
2023-08-29 16:11 ` [PATCH 4/6] fuse: Rename fuse_direct_io Bernd Schubert
2023-08-29 16:11 ` Bernd Schubert [this message]
2023-08-31  9:19   ` [PATCH 5/6] fuse: Remove fuse_direct_write_iter code path / use IOCB_DIRECT Hao Xu
2023-08-31  9:34     ` Bernd Schubert
2023-09-01  2:54       ` Hao Xu
2023-08-29 16:11 ` [PATCH 6/6] fuse: Remove page flush/invaliation in fuse_direct_io Bernd Schubert

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230829161116.2914040-6-bschubert@ddn.com \
    --to=bschubert@ddn.com \
    --cc=bernd.schubert@fastmail.fm \
    --cc=dsingh@ddn.com \
    --cc=howeyxu@tencent.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=miklos@szeredi.hu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.