diff mbox series

[2/2] xfs: reduce ilock acquisitions in xfs_file_fsync

Message ID 20210111161544.1414409-3-hch@lst.de (mailing list archive)
State Superseded
Headers show
Series [1/2] xfs: refactor xfs_file_fsync | expand

Commit Message

Christoph Hellwig Jan. 11, 2021, 4:15 p.m. UTC
If the inode is not pinned by the time fsync is called we don't need the
ilock to protect against concurrent clearing of ili_fsync_fields as the
inode won't need a log flush or clearing of these fields.  Not taking
the iolock allows for full concurrency of fsync and thus O_DSYNC
completions with io_uring/aio write submissions.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_file.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Brian Foster Jan. 12, 2021, 3:34 p.m. UTC | #1
On Mon, Jan 11, 2021 at 05:15:44PM +0100, Christoph Hellwig wrote:
> If the inode is not pinned by the time fsync is called we don't need the
> ilock to protect against concurrent clearing of ili_fsync_fields as the
> inode won't need a log flush or clearing of these fields.  Not taking
> the iolock allows for full concurrency of fsync and thus O_DSYNC
> completions with io_uring/aio write submissions.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---

So this changes fsync semantics for when a concurrent modification might
already be in progress (but not yet complete) to essentially skip the
log force rather than serialize/wait and force. This seems.. reasonable
I suppose since nothign has committed at that point, but I feel like
could use more documentation and justification around that and why this
might be acceptable behavior.

Brian

>  fs/xfs/xfs_file.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
> index 414d856e2e755a..ba02780dee6439 100644
> --- a/fs/xfs/xfs_file.c
> +++ b/fs/xfs/xfs_file.c
> @@ -200,7 +200,8 @@ xfs_file_fsync(
>  	else if (mp->m_logdev_targp != mp->m_ddev_targp)
>  		xfs_blkdev_issue_flush(mp->m_ddev_targp);
>  
> -	error = xfs_fsync_flush_log(ip, datasync, &log_flushed);
> +	if (xfs_ipincount(ip))
> +		error = xfs_fsync_flush_log(ip, datasync, &log_flushed);
>  
>  	/*
>  	 * If we only have a single device, and the log force about was
> -- 
> 2.29.2
>
diff mbox series

Patch

diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
index 414d856e2e755a..ba02780dee6439 100644
--- a/fs/xfs/xfs_file.c
+++ b/fs/xfs/xfs_file.c
@@ -200,7 +200,8 @@  xfs_file_fsync(
 	else if (mp->m_logdev_targp != mp->m_ddev_targp)
 		xfs_blkdev_issue_flush(mp->m_ddev_targp);
 
-	error = xfs_fsync_flush_log(ip, datasync, &log_flushed);
+	if (xfs_ipincount(ip))
+		error = xfs_fsync_flush_log(ip, datasync, &log_flushed);
 
 	/*
 	 * If we only have a single device, and the log force about was