On Fri 17-08-12 09:04:00, Dave Chinner wrote:

On Thu, Aug 16, 2012 at 03:56:31PM +0200, Jan Kara wrote:

quoted

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.

quoted

--- 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.

quoted

+#
+# 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 [off-list ref]
SUSE Labs, CR

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