Re: [PATCH] bulkstat fixups

[David Chinner <dgc@sgi.com>]

On Fri, Nov 16, 2007 at 03:34:40PM +1100, Lachlan McIlroy wrote:
> Updated patch - I added cond_resched() calls into each loop - for loops that
> have a 'continue' somewhere in them I added the cond_resched() at the start,
> otherwise I put it at the end.

You probably don't need the call in the innermost loop (the walking across
the inode cluster).

> >>>Userspace visile change. What applications do we have that rely on this
> >>>behaviour that will be broken by this change?
> >>Any apps that rely on the existing behaviour are probably broken.  If an 
> >>app
> >>wants to call xfs_bulkstat_single() it should use 
> >>XFS_IOC_FSBULKSTAT_SINGLE.
> >
> >Perhaps, but we can't arbitrarily decide that those apps will now break on
> >a new kernel with this change. At minimum we need to audit all of the code
> >we have that uses bulkstat for such breakage (including DMF!) before we 
> >make a
> >change like this.
> 
> I've looked through everything we have in xfs-cmds and nothing relies on
> this bug being present.  Vlad helped me with the DMF side - DMF does not
> use the XFS_IOC_FSBULKSTAT ioctl, it has it's own interface into the kernel
> which calls xfs_bulkstat() directly so it wont be affected by this change.

Sounds like it really is a bug as nothing is trying to exploit that
behaviour. Ok, seems fair to fix it.

Cheers,

Dave.
-- 
Dave Chinner
Principal Engineer
SGI Australian Software Group