[PATCH] common: ext4's data=journal mode doesn't support O_DIRECT

[PATCH] common: ext4's data=journal mode doesn't support O_DIRECT

[Theodore Ts'o]

Teach _require_odirect that ext4's data=journal mode doesn't support
O_DIRECT.

Signed-off-by: Theodore Ts'o <tytso@mit.edu>
---
 common/rc | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/common/rc b/common/rc
index 069df58..791e6e5 100644
--- a/common/rc
+++ b/common/rc
@@ -1932,9 +1932,13 @@ _require_xfs_db_command()
 # check that kernel and filesystem support direct I/O
 _require_odirect()
 {
-       if [ $FSTYP = "ext4" ] &&
-	  echo "$MOUNT_OPTIONS" | grep -q "test_dummy_encryption" ; then
-		_notrun "ext4 encryption doesn't support O_DIRECT"
+       if [ $FSTYP = "ext4" ] ; then
+           if echo "$MOUNT_OPTIONS" | grep -q "test_dummy_encryption" ; then
+	       _notrun "ext4 encryption doesn't support O_DIRECT"
+	   fi
+           if echo "$MOUNT_OPTIONS" | grep -q "data=journal" ; then
+	       _notrun "ext4 data=journal mode doesn't support O_DIRECT"
+	   fi
        fi
        testfile=$TEST_DIR/$$.direct
        $XFS_IO_PROG -F -f -d -c "pwrite 0 20k" $testfile > /dev/null 2>&1
-- 
2.5.0

Re: [PATCH] common: ext4's data=journal mode doesn't support O_DIRECT

[Eryu Guan]

On Thu, Jun 09, 2016 at 02:42:42PM -0400, Theodore Ts'o wrote:
> Teach _require_odirect that ext4's data=journal mode doesn't support
> O_DIRECT.
> 
> Signed-off-by: Theodore Ts'o <tytso@mit.edu>
> ---
>  common/rc | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/common/rc b/common/rc
> index 069df58..791e6e5 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -1932,9 +1932,13 @@ _require_xfs_db_command()
>  # check that kernel and filesystem support direct I/O
>  _require_odirect()
>  {
> -       if [ $FSTYP = "ext4" ] &&
> -	  echo "$MOUNT_OPTIONS" | grep -q "test_dummy_encryption" ; then
> -		_notrun "ext4 encryption doesn't support O_DIRECT"
> +       if [ $FSTYP = "ext4" ] ; then
> +           if echo "$MOUNT_OPTIONS" | grep -q "test_dummy_encryption" ; then
> +	       _notrun "ext4 encryption doesn't support O_DIRECT"
> +	   fi
> +           if echo "$MOUNT_OPTIONS" | grep -q "data=journal" ; then
> +	       _notrun "ext4 data=journal mode doesn't support O_DIRECT"
> +	   fi

This hunk doesn't apply, there's no detection code for ext4 encryption
in current master. And do we need to filter out ext3 journal mode as
well?

And this patch mixed space and tab for indention.

Just curious, what's the problem running direct I/O tests on journal
mode ext4? ext4 falls back to buffered I/O in this case and I don't see
any test failures caused by it. Perhaps it'd be better to add this
information to commit log too.

Thanks,
Eryu

>         fi
>         testfile=$TEST_DIR/$$.direct
>         $XFS_IO_PROG -F -f -d -c "pwrite 0 20k" $testfile > /dev/null 2>&1
> -- 
> 2.5.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe fstests" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] common: ext4's data=journal mode doesn't support O_DIRECT

[Theodore Ts'o]

On Sat, Jun 11, 2016 at 08:20:26PM +0800, Eryu Guan wrote:
> 
> This hunk doesn't apply, there's no detection code for ext4 encryption
> in current master. And do we need to filter out ext3 journal mode as
> well?

Oops, there's another patch this depends upon that I forgot to send
out this time around.  Let me fix up the spaces and tabs, and probably
just combine the two patches.

> Just curious, what's the problem running direct I/O tests on journal
> mode ext4? ext4 falls back to buffered I/O in this case and I don't see
> any test failures caused by it. Perhaps it'd be better to add this
> information to commit log too.

I'll double check but I think there was at least one dmflaky that
failed (or maybe it was just flaky) because the DIO write wasn't
really DIO, and if the device went read-only too early, the data write
wouldn't make it to the disk, and this caused the test failure.

More generally, if a particular file system mode doesn't support
Direct I/O, even if it doesn't cause a test failure, it's likely a
waste of test resources to run a test which expected that writes be
DIO when it really isn't.

					- Ted