From: Amir Goldstein <amir73il@gmail.com>
To: "Darrick J . Wong" <djwong@kernel.org>
Cc: Leah Rumancik <leah.rumancik@gmail.com>,
Chandan Babu R <chandan.babu@oracle.com>,
linux-xfs@vger.kernel.org, fstests@vger.kernel.org,
Christoph Hellwig <hch@lst.de>, Brian Foster <bfoster@redhat.com>,
Dave Chinner <dchinner@redhat.com>
Subject: [PATCH 5.10 CANDIDATE 1/9] xfs: refactor xfs_file_fsync
Date: Tue, 26 Jul 2022 11:21:17 +0200 [thread overview]
Message-ID: <20220726092125.3899077-2-amir73il@gmail.com> (raw)
In-Reply-To: <20220726092125.3899077-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
next prev parent reply other threads:[~2022-07-26 9:21 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-07-26 9:21 [PATCH 5.10 CANDIDATE 0/9] xfs stable candidate patches for 5.10.y (from v5.13+) Amir Goldstein
2022-07-26 9:21 ` Amir Goldstein [this message]
2022-07-26 9:21 ` [PATCH 5.10 CANDIDATE 2/9] xfs: xfs_log_force_lsn isn't passed a LSN Amir Goldstein
2022-07-26 9:21 ` [PATCH 5.10 CANDIDATE 3/9] xfs: prevent UAF in xfs_log_item_in_current_chkpt Amir Goldstein
2022-07-26 9:21 ` [PATCH 5.10 CANDIDATE 4/9] xfs: fix log intent recovery ENOSPC shutdowns when inactivating inodes Amir Goldstein
2022-07-26 9:21 ` [PATCH 5.10 CANDIDATE 5/9] xfs: force the log offline when log intent item recovery fails Amir Goldstein
2022-07-26 9:21 ` [PATCH 5.10 CANDIDATE 6/9] xfs: hold buffer across unpin and potential shutdown processing Amir Goldstein
2022-07-26 9:21 ` [PATCH 5.10 CANDIDATE 7/9] xfs: remove dead stale buf unpin handling code Amir Goldstein
2022-07-26 9:21 ` [PATCH 5.10 CANDIDATE 8/9] xfs: logging the on disk inode LSN can make it go backwards Amir Goldstein
2022-07-26 9:21 ` [PATCH 5.10 CANDIDATE 9/9] xfs: Enforce attr3 buffer recovery order Amir Goldstein
2022-07-27 19:17 ` [PATCH 5.10 CANDIDATE 0/9] xfs stable candidate patches for 5.10.y (from v5.13+) Amir Goldstein
2022-07-28 2:01 ` Darrick J. Wong
2022-07-28 2:07 ` Darrick J. Wong
2022-07-28 9:39 ` Amir Goldstein
2022-07-29 16:15 ` 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=20220726092125.3899077-2-amir73il@gmail.com \
--to=amir73il@gmail.com \
--cc=bfoster@redhat.com \
--cc=chandan.babu@oracle.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 \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).