xfs
[Top] [All Lists]

Re: [PATCH] xfstests: fsync-tester: fix pwrite() return check and disabl

To: Josef Bacik <jbacik@xxxxxxxxxxxx>
Subject: Re: [PATCH] xfstests: fsync-tester: fix pwrite() return check and disable direct for test 19
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Wed, 12 Jun 2013 12:48:09 +1000
Cc: xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <20130612023515.GV29338@dastard>
References: <1370999583-16725-1-git-send-email-jbacik@xxxxxxxxxxxx> <20130612023515.GV29338@dastard>
User-agent: Mutt/1.5.21 (2010-09-15)
On Wed, Jun 12, 2013 at 12:35:15PM +1000, Dave Chinner wrote:
> On Tue, Jun 11, 2013 at 09:13:03PM -0400, Josef Bacik wrote:
> > Test 19 in direct mode was failing on xfs because it was not actually doing 
> > the
> > write because the writes were not sectorsize aligned.  This test is to test
> > btrfs's inline extent fsync()ing so the writes won't be sectorsize aligned, 
> > and
> > inline extents will fall back to buffered anyway so direct mode is 
> > meaningless
> > for this test.  So just check if we are test 19 and disable direct mode so 
> > we
> > don't have to change the golden output.  Also change test_five() to compare
> > against a ssize_t instead of a size_t since apparently comparing against 
> > size_t
> > makes it cast the return value of pwrite() to size_t which screws up the 
> > error
> > case, so instead of seeing the pwrite() error on xfs which would have 
> > explained
> > this all it appeared as if it was succeeding and screwing up the fsync(), 
> > which
> > unfortunately wasted a bit of Daves time.  This patch should fix all this 
> > up.
> 
> You need to breathe when you type, Josef. ;)
> 
> > Thanks,
> > 
> > Signed-off-by: Josef Bacik <jbacik@xxxxxxxxxxxx>
> > ---
> >  src/fsync-tester.c |   10 +++++++++-
> >  1 files changed, 9 insertions(+), 1 deletions(-)
> > 
> > diff --git a/src/fsync-tester.c b/src/fsync-tester.c
> > index f0875fc..1c03ed0 100644
> > --- a/src/fsync-tester.c
> > +++ b/src/fsync-tester.c
> > @@ -230,7 +230,7 @@ static int test_five()
> >  
> >     memset(buf, character, 3072);
> >     for (i = 0; i < runs; i++) {
> > -           size_t write_size = (random() % 3072) + 1;
> > +           ssize_t write_size = (random() % 3072) + 1;
> >  
> >             if (pwrite(test_fd, buf, write_size, 0) < write_size) {
> >                     fprintf(stderr, "Short write %d\n", errno);
> > @@ -419,6 +419,14 @@ int main(int argc, char **argv)
> >     if (optind >= argc)
> >             usage();
> >  
> > +   /*
> > +    * test 19 is for smaller than blocksize writes to test btrfs's inline
> > +    * extent fsyncing, so direct_io doesn't make sense and in fact doesn't
> > +    * work for other file systems, so just disable direct io for this test.
> > +    */
> > +   if (test == 19)
> > +           direct_io = 0;
> > +
> 
> Yup, looks good. Passes for me now ;)

And I forgot:

Reviewed-by: Dave Chinner <dchinner@xxxxxxxxxx>

-- 
Dave Chinner
david@xxxxxxxxxxxxx

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