On 2/24/14, 5:56 PM, Dave Chinner wrote: > On Mon, Feb 24, 2014 at 05:10:31PM -0600, Eric Sandeen wrote: >> Because we have lazy counters, it's possible that we over-allocate >> inodes past the maxicount (imaxpct) limit. >> >> A previous commit, >> >> 2fe3366 xfs: ensure f_ffree returned by statfs() is non-negative >> >> stopped statfs from underflowing f_ffree in this case, but that >> only happened when we mis-reported f_files, capped at maxicount. >> >> Change statfs to report the actual number of inodes allocated, >> even if it is greater than maxicount. It's reality. >> Deal with it. >> >> (New clearer code flow thanks to Brian!) >> >> Logic-made-readable-by: Brian Foster <bfoster@xxxxxxxxxx> >> Signed-off-by: Eric Sandeen <sandeen@xxxxxxxxxx> >> --- >> >> V2: Use Brian's suggested logic for working out the numbers >> >> diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c >> index f317488..0dbcc17 100644 >> --- a/fs/xfs/xfs_super.c >> +++ b/fs/xfs/xfs_super.c >> @@ -1083,7 +1083,6 @@ xfs_fs_statfs( >> struct xfs_inode *ip = XFS_I(dentry->d_inode); >> __uint64_t fakeinos, id; >> xfs_extlen_t lsize; >> - __int64_t ffree; >> >> statp->f_type = XFS_SB_MAGIC; >> statp->f_namelen = MAXNAMELEN - 1; >> @@ -1100,17 +1099,19 @@ xfs_fs_statfs( >> statp->f_blocks = sbp->sb_dblocks - lsize; >> statp->f_bfree = statp->f_bavail = >> sbp->sb_fdblocks - XFS_ALLOC_SET_ASIDE(mp); >> + /* >> + * Potential number of new inodes in free blocks, limited by maxicount. >> + */ >> fakeinos = statp->f_bfree << sbp->sb_inopblog; > > Can we rename "fakeinos" to something like "free_inodes" so that > the code reads a little bit better while we are touching this > code? yeah, I thought about that too. >> - statp->f_files = >> - MIN(sbp->sb_icount + fakeinos, (__uint64_t)XFS_MAXINUMBER); >> if (mp->m_maxicount) >> - statp->f_files = min_t(typeof(statp->f_files), >> - statp->f_files, >> - mp->m_maxicount); >> + fakeinos = mp->m_maxicount > sbp->sb_icount ? >> + MIN(mp->m_maxicount - sbp->sb_icount, fakeinos) : 0; > > Get rid of MIN - it should be min() or min_t(). they are the same types, but yeah, that's better. > Also the mix of if() and ternary operations makes this difficult to > follow the logic. Better, IMO, is this: > > free_inodes = statp->f_bfree << sbp->sb_inopblog; > if (mp->m_maxicount > sbp->sb_icount) > free_inodes = min(mp->m_maxicount - sbp->sb_icount, > free_inodes); > else if (mp->m_maxicount) > free_inodes = 0; ok good point. > >> + >> + /* Total possible files is current inodes + potential new inodes */ >> + statp->f_files = MIN(sbp->sb_icount + fakeinos, >> + (__uint64_t) XFS_MAXINUMBER); > > statp->f_files = min_t(u64, sbp->sb_icount + free_inodes, > XFS_MAXINUMBER); > > And for bonus points: while we are looking at maxicount, the setting > on maxicount in the growfs code should call xfs_set_maxicount() > rather than open coding it, and xfs_set_maxicount() needs to be > reworked to prevent overflow when sbp->sb_dblocks * sbp->sb_imax_pct > is greater than 64 bits.... Ok. Well, that's a different patch I think. I'll also have to invent another tag for you, Dave. Nitpicked-by: perhaps. ;) (I kid! I kid!) -Eric > Cheers, > > Dave. > _______________________________________________ xfs mailing list xfs@xxxxxxxxxxx http://oss.sgi.com/mailman/listinfo/xfs
