xfs
[Top] [All Lists]

[PATCH] xfs: clean up xfs_set_maxicount & use in growfs

To: xfs-oss <xfs@xxxxxxxxxxx>
Subject: [PATCH] xfs: clean up xfs_set_maxicount & use in growfs
From: Eric Sandeen <sandeen@xxxxxxxxxx>
Date: Mon, 24 Feb 2014 23:27:35 -0600
Delivered-to: xfs@xxxxxxxxxxx
User-agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.9; rv:24.0) Gecko/20100101 Thunderbird/24.3.0
xfs_set_maxicount() overflowed fairly easily for large filesystems
and large maxicount; we started out by multiplying dblocks by
the percentage, *then* dividing by 100, and never checked for
an overflow.  The calculations were also, IMHO, a little hard
to follow.

I rewrote it using mult_frac (so handy!) and rounddown, to
do a nice no-overflow, no-precision-loss multiplication by
(%/100), then round down to the inode allocation unit.
Then check to see if we've overflowed the u64, and set to
XFS_MAXINUMBER if so.

Also, growfs open-coded the setting of maxicount; call the
handy helper instead.

This slightly changes growfs behavior, because we weren't
rounding to the allocation multiple in that code.  I checked
that we get the same answers for non-growfs cases by running
100 mkfs.xfs's with imaxpct from 1 to 100, and it gives the
same inode count in all cases.

Thanks to dchinner for pointing out that this could use fixing.

Signed-off-by: Eric Sandeen <sandeen@xxxxxxxxxx>
---

Ok Brian, have at it.  ;)


diff --git a/fs/xfs/xfs_fsops.c b/fs/xfs/xfs_fsops.c
index 02fb943..867e476 100644
--- a/fs/xfs/xfs_fsops.c
+++ b/fs/xfs/xfs_fsops.c
@@ -471,12 +471,7 @@ xfs_growfs_data_private(
        /* New allocation groups fully initialized, so update mount struct */
        if (nagimax)
                mp->m_maxagi = nagimax;
-       if (mp->m_sb.sb_imax_pct) {
-               __uint64_t icount = mp->m_sb.sb_dblocks * mp->m_sb.sb_imax_pct;
-               do_div(icount, 100);
-               mp->m_maxicount = icount << mp->m_sb.sb_inopblog;
-       } else
-               mp->m_maxicount = 0;
+       xfs_set_maxicount(mp);
        xfs_set_low_space_thresholds(mp);
 
        /* update secondary superblocks. */
diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
index 02df7b4..0ec2b19 100644
--- a/fs/xfs/xfs_mount.c
+++ b/fs/xfs/xfs_mount.c
@@ -419,24 +419,24 @@ xfs_update_alignment(xfs_mount_t *mp)
 }
 
 /*
- * Set the maximum inode count for this filesystem
+ * Set the maximum inode count for this filesystem based on sb_imax_pct
  */
-STATIC void
+void
 xfs_set_maxicount(xfs_mount_t *mp)
 {
        xfs_sb_t        *sbp = &(mp->m_sb);
-       __uint64_t      icount;
+       __uint64_t      iblocks;
 
        if (sbp->sb_imax_pct) {
                /*
                 * Make sure the maximum inode count is a multiple
                 * of the units we allocate inodes in.
                 */
-               icount = sbp->sb_dblocks * sbp->sb_imax_pct;
-               do_div(icount, 100);
-               do_div(icount, mp->m_ialloc_blks);
-               mp->m_maxicount = (icount * mp->m_ialloc_blks)  <<
-                                  sbp->sb_inopblog;
+               iblocks = mult_frac(sbp->sb_dblocks, sbp->sb_imax_pct, 100);
+               iblocks = rounddown(iblocks, mp->m_ialloc_blks);
+               mp->m_maxicount = iblocks << sbp->sb_inopblog;
+               if (mp->m_maxicount < iblocks)
+                       mp->m_maxicount = XFS_MAXINUMBER;
        } else {
                mp->m_maxicount = 0;
        }
diff --git a/fs/xfs/xfs_mount.h b/fs/xfs/xfs_mount.h
index a466c5e..6288a56 100644
--- a/fs/xfs/xfs_mount.h
+++ b/fs/xfs/xfs_mount.h
@@ -388,6 +388,7 @@ extern int  xfs_mod_incore_sb_batch(xfs_mount_t *, 
xfs_mod_sb_t *,
 extern int     xfs_mount_log_sb(xfs_mount_t *, __int64_t);
 extern struct xfs_buf *xfs_getsb(xfs_mount_t *, int);
 extern int     xfs_readsb(xfs_mount_t *, int);
+void           xfs_set_maxicount(xfs_mount_t *mp);
 extern void    xfs_freesb(xfs_mount_t *);
 extern int     xfs_fs_writable(xfs_mount_t *);
 extern int     xfs_sb_validate_fsb_count(struct xfs_sb *, __uint64_t);

<Prev in Thread] Current Thread [Next in Thread>