diff mbox series

[06/43] xfs: refactor xfs_fs_statfs

Message ID 20241211085636.1380516-7-hch@lst.de (mailing list archive)
State New
Headers show
Series [01/43] xfs: constify feature checks | expand

Commit Message

Christoph Hellwig Dec. 11, 2024, 8:54 a.m. UTC
Split out helper for data, rt data and inode related informations,
and assing f_bavail once instead of in three places.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_qm_bhv.c |   2 +-
 fs/xfs/xfs_super.c  | 128 ++++++++++++++++++++++++++------------------
 2 files changed, 78 insertions(+), 52 deletions(-)

Comments

Darrick J. Wong Dec. 12, 2024, 9:24 p.m. UTC | #1
On Wed, Dec 11, 2024 at 09:54:31AM +0100, Christoph Hellwig wrote:
> Split out helper for data, rt data and inode related informations,
> and assing f_bavail once instead of in three places.

      ^^^^^^ word choice

("assigning"?)

> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/xfs/xfs_qm_bhv.c |   2 +-
>  fs/xfs/xfs_super.c  | 128 ++++++++++++++++++++++++++------------------
>  2 files changed, 78 insertions(+), 52 deletions(-)
> 
> diff --git a/fs/xfs/xfs_qm_bhv.c b/fs/xfs/xfs_qm_bhv.c
> index 847ba29630e9..6d5de3fa58e8 100644
> --- a/fs/xfs/xfs_qm_bhv.c
> +++ b/fs/xfs/xfs_qm_bhv.c
> @@ -34,7 +34,7 @@ xfs_fill_statvfs_from_dquot(
>  		blkres->hardlimit;
>  	if (limit && statp->f_blocks > limit) {
>  		statp->f_blocks = limit;
> -		statp->f_bfree = statp->f_bavail =
> +		statp->f_bfree =
>  			(statp->f_blocks > blkres->reserved) ?
>  			 (statp->f_blocks - blkres->reserved) : 0;
>  	}
> diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> index bfa8cc927009..a74a0cc1f6f6 100644
> --- a/fs/xfs/xfs_super.c
> +++ b/fs/xfs/xfs_super.c
> @@ -819,20 +819,74 @@ xfs_fs_sync_fs(
>  	return 0;
>  }
>  
> +static xfs_extlen_t
> +xfs_internal_log_size(
> +	struct xfs_mount	*mp)
> +{
> +	if (!mp->m_sb.sb_logstart)
> +		return 0;
> +	return mp->m_sb.sb_logblocks;
> +}
> +
> +static void
> +xfs_statfs_data(
> +	struct xfs_mount	*mp,
> +	struct kstatfs		*st)
> +{
> +	int64_t			fdblocks =
> +		percpu_counter_sum(&mp->m_fdblocks);
> +
> +	/* make sure st->f_bfree does not underflow */
> +	st->f_bfree = max(0LL, fdblocks - xfs_fdblocks_unavailable(mp));
> +	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)
> +{
> +	int64_t			freertx =
> +		percpu_counter_sum_positive(&mp->m_frextents);
> +
> +	st->f_bfree = xfs_rtbxlen_to_blen(mp, freertx);
> +	st->f_blocks = mp->m_sb.sb_rblocks;
> +}
> +
> +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);
> +
> +	st->f_files = min(icount + fakeinos, (uint64_t)XFS_MAXINUMBER);
> +	if (M_IGEO(mp)->maxicount)
> +		st->f_files = min_t(typeof(st->f_files), st->f_files,
> +					M_IGEO(mp)->maxicount);
> +
> +	/* If sb_icount overshot maxicount, report actual allocation */
> +	st->f_files = max_t(typeof(st->f_files), st->f_files,
> +			mp->m_sb.sb_icount);
> +
> +	/* Make sure st->f_ffree does not underflow */
> +	st->f_ffree = max_t(int64_t, 0, st->f_files - (icount - ifree));
> +}
> +
>  STATIC int
>  xfs_fs_statfs(
>  	struct dentry		*dentry,
> -	struct kstatfs		*statp)
> +	struct kstatfs		*st)
>  {
>  	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
> @@ -840,56 +894,28 @@ xfs_fs_statfs(
>  	 */
>  	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);
> -
> -	icount = percpu_counter_sum(&mp->m_icount);
> -	ifree = percpu_counter_sum(&mp->m_ifree);
> -	fdblocks = percpu_counter_sum(&mp->m_fdblocks);
> -
> -	statp->f_bsize = sbp->sb_blocksize;
> -	lsize = sbp->sb_logstart ? sbp->sb_logblocks : 0;
> -	statp->f_blocks = sbp->sb_dblocks - lsize;
> -
> -	/* 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;
> -
> -	fakeinos = XFS_FSB_TO_INO(mp, statp->f_bfree);
> -	statp->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,
> -					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);
> -
> -	/* make sure statp->f_ffree does not underflow */
> -	ffree = statp->f_files - (icount - ifree);
> -	statp->f_ffree = max_t(int64_t, ffree, 0);
> +	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));
> +		

Whitespace ^^^ damage here.

> +	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))) {
> -		s64	freertx;
> -
> -		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);
> -	}
> +	    (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);

Nice cleanup of all of that.

> +	/*
> +	 * XFS does not distinguish between blocks available to privileged and
> +	 * unprivileged users.
> +	 */
> +	st->f_bavail = st->f_bfree;

Not relevant to this patch, but I noticed that (a) the statfs manpage
now tells me to go look at statvfs, and (b) statvfs advertises a
f_favail field that nobody in the kernel actually sets.

--D

>  	return 0;
>  }
>  
> -- 
> 2.45.2
> 
>
Christoph Hellwig Dec. 13, 2024, 5:08 a.m. UTC | #2
On Thu, Dec 12, 2024 at 01:24:00PM -0800, Darrick J. Wong wrote:
> > +	 * XFS does not distinguish between blocks available to privileged and
> > +	 * unprivileged users.
> > +	 */
> > +	st->f_bavail = st->f_bfree;
> 
> Not relevant to this patch, but I noticed that (a) the statfs manpage
> now tells me to go look at statvfs, and (b) statvfs advertises a
> f_favail field that nobody in the kernel actually sets.

The kernel doesn't implement statvfs and thus doesn't have the field.
This is what glibc does for it:

  /* XXX I have no idea how to compute f_favail.  Any idea???  */
  buf->f_favail = buf->f_ffree;
diff mbox series

Patch

diff --git a/fs/xfs/xfs_qm_bhv.c b/fs/xfs/xfs_qm_bhv.c
index 847ba29630e9..6d5de3fa58e8 100644
--- a/fs/xfs/xfs_qm_bhv.c
+++ b/fs/xfs/xfs_qm_bhv.c
@@ -34,7 +34,7 @@  xfs_fill_statvfs_from_dquot(
 		blkres->hardlimit;
 	if (limit && statp->f_blocks > limit) {
 		statp->f_blocks = limit;
-		statp->f_bfree = statp->f_bavail =
+		statp->f_bfree =
 			(statp->f_blocks > blkres->reserved) ?
 			 (statp->f_blocks - blkres->reserved) : 0;
 	}
diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index bfa8cc927009..a74a0cc1f6f6 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -819,20 +819,74 @@  xfs_fs_sync_fs(
 	return 0;
 }
 
+static xfs_extlen_t
+xfs_internal_log_size(
+	struct xfs_mount	*mp)
+{
+	if (!mp->m_sb.sb_logstart)
+		return 0;
+	return mp->m_sb.sb_logblocks;
+}
+
+static void
+xfs_statfs_data(
+	struct xfs_mount	*mp,
+	struct kstatfs		*st)
+{
+	int64_t			fdblocks =
+		percpu_counter_sum(&mp->m_fdblocks);
+
+	/* make sure st->f_bfree does not underflow */
+	st->f_bfree = max(0LL, fdblocks - xfs_fdblocks_unavailable(mp));
+	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)
+{
+	int64_t			freertx =
+		percpu_counter_sum_positive(&mp->m_frextents);
+
+	st->f_bfree = xfs_rtbxlen_to_blen(mp, freertx);
+	st->f_blocks = mp->m_sb.sb_rblocks;
+}
+
+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);
+
+	st->f_files = min(icount + fakeinos, (uint64_t)XFS_MAXINUMBER);
+	if (M_IGEO(mp)->maxicount)
+		st->f_files = min_t(typeof(st->f_files), st->f_files,
+					M_IGEO(mp)->maxicount);
+
+	/* If sb_icount overshot maxicount, report actual allocation */
+	st->f_files = max_t(typeof(st->f_files), st->f_files,
+			mp->m_sb.sb_icount);
+
+	/* Make sure st->f_ffree does not underflow */
+	st->f_ffree = max_t(int64_t, 0, st->f_files - (icount - ifree));
+}
+
 STATIC int
 xfs_fs_statfs(
 	struct dentry		*dentry,
-	struct kstatfs		*statp)
+	struct kstatfs		*st)
 {
 	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
@@ -840,56 +894,28 @@  xfs_fs_statfs(
 	 */
 	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);
-
-	icount = percpu_counter_sum(&mp->m_icount);
-	ifree = percpu_counter_sum(&mp->m_ifree);
-	fdblocks = percpu_counter_sum(&mp->m_fdblocks);
-
-	statp->f_bsize = sbp->sb_blocksize;
-	lsize = sbp->sb_logstart ? sbp->sb_logblocks : 0;
-	statp->f_blocks = sbp->sb_dblocks - lsize;
-
-	/* 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;
-
-	fakeinos = XFS_FSB_TO_INO(mp, statp->f_bfree);
-	statp->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,
-					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);
-
-	/* make sure statp->f_ffree does not underflow */
-	ffree = statp->f_files - (icount - ifree);
-	statp->f_ffree = max_t(int64_t, ffree, 0);
+	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))) {
-		s64	freertx;
-
-		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);
-	}
+	    (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;
 }