Re: [PATCH v5 06/10] xfs: add XFS_IOC_FREE_EOFBLOCKS ioctl

Dave Chinner
2012-10-22T22:21:05Z

On Mon, Oct 22, 2012 at 09:23:11AM -0400, Brian Foster wrote:
> On 10/22/2012 03:34 AM, Dave Chinner wrote:
> > On Thu, Oct 18, 2012 at 08:16:57AM -0400, Brian Foster wrote:
> >> On 10/17/2012 06:40 PM, Ben Myers wrote:
> >>>>> FWIW, given the background cleanup code can be trivially verified to
> >>>>> work (open, apend, close, repeat, wait 5 minutes) and is the
> >>>>> functionality that is needed in mainline, having something to test
> >>>>> the ioctls should not stop the patchset from being merged.
> >>>
> >>> Can we be assured that we'll get an xfstest for it eventually?
> >>
> >> Absolutely. Getting a command into xfs_io to support such a test is now
> >> the top of my todo list with regard to XFS. :)
> >
> > Here's a patch to the new xfs_spaceman program I'm writing that adds
> > control for these ioctls.
> >
>
> Very cool, thanks. Catchy name for the tool as well, btw ;).
>
> For some reason my mailer is stripping out the patch,

Probably because I simply added it inline below my sig. The mailer
is probably dropping everything below the /^-- $/ line that marks
the sig...
>
> but my only
> comment is with regard to minlen. Shouldn't that variable be handled as
> an unsigned?

Probably, but a ssize_t would be better, and ....

> Now that I think of it, that makes me wonder if I should
> make that a 64-bit unsigned in xfs_eofblocks..?

... yes, a __u64 would be better.

I've got a couple more comments now I've actually used and tested
it, so I'll do that today....

Cheers,

Dave.
--
Dave Chinner
david@fromorbit.com