Re: [PATCH] Check O_DIRECT support before testing direct I/O

Re: [PATCH] Check O_DIRECT support before testing direct I/O

[Dave Chinner]

On Tue, Dec 16, 2014 at 12:03:35AM +0100, Dushan Tcholich wrote:
> 
> In december 2013 Juhno Ryu posted a patch xfstests: check O_DIRECT 
> support before testing direct I/O to xfs@oss.sgi.com 
> http://oss.sgi.com/archives/xfs/2013-12/msg00759.html

At the time, xfstests was going through maintainer issues - SGI was
dropping the ball, and I didn't start to pick stuff up until mid
january 2014.

http://oss.sgi.com/archives/xfs/2014-01/msg00283.html

"Note that I didn't pull in the tmpfs support patch series this time
around as I'm not sure the discussions ever came to a conclusion
before xmas/new year/LCA intervened. That will need to be picked up
again..."

And there was no followup reposts of the patchset, so it's not gone
anywhere since.

> As there are no archives of fstests m-l I couldn't find why his 
> patches weren't applied so apologise if this is innapropriate but  
> I rebased his patch upon current xfstests code:

Tell me about it. If anyone knows the magic incantation to get the
usual mailing list archive sites to answer a simple "can you archive
this list" request, let me know, because I've tried repeatedly since
this list was created and haven't had a single reply from any of
them.

FYI, when send patches with comments like this, the comments should
be below the first "---" divider so that utilities like git am,
patch, etc strip the comments away. In this case, they strip this:

> ---
> 
> Some filesystems do not support O_DIRECT.  Check whether TEST_DIR supports 
> it by running xfs_io with and without -d flag.
> 
> Signed-off-by: Junho Ryu <jayr@google.com>
> Signed-off-by: Dushan Tcholich <dusanc@gmail.com>
> 
> ---

away. i.e. commit message first, commentary second. Also, this needs
a "From: Junho Ryu <jayr@google.com>" in the commit message.

> 
> 
> --- xfstests.orig/common/rc	2014-12-14 15:17:59.000000000 +0100
> +++ xfstests/common/rc	2014-12-15 19:40:36.000000000 +0100
> @@ -1391,6 +1395,7 @@
>          AIO_TEST=src/aio-dio-regress/$1
>          [ -x $AIO_TEST ] || _notrun "$AIO_TEST not built"
>      fi
> +    _require_odirect
>  }
>  
>  # run an aio-dio program
> @@ -1519,6 +1524,20 @@
>  		_notrun "xfs_io $command failed (old kernel/wrong fs?)"
>  }
>  
> +# check that kernel and filesystem support direct I/O
> +_require_odirect()
> +{
> +       testfile=$TEST_DIR/$$.direct
> +       $XFS_IO_PROG -F -f -c "pwrite 0 20k" $testfile 2>&1
> +       if [ $? -eq 0 ]; then
> +               $XFS_IO_PROG -F -f -d -c "pwrite 0 20k" $testfile 2>&1
> +               if [ $? -ne 0 ]; then
> +                       _notrun "O_DIRECT is not supported"
> +               fi
> +       fi

Why test for buffered IO, then test for direct IO?

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH] Check O_DIRECT support before testing direct I/O

[Theodore Ts'o]

On Tue, Dec 16, 2014 at 08:32:49AM +1100, Dave Chinner wrote:
> > As there are no archives of fstests m-l I couldn't find why his 
> > patches weren't applied so apologise if this is innapropriate but  
> > I rebased his patch upon current xfstests code:
> 
> Tell me about it. If anyone knows the magic incantation to get the
> usual mailing list archive sites to answer a simple "can you archive
> this list" request, let me know, because I've tried repeatedly since
> this list was created and haven't had a single reply from any of
> them.

I recently submitted a new list request to gmane, because of a similar
frustration, about a week ago.  Looks like they created the archive:

http://news.gmane.org/gmane.comp.file-systems.fstests

And even better, it looks like postmaster@vger.kernel.org even updated
for us.

http://vger.kernel.org/vger-lists.html#fstests

Gmane is sometimes a bit slow to update, so if someone can manage to
pursuade marc.info to also archive this list, that would be a good
thing.

					- Ted

[PATCH] Check O_DIRECT support before testing direct I/O

[Dushan Tcholich]

In december 2013 Juhno Ryu posted a patch xfstests: check O_DIRECT 
support before testing direct I/O to xfs@oss.sgi.com 
http://oss.sgi.com/archives/xfs/2013-12/msg00759.html

As there are no archives of fstests m-l I couldn't find why his 
patches weren't applied so apologise if this is innapropriate but  
I rebased his patch upon current xfstests code:

---

Some filesystems do not support O_DIRECT.  Check whether TEST_DIR supports 
it by running xfs_io with and without -d flag.

Signed-off-by: Junho Ryu <jayr@google.com>
Signed-off-by: Dushan Tcholich <dusanc@gmail.com>

---


--- xfstests.orig/common/rc	2014-12-14 15:17:59.000000000 +0100
+++ xfstests/common/rc	2014-12-15 19:40:36.000000000 +0100
@@ -1391,6 +1395,7 @@
         AIO_TEST=src/aio-dio-regress/$1
         [ -x $AIO_TEST ] || _notrun "$AIO_TEST not built"
     fi
+    _require_odirect
 }
 
 # run an aio-dio program
@@ -1519,6 +1524,20 @@
 		_notrun "xfs_io $command failed (old kernel/wrong fs?)"
 }
 
+# check that kernel and filesystem support direct I/O
+_require_odirect()
+{
+       testfile=$TEST_DIR/$$.direct
+       $XFS_IO_PROG -F -f -c "pwrite 0 20k" $testfile 2>&1
+       if [ $? -eq 0 ]; then
+               $XFS_IO_PROG -F -f -d -c "pwrite 0 20k" $testfile 2>&1
+               if [ $? -ne 0 ]; then
+                       _notrun "O_DIRECT is not supported"
+               fi
+       fi
+       rm -f $testfile 2>&1 > /dev/null
+}
+
 # Check that a fs has enough free space (in 1024b blocks)
 #
 _require_fs_space()

diff -urN xfstests.orig/tests/generic/091 xfstests/tests/generic/091
--- xfstests.orig/tests/generic/091	2014-12-14 15:18:00.000000000 +0100
+++ xfstests/tests/generic/091	2014-12-14 13:25:10.000000000 +0100
@@ -39,6 +39,7 @@
 _supported_fs generic
 _supported_os Linux
 _require_test
+_require_odirect
 
 rm -f $seqres.full

diff -urN xfstests.orig/tests/generic/130 xfstests/tests/generic/130
--- xfstests.orig/tests/generic/130	2014-12-14 15:18:00.000000000 +0100
+++ xfstests/tests/generic/130	2014-12-14 13:24:50.000000000 +0100
@@ -50,6 +50,7 @@
 
 _require_scratch
 _require_sparse_files
+_require_odirect
 
 _scratch_mkfs >/dev/null 2>&1
 _scratch_mount
diff -urN xfstests.orig/tests/generic/133 xfstests/tests/generic/133
--- xfstests.orig/tests/generic/133	2014-12-14 15:18:00.000000000 +0100
+++ xfstests/tests/generic/133	2014-12-14 13:25:43.000000000 +0100
@@ -39,6 +39,7 @@
 _supported_fs generic
 _supported_os Linux IRIX
 _require_test
+_require_odirect
 
 echo "Buffered writer, buffered reader"
 $XFS_IO_PROG -f -d -c 'pwrite -b 64k 0 512m' $TEST_DIR/io_test > /dev/null
diff -urN xfstests.orig/tests/generic/135 xfstests/tests/generic/135
--- xfstests.orig/tests/generic/135	2014-12-14 15:18:00.000000000 +0100
+++ xfstests/tests/generic/135	2014-12-14 13:22:52.000000000 +0100
@@ -39,6 +39,7 @@
 _supported_fs generic
 _supported_os Linux IRIX
 
+_require_odirect
 _require_scratch
 _scratch_mkfs >/dev/null 2>&1
 
diff -urN xfstests.orig/tests/generic/226 xfstests/tests/generic/226
--- xfstests.orig/tests/generic/226	2014-12-14 15:18:00.000000000 +0100
+++ xfstests/tests/generic/226	2014-12-14 13:22:52.000000000 +0100
@@ -37,6 +37,7 @@
 _supported_fs generic
 _supported_os Linux IRIX
 _require_scratch
+_require_odirect
 
 # real QA test starts here
 rm -f $seqres.full
diff -urN xfstests.orig/tests/generic/263 xfstests/tests/generic/263
--- xfstests.orig/tests/generic/263	2014-12-14 15:18:00.000000000 +0100
+++ xfstests/tests/generic/263	2014-12-14 13:26:23.000000000 +0100
@@ -39,6 +39,7 @@
 _supported_fs generic
 _supported_os Linux
 _require_test
+_require_odirect
 
 rm -f $seqres.full
 
diff -urN xfstests.orig/tests/generic/299 xfstests/tests/generic/299
--- xfstests.orig/tests/generic/299	2014-12-14 15:18:00.000000000 +0100
+++ xfstests/tests/generic/299	2014-12-14 13:22:52.000000000 +0100
@@ -43,6 +43,7 @@
 _supported_os Linux
 _need_to_be_root
 _require_scratch
+_require_odirect
 
 NUM_JOBS=$((4*LOAD_FACTOR))
 BLK_DEV_SIZE=`blockdev --getsz $SCRATCH_DEV`
diff -urN xfstests.orig/tests/generic/300 xfstests/tests/generic/300
--- xfstests.orig/tests/generic/300	2014-12-14 15:18:00.000000000 +0100
+++ xfstests/tests/generic/300	2014-12-14 13:22:52.000000000 +0100
@@ -43,6 +43,7 @@
 _supported_os Linux
 _need_to_be_root
 _require_scratch
+_require_odirect
 
 # xfs_io is not required for this test, but it's the best way to verify
 # the test system supports fallocate() for allocation and hole punching

Re: [PATCH] Check O_DIRECT support before testing direct I/O

[Dave Chinner]

On Tue, Dec 16, 2014 at 12:03:35AM +0100, Dushan Tcholich wrote:
> 
> In december 2013 Juhno Ryu posted a patch xfstests: check O_DIRECT 
> support before testing direct I/O to xfs@oss.sgi.com 
> http://oss.sgi.com/archives/xfs/2013-12/msg00759.html
> 
> As there are no archives of fstests m-l I couldn't find why his 
> patches weren't applied so apologise if this is innapropriate but  
> I rebased his patch upon current xfstests code:
> 
> ---
> 
> Some filesystems do not support O_DIRECT.  Check whether TEST_DIR supports 
> it by running xfs_io with and without -d flag.
> 
> Signed-off-by: Junho Ryu <jayr@google.com>
> Signed-off-by: Dushan Tcholich <dusanc@gmail.com>
> 
> ---
> 
> 
> --- xfstests.orig/common/rc	2014-12-14 15:17:59.000000000 +0100
> +++ xfstests/common/rc	2014-12-15 19:40:36.000000000 +0100
> @@ -1391,6 +1395,7 @@
>          AIO_TEST=src/aio-dio-regress/$1
>          [ -x $AIO_TEST ] || _notrun "$AIO_TEST not built"
>      fi
> +    _require_odirect
>  }
>  
>  # run an aio-dio program
> @@ -1519,6 +1524,20 @@
>  		_notrun "xfs_io $command failed (old kernel/wrong fs?)"
>  }
>  
> +# check that kernel and filesystem support direct I/O
> +_require_odirect()
> +{
> +       testfile=$TEST_DIR/$$.direct
> +       $XFS_IO_PROG -F -f -c "pwrite 0 20k" $testfile 2>&1
> +       if [ $? -eq 0 ]; then
> +               $XFS_IO_PROG -F -f -d -c "pwrite 0 20k" $testfile 2>&1
> +               if [ $? -ne 0 ]; then
> +                       _notrun "O_DIRECT is not supported"
> +               fi
> +       fi
> +       rm -f $testfile 2>&1 > /dev/null
> +}

FWIW, I'd appreciate it if people tested their patches to ensure
they don't cause test failures before posting them. This causes
every test that _requires_odirect to fail because the xfs_io output
is not directed to /dev/null:

generic/036 11s ... - output mismatch (see /home/dave/src/xfstests-dev/results//generic/036.out.bad)
    --- tests/generic/036.out   2014-11-13 16:14:32.000000000 +1100
    +++ /home/dave/src/xfstests-dev/results//generic/036.out.bad        2014-12-16 12:46:53.000000000 +1100
    @@ -1,2 +1,4 @@
     QA output created by 036
    +wrote 20480/20480 bytes at offset 0
    +20 KiB, 5 ops; 0.0000 sec (3.896 MiB/sec and 997.4067 ops/sec)
     All tasks are spawned
    ...
    (Run 'diff -u tests/generic/036.out /home/dave/src/xfstests-dev/results//generic/036.out.bad'  to see the entire diff)

I've fixed this in v2 of the patch....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH] Check O_DIRECT support before testing direct I/O

[Dušan Čolić]

On Tue, Dec 16, 2014 at 3:00 AM, Dave Chinner <david@fromorbit.com> wrote:
> On Tue, Dec 16, 2014 at 12:03:35AM +0100, Dushan Tcholich wrote:
>>
>> In december 2013 Juhno Ryu posted a patch xfstests: check O_DIRECT
>> support before testing direct I/O to xfs@oss.sgi.com
>> http://oss.sgi.com/archives/xfs/2013-12/msg00759.html
>>
>> As there are no archives of fstests m-l I couldn't find why his
>> patches weren't applied so apologise if this is innapropriate but
>> I rebased his patch upon current xfstests code:
>>
>> ---
>>
>> Some filesystems do not support O_DIRECT.  Check whether TEST_DIR supports
>> it by running xfs_io with and without -d flag.
>>
>> Signed-off-by: Junho Ryu <jayr@google.com>
>> Signed-off-by: Dushan Tcholich <dusanc@gmail.com>
>>
>> ---
>>
>>
>> --- xfstests.orig/common/rc   2014-12-14 15:17:59.000000000 +0100
>> +++ xfstests/common/rc        2014-12-15 19:40:36.000000000 +0100
>> @@ -1391,6 +1395,7 @@
>>          AIO_TEST=src/aio-dio-regress/$1
>>          [ -x $AIO_TEST ] || _notrun "$AIO_TEST not built"
>>      fi
>> +    _require_odirect
>>  }
>>
>>  # run an aio-dio program
>> @@ -1519,6 +1524,20 @@
>>               _notrun "xfs_io $command failed (old kernel/wrong fs?)"
>>  }
>>
>> +# check that kernel and filesystem support direct I/O
>> +_require_odirect()
>> +{
>> +       testfile=$TEST_DIR/$$.direct
>> +       $XFS_IO_PROG -F -f -c "pwrite 0 20k" $testfile 2>&1
>> +       if [ $? -eq 0 ]; then
>> +               $XFS_IO_PROG -F -f -d -c "pwrite 0 20k" $testfile 2>&1
>> +               if [ $? -ne 0 ]; then
>> +                       _notrun "O_DIRECT is not supported"
>> +               fi
>> +       fi
>> +       rm -f $testfile 2>&1 > /dev/null
>> +}
>
> FWIW, I'd appreciate it if people tested their patches to ensure
> they don't cause test failures before posting them. This causes
> every test that _requires_odirect to fail because the xfs_io output
> is not directed to /dev/null:
>
Happily sending v2 patch ASAP is not as important as checking if patch
really works in all test cases, who would've thought.
My xfstests one patch at a time world domination plans crumbled to the dust :D

I'm grateful for your time and effort to teach me something I
should've thought myself.
Lesson learned.

Thanks
Dushan

> generic/036 11s ... - output mismatch (see /home/dave/src/xfstests-dev/results//generic/036.out.bad)
>     --- tests/generic/036.out   2014-11-13 16:14:32.000000000 +1100
>     +++ /home/dave/src/xfstests-dev/results//generic/036.out.bad        2014-12-16 12:46:53.000000000 +1100
>     @@ -1,2 +1,4 @@
>      QA output created by 036
>     +wrote 20480/20480 bytes at offset 0
>     +20 KiB, 5 ops; 0.0000 sec (3.896 MiB/sec and 997.4067 ops/sec)
>      All tasks are spawned
>     ...
>     (Run 'diff -u tests/generic/036.out /home/dave/src/xfstests-dev/results//generic/036.out.bad'  to see the entire diff)
>
> I've fixed this in v2 of the patch....
>
> Cheers,
>
> Dave.
> --
> Dave Chinner
> david@fromorbit.com