All of lore.kernel.org
 help / color / mirror / Atom feed
From: Amir Goldstein <amir73il@gmail.com>
To: "Darrick J . Wong" <djwong@kernel.org>
Cc: Leah Rumancik <leah.rumancik@gmail.com>,
	Luis Chamberlain <mcgrof@kernel.org>,
	Dave Chinner <david@fromorbit.com>,
	Christoph Hellwig <hch@lst.de>,
	Christian Brauner <christian.brauner@ubuntu.com>,
	linux-xfs@vger.kernel.org, fstests@vger.kernel.org,
	Brian Foster <bfoster@redhat.com>,
	Dave Chinner <dchinner@redhat.com>
Subject: [PATCH 5.10 CANDIDATE 06/11] xfs: refactor xfs_file_fsync
Date: Fri, 17 Jun 2022 13:06:36 +0300	[thread overview]
Message-ID: <20220617100641.1653164-7-amir73il@gmail.com> (raw)
In-Reply-To: <20220617100641.1653164-1-amir73il@gmail.com>

From: Christoph Hellwig <hch@lst.de>

commit f22c7f87777361f94aa17f746fbadfa499248dc8 upstream.

[backported for dependency]

Factor out the log syncing logic into two helpers to make the code easier
to read and more maintainable.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Brian Foster <bfoster@redhat.com>
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Reviewed-by: Dave Chinner <dchinner@redhat.com>
Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 fs/xfs/xfs_file.c | 81 +++++++++++++++++++++++++++++------------------
 1 file changed, 50 insertions(+), 31 deletions(-)

diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
index 5b0f93f73837..414d856e2e75 100644
--- a/fs/xfs/xfs_file.c
+++ b/fs/xfs/xfs_file.c
@@ -118,6 +118,54 @@ xfs_dir_fsync(
 	return xfs_log_force_inode(ip);
 }
 
+static xfs_lsn_t
+xfs_fsync_lsn(
+	struct xfs_inode	*ip,
+	bool			datasync)
+{
+	if (!xfs_ipincount(ip))
+		return 0;
+	if (datasync && !(ip->i_itemp->ili_fsync_fields & ~XFS_ILOG_TIMESTAMP))
+		return 0;
+	return ip->i_itemp->ili_last_lsn;
+}
+
+/*
+ * All metadata updates are logged, which means that we just have to flush the
+ * log up to the latest LSN that touched the inode.
+ *
+ * If we have concurrent fsync/fdatasync() calls, we need them to all block on
+ * the log force before we clear the ili_fsync_fields field. This ensures that
+ * we don't get a racing sync operation that does not wait for the metadata to
+ * hit the journal before returning.  If we race with clearing ili_fsync_fields,
+ * then all that will happen is the log force will do nothing as the lsn will
+ * already be on disk.  We can't race with setting ili_fsync_fields because that
+ * is done under XFS_ILOCK_EXCL, and that can't happen because we hold the lock
+ * shared until after the ili_fsync_fields is cleared.
+ */
+static  int
+xfs_fsync_flush_log(
+	struct xfs_inode	*ip,
+	bool			datasync,
+	int			*log_flushed)
+{
+	int			error = 0;
+	xfs_lsn_t		lsn;
+
+	xfs_ilock(ip, XFS_ILOCK_SHARED);
+	lsn = xfs_fsync_lsn(ip, datasync);
+	if (lsn) {
+		error = xfs_log_force_lsn(ip->i_mount, lsn, XFS_LOG_SYNC,
+					  log_flushed);
+
+		spin_lock(&ip->i_itemp->ili_lock);
+		ip->i_itemp->ili_fsync_fields = 0;
+		spin_unlock(&ip->i_itemp->ili_lock);
+	}
+	xfs_iunlock(ip, XFS_ILOCK_SHARED);
+	return error;
+}
+
 STATIC int
 xfs_file_fsync(
 	struct file		*file,
@@ -125,13 +173,10 @@ xfs_file_fsync(
 	loff_t			end,
 	int			datasync)
 {
-	struct inode		*inode = file->f_mapping->host;
-	struct xfs_inode	*ip = XFS_I(inode);
-	struct xfs_inode_log_item *iip = ip->i_itemp;
+	struct xfs_inode	*ip = XFS_I(file->f_mapping->host);
 	struct xfs_mount	*mp = ip->i_mount;
 	int			error = 0;
 	int			log_flushed = 0;
-	xfs_lsn_t		lsn = 0;
 
 	trace_xfs_file_fsync(ip);
 
@@ -155,33 +200,7 @@ xfs_file_fsync(
 	else if (mp->m_logdev_targp != mp->m_ddev_targp)
 		xfs_blkdev_issue_flush(mp->m_ddev_targp);
 
-	/*
-	 * All metadata updates are logged, which means that we just have to
-	 * flush the log up to the latest LSN that touched the inode. If we have
-	 * concurrent fsync/fdatasync() calls, we need them to all block on the
-	 * log force before we clear the ili_fsync_fields field. This ensures
-	 * that we don't get a racing sync operation that does not wait for the
-	 * metadata to hit the journal before returning. If we race with
-	 * clearing the ili_fsync_fields, then all that will happen is the log
-	 * force will do nothing as the lsn will already be on disk. We can't
-	 * race with setting ili_fsync_fields because that is done under
-	 * XFS_ILOCK_EXCL, and that can't happen because we hold the lock shared
-	 * until after the ili_fsync_fields is cleared.
-	 */
-	xfs_ilock(ip, XFS_ILOCK_SHARED);
-	if (xfs_ipincount(ip)) {
-		if (!datasync ||
-		    (iip->ili_fsync_fields & ~XFS_ILOG_TIMESTAMP))
-			lsn = iip->ili_last_lsn;
-	}
-
-	if (lsn) {
-		error = xfs_log_force_lsn(mp, lsn, XFS_LOG_SYNC, &log_flushed);
-		spin_lock(&iip->ili_lock);
-		iip->ili_fsync_fields = 0;
-		spin_unlock(&iip->ili_lock);
-	}
-	xfs_iunlock(ip, XFS_ILOCK_SHARED);
+	error = xfs_fsync_flush_log(ip, datasync, &log_flushed);
 
 	/*
 	 * If we only have a single device, and the log force about was
-- 
2.25.1


  parent reply	other threads:[~2022-06-17 10:07 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-06-17 10:06 [PATCH 5.10 CANDIDATE 00/11] xfs stable candidate patches for 5.10.y (v5.15+) Amir Goldstein
2022-06-17 10:06 ` [PATCH 5.10 CANDIDATE 01/11] xfs: use kmem_cache_free() for kmem_cache objects Amir Goldstein
2022-06-17 10:06 ` [PATCH 5.10 CANDIDATE 02/11] xfs: punch out data fork delalloc blocks on COW writeback failure Amir Goldstein
2022-06-17 10:06 ` [PATCH 5.10 CANDIDATE 03/11] xfs: Fix the free logic of state in xfs_attr_node_hasname Amir Goldstein
2022-06-22 16:32   ` Darrick J. Wong
2022-06-22 18:46     ` Amir Goldstein
2022-06-22 21:50       ` Darrick J. Wong
2022-06-17 10:06 ` [PATCH 5.10 CANDIDATE 04/11] xfs: remove all COW fork extents when remounting readonly Amir Goldstein
2022-06-17 10:06 ` [PATCH 5.10 CANDIDATE 05/11] xfs: check sb_meta_uuid for dabuf buffer recovery Amir Goldstein
2022-06-17 10:06 ` Amir Goldstein [this message]
2022-06-17 10:06 ` [PATCH 5.10 CANDIDATE 07/11] xfs: xfs_log_force_lsn isn't passed a LSN Amir Goldstein
2022-06-22 16:45   ` Darrick J. Wong
2022-06-22 17:09     ` Amir Goldstein
2022-06-17 10:06 ` [PATCH 5.10 CANDIDATE 08/11] xfs: prevent UAF in xfs_log_item_in_current_chkpt Amir Goldstein
2022-06-17 10:06 ` [PATCH 5.10 CANDIDATE 09/11] xfs: only bother with sync_filesystem during readonly remount Amir Goldstein
2022-06-22 16:38   ` Darrick J. Wong
2022-06-22 16:54     ` Amir Goldstein
2022-06-22 23:42       ` Darrick J. Wong
2022-06-23  6:38         ` Amir Goldstein
2022-06-17 10:06 ` [PATCH 5.10 CANDIDATE 10/11] xfs: fix up non-directory creation in SGID directories Amir Goldstein
2022-06-17 10:06 ` [PATCH 5.10 CANDIDATE 11/11] xfs: use setattr_copy to set vfs inode attributes Amir Goldstein
2022-06-22 16:41   ` Darrick J. Wong
2022-06-22 18:36     ` Amir Goldstein
2022-06-22 22:17       ` Leah Rumancik
2022-06-23  4:22         ` Amir Goldstein
2022-06-22 23:45 ` [PATCH 5.10 CANDIDATE 00/11] xfs stable candidate patches for 5.10.y (v5.15+) Darrick J. Wong
2022-06-23  7:33   ` Amir Goldstein
2022-06-23 16:05     ` Darrick J. Wong
2022-07-24  8:36       ` Amir Goldstein
2022-07-26  2:10         ` Darrick J. Wong
2022-07-26  8:41           ` Amir Goldstein

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=20220617100641.1653164-7-amir73il@gmail.com \
    --to=amir73il@gmail.com \
    --cc=bfoster@redhat.com \
    --cc=christian.brauner@ubuntu.com \
    --cc=david@fromorbit.com \
    --cc=dchinner@redhat.com \
    --cc=djwong@kernel.org \
    --cc=fstests@vger.kernel.org \
    --cc=hch@lst.de \
    --cc=leah.rumancik@gmail.com \
    --cc=linux-xfs@vger.kernel.org \
    --cc=mcgrof@kernel.org \
    /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.