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
|