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
|