[Top] [All Lists]

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

To: Brian Foster <bfoster@xxxxxxxxxx>
Subject: Re: [PATCH v5 06/10] xfs: add XFS_IOC_FREE_EOFBLOCKS ioctl
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Tue, 23 Oct 2012 09:22:28 +1100
Cc: Ben Myers <bpm@xxxxxxx>, xfs@xxxxxxxxxxx
In-reply-to: <508548BF.9090103@xxxxxxxxxx>
References: <1349446636-8611-7-git-send-email-bfoster@xxxxxxxxxx> <20121011141335.GY13214@xxxxxxx> <507749A2.4020206@xxxxxxxxxx> <20121015224626.GU24986@xxxxxxx> <20121015234902.GH2739@dastard> <20121016013901.GI2739@dastard> <20121017224004.GG1377@xxxxxxx> <507FF339.8020208@xxxxxxxxxx> <20121022073422.GC2739@dastard> <508548BF.9090103@xxxxxxxxxx>
User-agent: Mutt/1.5.21 (2010-09-15)
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....


Dave Chinner

<Prev in Thread] Current Thread [Next in Thread>