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

From: Ben Myers <bpm@sgi.com>
To: Dave Chinner <david@fromorbit.com>, Brian Foster <bfoster@redhat.com>
Cc: xfs@oss.sgi.com
Subject: Re: [PATCH v5 06/10] xfs: add XFS_IOC_FREE_EOFBLOCKS ioctl
Date: Wed, 17 Oct 2012 17:40:04 -0500	[thread overview]
Message-ID: <20121017224004.GG1377@sgi.com> (raw)
In-Reply-To: <20121016013901.GI2739@dastard>

Hi Fellas,

On Tue, Oct 16, 2012 at 12:39:01PM +1100, Dave Chinner wrote:
> On Tue, Oct 16, 2012 at 10:49:02AM +1100, Dave Chinner wrote:
> > On Mon, Oct 15, 2012 at 05:46:26PM -0500, Ben Myers wrote:
> > > Hey Brian,
> > > 
> > > On Thu, Oct 11, 2012 at 06:35:14PM -0400, Brian Foster wrote:
> > > > On 10/11/2012 10:13 AM, Ben Myers wrote:
> > > > > Hey Brian,
> > > > > 
> > > > > On Fri, Oct 05, 2012 at 10:17:12AM -0400, Brian Foster wrote:
> > > > >> The XFS_IOC_FREE_EOFBLOCKS ioctl allows users to invoke an EOFBLOCKS
> > > > >> scan. The xfs_eofblocks structure is defined to support the command
> > > > >> parameters (scan mode).
> > > > > 
> > > > > It would help to have an xfstest to exercise this ioctl to pull in with this
> > > > > series.  Do you have any code that could be wrangled into a test case?
> > > > > 
> > > > 
> > > > Yes, makes sense. I have some very basic test code I could put somewhere
> > > > to invoke the ioctl(). One of the questions I've been meaning to ask is
> > > > whether it would be relevant for that code to live in a common tool,
> > > > such as adding a new command to xfs_io. Then perhaps create an xfstests
> > > > test using that. Thoughts?
> > > 
> > > IMO you are right on the mark.  xfs_io is a great place for this.
> > > 
> > > > FYI, I have a few other things on my plate at the moment so
> > > > unfortunately it will be a bit before I can get back to XFS work... But
> > > > I'm fine with the set pending until I can come up with some test
> > > > coverage if that is preferable, of course.
> > > 
> > > I do think it is preferable to have a test case go in with the code where
> > > possible.  Since you don't mind waiting a bit, that seems to be the way to go.
> > > The other option could be to look for a volunteer to work on the test.  ;)
> > 
> > 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?

I think we can pull this in if Brian is willing post his test code.  Initially
it needn't be posted as an xfstest, that's fine.  As it stands today it appears
that Brian is the only one to ever use the ioctl and there is no way for anyone
else to test it.  Not an ideal situation.

I think this is a reasonable request.  Usually it's Christoph who asks for a
test case.  ;)

> i.e.:
> 
> $ for i in `seq 0 512`; do
> > xfs_io -f -c "pwrite $((i * 4096)) 4096" /mnt/scratch/foo
> > done
> $ stat -c %b /mnt/scratch/foo
> 8192
> $ sync; stat -c %b /mnt/scratch/foo
> 8192
> $ sleep 30; stat -c %b /mnt/scratch/foo
> 8192
> $ sleep 300; stat -c %b /mnt/scratch/foo
> 4104
> 
> It works. ;)

Nice!

Regards,
Ben

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs