[PATCH] xfs: negative_icount.patch V2

[PATCH] xfs: negative_icount.patch V2

[Stuart Brodsky]

can somebody please review this for me?

Thanks,
------------

Because of delayed updates to sb_icount field in the super block, it is
possible to allocate over maxicount number of inodes.  This causes the
arithmetic to calculate a negative number of free inodes in user
commands like df or stat -f.  

Since maxicount is a somewhat arbitrary number, a slight over allocation
is not critical but user commands should be displayed as 0 or greater
and never go negative.  To do this the value in the stats buffer f_ffree
is capped to never go negative.

Signed-off-by: Stu Brodsky <sbrodsky@sgi.com>

--- a/fa/xfs/linux-2.6/xfs_super.c.orig 2010-07-20 06:06:19.269572013
-0500
+++ b/fa/xfs/linux-2.6/xfs_super.c 2010-07-20 08:09:17.773570840 -0500
@@ -1226,6 +1226,7 @@
        struct xfs_inode        *ip = XFS_I(dentry->d_inode);
        __uint64_t              fakeinos, id;
        xfs_extlen_t            lsize;
+       long long               i;
 
        statp->f_type = XFS_SB_MAGIC;
        statp->f_namelen = MAXNAMELEN - 1;
@@ -1249,7 +1250,12 @@
                statp->f_files = min_t(typeof(statp->f_files),
                                        statp->f_files,
                                        mp->m_maxicount);
-       statp->f_ffree = statp->f_files - (sbp->sb_icount -
sbp->sb_ifree);
+       i = statp->f_files - (sbp->sb_icount - sbp->sb_ifree);
+       if( i < 0 )
+               statp->f_ffree = 0;     /* cap at 0 */
+       else
+               statp->f_ffree = (__u64)i;
+
        spin_unlock(&mp->m_sb_lock);
 
        if ((ip->i_d.di_flags & XFS_DIFLAG_PROJINHERIT) ||
        
-- 
Stuart Brodsky
2750 Blue Water Road
Eagan, MN. 55121
651-683-7910
<sbrodsky@sgi.com>

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] xfs: negative_icount.patch V2

[Christoph Hellwig]

On Thu, Jul 29, 2010 at 03:18:38PM -0500, Stuart Brodsky wrote:
> can somebody please review this for me?

For a start it's still missing a useful subject line.

> +       long long               i;

Very bad naming of the variable, something like ffree would bebetter.
Also to fit with the rest of XFS it probably should be a __int64_t.

> +       i = statp->f_files - (sbp->sb_icount - sbp->sb_ifree);
> +       if( i < 0 )

wrong indentation.

> +               statp->f_ffree = 0;     /* cap at 0 */
> +       else
> +               statp->f_ffree = (__u64)i;

and it could probably done simpler as a

	statp->f_ffree = max_t(__int64_t, 0,
			statp->f_files + sbp->sb_ifree - sbp->sb_icount));

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs