* [PATCH 1/2] xfs: don't take m_sb_lock in xfs_fs_statfs
@ 2025-01-13 4:32 Christoph Hellwig
2025-01-13 4:32 ` [PATCH 2/2] xfs: refactor xfs_fs_statfs Christoph Hellwig
2025-01-14 10:30 ` [PATCH 1/2] xfs: don't take m_sb_lock in xfs_fs_statfs Carlos Maiolino
0 siblings, 2 replies; 4+ messages in thread
From: Christoph Hellwig @ 2025-01-13 4:32 UTC (permalink / raw)
To: cem; +Cc: djwong, linux-xfs
The only non-constant value read under m_sb_lock in xfs_fs_statfs is
sb_dblocks, and it could become stale right after dropping the lock
anyway. Remove the thus pointless lock section.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>
---
fs/xfs/xfs_super.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index 7c3f996cd39e..20cc00b992a6 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -850,11 +850,13 @@ xfs_fs_statfs(
ifree = percpu_counter_sum(&mp->m_ifree);
fdblocks = percpu_counter_sum(&mp->m_fdblocks);
- spin_lock(&mp->m_sb_lock);
statp->f_bsize = sbp->sb_blocksize;
lsize = sbp->sb_logstart ? sbp->sb_logblocks : 0;
+ /*
+ * sb_dblocks can change during growfs, but nothing cares about reporting
+ * the old or new value during growfs.
+ */
statp->f_blocks = sbp->sb_dblocks - lsize;
- spin_unlock(&mp->m_sb_lock);
/* make sure statp->f_bfree does not underflow */
statp->f_bfree = max_t(int64_t, 0,
--
2.45.2
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH 2/2] xfs: refactor xfs_fs_statfs
2025-01-13 4:32 [PATCH 1/2] xfs: don't take m_sb_lock in xfs_fs_statfs Christoph Hellwig
@ 2025-01-13 4:32 ` Christoph Hellwig
2025-01-13 5:04 ` Darrick J. Wong
2025-01-14 10:30 ` [PATCH 1/2] xfs: don't take m_sb_lock in xfs_fs_statfs Carlos Maiolino
1 sibling, 1 reply; 4+ messages in thread
From: Christoph Hellwig @ 2025-01-13 4:32 UTC (permalink / raw)
To: cem; +Cc: djwong, linux-xfs
Split out helpers for data, rt data and inode related information, and
assigning f_bavail once instead of in three places.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/xfs/xfs_qm_bhv.c | 1 -
fs/xfs/xfs_super.c | 132 ++++++++++++++++++++++++++------------------
2 files changed, 78 insertions(+), 55 deletions(-)
diff --git a/fs/xfs/xfs_qm_bhv.c b/fs/xfs/xfs_qm_bhv.c
index db5b8afd9d1b..37f1230e7584 100644
--- a/fs/xfs/xfs_qm_bhv.c
+++ b/fs/xfs/xfs_qm_bhv.c
@@ -40,7 +40,6 @@ xfs_fill_statvfs_from_dquot(
statp->f_blocks = min(statp->f_blocks, limit);
statp->f_bfree = min(statp->f_bfree, remaining);
- statp->f_bavail = min(statp->f_bavail, remaining);
}
limit = dqp->q_ino.softlimit ?
diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index 20cc00b992a6..809ac6d1813c 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -819,81 +819,105 @@ xfs_fs_sync_fs(
return 0;
}
-STATIC int
-xfs_fs_statfs(
- struct dentry *dentry,
- struct kstatfs *statp)
+static xfs_extlen_t
+xfs_internal_log_size(
+ struct xfs_mount *mp)
{
- struct xfs_mount *mp = XFS_M(dentry->d_sb);
- xfs_sb_t *sbp = &mp->m_sb;
- struct xfs_inode *ip = XFS_I(d_inode(dentry));
- uint64_t fakeinos, id;
- uint64_t icount;
- uint64_t ifree;
- uint64_t fdblocks;
- xfs_extlen_t lsize;
- int64_t ffree;
-
- /*
- * Expedite background inodegc but don't wait. We do not want to block
- * here waiting hours for a billion extent file to be truncated.
- */
- xfs_inodegc_push(mp);
-
- statp->f_type = XFS_SUPER_MAGIC;
- statp->f_namelen = MAXNAMELEN - 1;
-
- id = huge_encode_dev(mp->m_ddev_targp->bt_dev);
- statp->f_fsid = u64_to_fsid(id);
+ if (!mp->m_sb.sb_logstart)
+ return 0;
+ return mp->m_sb.sb_logblocks;
+}
- icount = percpu_counter_sum(&mp->m_icount);
- ifree = percpu_counter_sum(&mp->m_ifree);
- fdblocks = percpu_counter_sum(&mp->m_fdblocks);
+static void
+xfs_statfs_data(
+ struct xfs_mount *mp,
+ struct kstatfs *st)
+{
+ int64_t fdblocks =
+ percpu_counter_sum(&mp->m_fdblocks);
- statp->f_bsize = sbp->sb_blocksize;
- lsize = sbp->sb_logstart ? sbp->sb_logblocks : 0;
+ /* make sure st->f_bfree does not underflow */
+ st->f_bfree = max(0LL, fdblocks - xfs_fdblocks_unavailable(mp));
/*
* sb_dblocks can change during growfs, but nothing cares about reporting
* the old or new value during growfs.
*/
- statp->f_blocks = sbp->sb_dblocks - lsize;
+ st->f_blocks = mp->m_sb.sb_dblocks - xfs_internal_log_size(mp);
+}
+
+/*
+ * When stat(v)fs is called on a file with the realtime bit set or a directory
+ * with the rtinherit bit, report freespace information for the RT device
+ * instead of the main data device.
+ */
+static void
+xfs_statfs_rt(
+ struct xfs_mount *mp,
+ struct kstatfs *st)
+{
+ st->f_bfree = xfs_rtbxlen_to_blen(mp,
+ percpu_counter_sum_positive(&mp->m_frextents));
+ st->f_blocks = mp->m_sb.sb_rblocks;
+}
- /* make sure statp->f_bfree does not underflow */
- statp->f_bfree = max_t(int64_t, 0,
- fdblocks - xfs_fdblocks_unavailable(mp));
- statp->f_bavail = statp->f_bfree;
+static void
+xfs_statfs_inodes(
+ struct xfs_mount *mp,
+ struct kstatfs *st)
+{
+ uint64_t icount = percpu_counter_sum(&mp->m_icount);
+ uint64_t ifree = percpu_counter_sum(&mp->m_ifree);
+ uint64_t fakeinos = XFS_FSB_TO_INO(mp, st->f_bfree);
- fakeinos = XFS_FSB_TO_INO(mp, statp->f_bfree);
- statp->f_files = min(icount + fakeinos, (uint64_t)XFS_MAXINUMBER);
+ st->f_files = min(icount + fakeinos, (uint64_t)XFS_MAXINUMBER);
if (M_IGEO(mp)->maxicount)
- statp->f_files = min_t(typeof(statp->f_files),
- statp->f_files,
+ st->f_files = min_t(typeof(st->f_files), st->f_files,
M_IGEO(mp)->maxicount);
/* If sb_icount overshot maxicount, report actual allocation */
- statp->f_files = max_t(typeof(statp->f_files),
- statp->f_files,
- sbp->sb_icount);
+ st->f_files = max_t(typeof(st->f_files), st->f_files,
+ mp->m_sb.sb_icount);
- /* make sure statp->f_ffree does not underflow */
- ffree = statp->f_files - (icount - ifree);
- statp->f_ffree = max_t(int64_t, ffree, 0);
+ /* Make sure st->f_ffree does not underflow */
+ st->f_ffree = max_t(int64_t, 0, st->f_files - (icount - ifree));
+}
- if (XFS_IS_REALTIME_MOUNT(mp) &&
- (ip->i_diflags & (XFS_DIFLAG_RTINHERIT | XFS_DIFLAG_REALTIME))) {
- s64 freertx;
+STATIC int
+xfs_fs_statfs(
+ struct dentry *dentry,
+ struct kstatfs *st)
+{
+ struct xfs_mount *mp = XFS_M(dentry->d_sb);
+ struct xfs_inode *ip = XFS_I(d_inode(dentry));
- statp->f_blocks = sbp->sb_rblocks;
- freertx = percpu_counter_sum_positive(&mp->m_frextents);
- statp->f_bavail = statp->f_bfree =
- xfs_rtbxlen_to_blen(mp, freertx);
- }
+ /*
+ * Expedite background inodegc but don't wait. We do not want to block
+ * here waiting hours for a billion extent file to be truncated.
+ */
+ xfs_inodegc_push(mp);
+
+ st->f_type = XFS_SUPER_MAGIC;
+ st->f_namelen = MAXNAMELEN - 1;
+ st->f_bsize = mp->m_sb.sb_blocksize;
+ st->f_fsid = u64_to_fsid(huge_encode_dev(mp->m_ddev_targp->bt_dev));
+
+ xfs_statfs_data(mp, st);
+ xfs_statfs_inodes(mp, st);
+
+ if (XFS_IS_REALTIME_MOUNT(mp) &&
+ (ip->i_diflags & (XFS_DIFLAG_RTINHERIT | XFS_DIFLAG_REALTIME)))
+ xfs_statfs_rt(mp, st);
if ((ip->i_diflags & XFS_DIFLAG_PROJINHERIT) &&
((mp->m_qflags & (XFS_PQUOTA_ACCT|XFS_PQUOTA_ENFD))) ==
(XFS_PQUOTA_ACCT|XFS_PQUOTA_ENFD))
- xfs_qm_statvfs(ip, statp);
+ xfs_qm_statvfs(ip, st);
+ /*
+ * XFS does not distinguish between blocks available to privileged and
+ * unprivileged users.
+ */
+ st->f_bavail = st->f_bfree;
return 0;
}
--
2.45.2
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 2/2] xfs: refactor xfs_fs_statfs
2025-01-13 4:32 ` [PATCH 2/2] xfs: refactor xfs_fs_statfs Christoph Hellwig
@ 2025-01-13 5:04 ` Darrick J. Wong
0 siblings, 0 replies; 4+ messages in thread
From: Darrick J. Wong @ 2025-01-13 5:04 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: cem, linux-xfs
On Mon, Jan 13, 2025 at 05:32:59AM +0100, Christoph Hellwig wrote:
> Split out helpers for data, rt data and inode related information, and
> assigning f_bavail once instead of in three places.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
Looks good now,
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>
--D
> ---
> fs/xfs/xfs_qm_bhv.c | 1 -
> fs/xfs/xfs_super.c | 132 ++++++++++++++++++++++++++------------------
> 2 files changed, 78 insertions(+), 55 deletions(-)
>
> diff --git a/fs/xfs/xfs_qm_bhv.c b/fs/xfs/xfs_qm_bhv.c
> index db5b8afd9d1b..37f1230e7584 100644
> --- a/fs/xfs/xfs_qm_bhv.c
> +++ b/fs/xfs/xfs_qm_bhv.c
> @@ -40,7 +40,6 @@ xfs_fill_statvfs_from_dquot(
>
> statp->f_blocks = min(statp->f_blocks, limit);
> statp->f_bfree = min(statp->f_bfree, remaining);
> - statp->f_bavail = min(statp->f_bavail, remaining);
> }
>
> limit = dqp->q_ino.softlimit ?
> diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> index 20cc00b992a6..809ac6d1813c 100644
> --- a/fs/xfs/xfs_super.c
> +++ b/fs/xfs/xfs_super.c
> @@ -819,81 +819,105 @@ xfs_fs_sync_fs(
> return 0;
> }
>
> -STATIC int
> -xfs_fs_statfs(
> - struct dentry *dentry,
> - struct kstatfs *statp)
> +static xfs_extlen_t
> +xfs_internal_log_size(
> + struct xfs_mount *mp)
> {
> - struct xfs_mount *mp = XFS_M(dentry->d_sb);
> - xfs_sb_t *sbp = &mp->m_sb;
> - struct xfs_inode *ip = XFS_I(d_inode(dentry));
> - uint64_t fakeinos, id;
> - uint64_t icount;
> - uint64_t ifree;
> - uint64_t fdblocks;
> - xfs_extlen_t lsize;
> - int64_t ffree;
> -
> - /*
> - * Expedite background inodegc but don't wait. We do not want to block
> - * here waiting hours for a billion extent file to be truncated.
> - */
> - xfs_inodegc_push(mp);
> -
> - statp->f_type = XFS_SUPER_MAGIC;
> - statp->f_namelen = MAXNAMELEN - 1;
> -
> - id = huge_encode_dev(mp->m_ddev_targp->bt_dev);
> - statp->f_fsid = u64_to_fsid(id);
> + if (!mp->m_sb.sb_logstart)
> + return 0;
> + return mp->m_sb.sb_logblocks;
> +}
>
> - icount = percpu_counter_sum(&mp->m_icount);
> - ifree = percpu_counter_sum(&mp->m_ifree);
> - fdblocks = percpu_counter_sum(&mp->m_fdblocks);
> +static void
> +xfs_statfs_data(
> + struct xfs_mount *mp,
> + struct kstatfs *st)
> +{
> + int64_t fdblocks =
> + percpu_counter_sum(&mp->m_fdblocks);
>
> - statp->f_bsize = sbp->sb_blocksize;
> - lsize = sbp->sb_logstart ? sbp->sb_logblocks : 0;
> + /* make sure st->f_bfree does not underflow */
> + st->f_bfree = max(0LL, fdblocks - xfs_fdblocks_unavailable(mp));
> /*
> * sb_dblocks can change during growfs, but nothing cares about reporting
> * the old or new value during growfs.
> */
> - statp->f_blocks = sbp->sb_dblocks - lsize;
> + st->f_blocks = mp->m_sb.sb_dblocks - xfs_internal_log_size(mp);
> +}
> +
> +/*
> + * When stat(v)fs is called on a file with the realtime bit set or a directory
> + * with the rtinherit bit, report freespace information for the RT device
> + * instead of the main data device.
> + */
> +static void
> +xfs_statfs_rt(
> + struct xfs_mount *mp,
> + struct kstatfs *st)
> +{
> + st->f_bfree = xfs_rtbxlen_to_blen(mp,
> + percpu_counter_sum_positive(&mp->m_frextents));
> + st->f_blocks = mp->m_sb.sb_rblocks;
> +}
>
> - /* make sure statp->f_bfree does not underflow */
> - statp->f_bfree = max_t(int64_t, 0,
> - fdblocks - xfs_fdblocks_unavailable(mp));
> - statp->f_bavail = statp->f_bfree;
> +static void
> +xfs_statfs_inodes(
> + struct xfs_mount *mp,
> + struct kstatfs *st)
> +{
> + uint64_t icount = percpu_counter_sum(&mp->m_icount);
> + uint64_t ifree = percpu_counter_sum(&mp->m_ifree);
> + uint64_t fakeinos = XFS_FSB_TO_INO(mp, st->f_bfree);
>
> - fakeinos = XFS_FSB_TO_INO(mp, statp->f_bfree);
> - statp->f_files = min(icount + fakeinos, (uint64_t)XFS_MAXINUMBER);
> + st->f_files = min(icount + fakeinos, (uint64_t)XFS_MAXINUMBER);
> if (M_IGEO(mp)->maxicount)
> - statp->f_files = min_t(typeof(statp->f_files),
> - statp->f_files,
> + st->f_files = min_t(typeof(st->f_files), st->f_files,
> M_IGEO(mp)->maxicount);
>
> /* If sb_icount overshot maxicount, report actual allocation */
> - statp->f_files = max_t(typeof(statp->f_files),
> - statp->f_files,
> - sbp->sb_icount);
> + st->f_files = max_t(typeof(st->f_files), st->f_files,
> + mp->m_sb.sb_icount);
>
> - /* make sure statp->f_ffree does not underflow */
> - ffree = statp->f_files - (icount - ifree);
> - statp->f_ffree = max_t(int64_t, ffree, 0);
> + /* Make sure st->f_ffree does not underflow */
> + st->f_ffree = max_t(int64_t, 0, st->f_files - (icount - ifree));
> +}
>
> - if (XFS_IS_REALTIME_MOUNT(mp) &&
> - (ip->i_diflags & (XFS_DIFLAG_RTINHERIT | XFS_DIFLAG_REALTIME))) {
> - s64 freertx;
> +STATIC int
> +xfs_fs_statfs(
> + struct dentry *dentry,
> + struct kstatfs *st)
> +{
> + struct xfs_mount *mp = XFS_M(dentry->d_sb);
> + struct xfs_inode *ip = XFS_I(d_inode(dentry));
>
> - statp->f_blocks = sbp->sb_rblocks;
> - freertx = percpu_counter_sum_positive(&mp->m_frextents);
> - statp->f_bavail = statp->f_bfree =
> - xfs_rtbxlen_to_blen(mp, freertx);
> - }
> + /*
> + * Expedite background inodegc but don't wait. We do not want to block
> + * here waiting hours for a billion extent file to be truncated.
> + */
> + xfs_inodegc_push(mp);
> +
> + st->f_type = XFS_SUPER_MAGIC;
> + st->f_namelen = MAXNAMELEN - 1;
> + st->f_bsize = mp->m_sb.sb_blocksize;
> + st->f_fsid = u64_to_fsid(huge_encode_dev(mp->m_ddev_targp->bt_dev));
> +
> + xfs_statfs_data(mp, st);
> + xfs_statfs_inodes(mp, st);
> +
> + if (XFS_IS_REALTIME_MOUNT(mp) &&
> + (ip->i_diflags & (XFS_DIFLAG_RTINHERIT | XFS_DIFLAG_REALTIME)))
> + xfs_statfs_rt(mp, st);
>
> if ((ip->i_diflags & XFS_DIFLAG_PROJINHERIT) &&
> ((mp->m_qflags & (XFS_PQUOTA_ACCT|XFS_PQUOTA_ENFD))) ==
> (XFS_PQUOTA_ACCT|XFS_PQUOTA_ENFD))
> - xfs_qm_statvfs(ip, statp);
> + xfs_qm_statvfs(ip, st);
>
> + /*
> + * XFS does not distinguish between blocks available to privileged and
> + * unprivileged users.
> + */
> + st->f_bavail = st->f_bfree;
> return 0;
> }
>
> --
> 2.45.2
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 1/2] xfs: don't take m_sb_lock in xfs_fs_statfs
2025-01-13 4:32 [PATCH 1/2] xfs: don't take m_sb_lock in xfs_fs_statfs Christoph Hellwig
2025-01-13 4:32 ` [PATCH 2/2] xfs: refactor xfs_fs_statfs Christoph Hellwig
@ 2025-01-14 10:30 ` Carlos Maiolino
1 sibling, 0 replies; 4+ messages in thread
From: Carlos Maiolino @ 2025-01-14 10:30 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: djwong, linux-xfs
On Mon, 13 Jan 2025 05:32:58 +0100, Christoph Hellwig wrote:
> The only non-constant value read under m_sb_lock in xfs_fs_statfs is
> sb_dblocks, and it could become stale right after dropping the lock
> anyway. Remove the thus pointless lock section.
>
>
Applied to for-next, thanks!
[1/2] xfs: don't take m_sb_lock in xfs_fs_statfs
commit: 72843ca62417a0587ca98791b172e4c3b3f8d3a8
[2/2] xfs: refactor xfs_fs_statfs
commit: dd324cb79e54d3a61621f09c346ee050315a4d2e
Best regards,
--
Carlos Maiolino <cem@kernel.org>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2025-01-14 10:30 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-01-13 4:32 [PATCH 1/2] xfs: don't take m_sb_lock in xfs_fs_statfs Christoph Hellwig
2025-01-13 4:32 ` [PATCH 2/2] xfs: refactor xfs_fs_statfs Christoph Hellwig
2025-01-13 5:04 ` Darrick J. Wong
2025-01-14 10:30 ` [PATCH 1/2] xfs: don't take m_sb_lock in xfs_fs_statfs Carlos Maiolino
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox