On Fri 17-08-12 09:04:00, Dave Chinner wrote:
> On Thu, Aug 16, 2012 at 03:56:31PM +0200, Jan Kara wrote:
> > ext3 in data=journal mode does not support direct IO. Tests which use
> > direct IO fail due to that. Provide function checking whether direct IO
> > is supported and skip tests needing direct IO if it's not.
> >
> > There are a few tests which use direct IO but would be meaningful even
> > without it since they test several different things. Making these tests
> > useful for filesystems without dio support is left for future if somebody
> > is interested...
>
> So this is just for the generic tests? There's a lot more XFS
> specific tests that require direct IO that aren't in this patch. ;)
Right but I was a lazy bastard and went just through the tests that
failed for me, checked that they failed due to direct IO, and added the
requirement. I can go through the XFS specific tests and add the
requirement but frankly I find a little use in that.
> Also, I suspect that you've missed all the tests that run fsstress,
> because that uses direct IO as well. There's probably others as
> well. No doub they didn't produce test failures, but it's entirely
> possible that they are not testing what they are supposed to be
> testing as a result of direct IO failing silently...
It's not failing silently. For ext3 we return EINVAL from open so I'd
expect reasonably written tests to complain.
> > --- a/198
> > +++ b/198
> > @@ -44,6 +44,7 @@ _cleanup()
> >
> > _supported_fs generic
> > _supported_os Linux
> > +_require_direct_io
> > _require_aiodio aiodio_sparse2
>
> For all the tests are already call _require_aiodio, just embed the
> _require_dio test inside that one.
OK, will do.
> > +#
> > +# Check if the filesystem supports direct IO
> > +#
> > +_require_direct_io()
> > +{
> > + testfile=$TEST_DIR/$$.dio
> > + testio=`$XFS_IO_PROG -F -f -d -c "" $testfile 2>&1`
>
> This assumes that both the test device and the scratch device are
> both using the same mount options, right?
>
> Some tests use the scratch device with different mount options, so
> may actually allow direct IO to work even though the test device
> fails. I haven't looked at whether any of the tests in this patch do
> that, but if they do then you might also need a _require_scratch_dio
> function for those tests....
I went through all the changed tests and none of them seem to play tricks
with mount options so _require_direct_io should be fine for all of them.
Thanks for review, I'll send a new version of the patch after a test run
finishes.
Honza
--
Jan Kara <jack@xxxxxxx>
SUSE Labs, CR
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
