2012-08-16 09:28:10

by Jan Kara

[permalink] [raw]
Subject: Test 272 fails for ext3

Hello,

I was looking into why test 272 from xfstests fails for ext3 and the
reason is that ext3 does not support direct IO to file with enabled data
journalling (open returns EINVAL because ->direct_IO callback is not
specified). So I was thinking how to accomodate this fact in the test -
the best I found was to just check using xfs_io whether O_DIRECT open
succeeds and perform the test only in that case. Attached patch does this
or do people have other ideas?

Honza
--
Jan Kara <[email protected]>
SUSE Labs, CR


Attachments:
(No filename) (527.00 B)
0001-Make-test-272-work-for-ext3.patch (999.00 B)
Download all attachments

2012-08-16 09:37:59

by Jan Kara

[permalink] [raw]
Subject: Test 272 fails for ext3

Hello,

I was looking into why test 272 from xfstests fails for ext3 and the
reason is that ext3 does not support direct IO to file with enabled data
journalling (open returns EINVAL because ->direct_IO callback is not
specified). So I was thinking how to accomodate this fact in the test -
the best I found was to just check using xfs_io whether O_DIRECT open
succeeds and perform the test only in that case. Attached patch does this
or do people have other ideas?

Honza

PS: Sending again with correct list address. I'm sorry for unnecessary email.
--
Jan Kara <[email protected]>
SUSE Labs, CR


Attachments:
(No filename) (606.00 B)
0001-Make-test-272-work-for-ext3.patch (999.00 B)
Download all attachments

2012-08-16 10:07:16

by Dmitry Monakhov

[permalink] [raw]
Subject: Re: Test 272 fails for ext3

On Thu, 16 Aug 2012 11:37:57 +0200, Jan Kara <[email protected]> wrote:
Non-text part: multipart/mixed
> Hello,
>
> I was looking into why test 272 from xfstests fails for ext3 and the
> reason is that ext3 does not support direct IO to file with enabled data
> journalling (open returns EINVAL because ->direct_IO callback is not
> specified). So I was thinking how to accomodate this fact in the test -
> the best I found was to just check using xfs_io whether O_DIRECT open
> succeeds and perform the test only in that case. Attached patch does this
> or do people have other ideas?
Looks reasonable
ACK.
>
> Honza
>
> PS: Sending again with correct list address. I'm sorry for unnecessary email.
> --
> Jan Kara <[email protected]>
> SUSE Labs, CR
> From 529429c8497314ef956e470fdb9b94bfe797df69 Mon Sep 17 00:00:00 2001
> From: Jan Kara <[email protected]>
> Date: Thu, 16 Aug 2012 11:14:35 +0200
> Subject: [PATCH] Make test 272 work for ext3
>
> ext3 does not support direct IO for files with data journalling. This
> confuses test 272. Make the test check whether open succeeds and perform
> the writing only if it does.
>
> Signed-off-by: Jan Kara <[email protected]>
> ---
> 272 | 6 ++++++
> 1 files changed, 6 insertions(+), 0 deletions(-)
>
> diff --git a/272 b/272
> index 26dfa3b..e39c52b 100755
> --- a/272
> +++ b/272
> @@ -52,6 +52,12 @@ _workout()
>
> echo "OP write_opt: $write_opt 4M, \
> chattr_opt: $chattr_opt"
> + if [ "$write_opt" = "oflag=direct" ]; then
> + # Some filesystems don't support direct IO
> + # in some cases, check for that
> + xfs_io -F -d -f -c "" $SCRATCH_MNT/file.$idx \
> + >> $seq.full 2>&1 || continue
> + fi
> dd if=/dev/zero of=$SCRATCH_MNT/file.$idx \
> bs=1M count=4 $write_opt \
> >> $seq.full 2>&1 || exit
> --
> 1.7.1
>

2012-08-16 22:48:58

by Dave Chinner

[permalink] [raw]
Subject: Re: Test 272 fails for ext3

On Thu, Aug 16, 2012 at 11:37:57AM +0200, Jan Kara wrote:
> Hello,
>
> I was looking into why test 272 from xfstests fails for ext3 and the
> reason is that ext3 does not support direct IO to file with enabled data
> journalling (open returns EINVAL because ->direct_IO callback is not
> specified). So I was thinking how to accomodate this fact in the test -
> the best I found was to just check using xfs_io whether O_DIRECT open
> succeeds and perform the test only in that case. Attached patch does this
> or do people have other ideas?


# ext3 doesn't support direct IO in journalling mode
ext3_write_opt_list="iflag=noatime conv=notrunc conv=fsync"
ext4_write_opt_list="iflag=noatime conv=notrunc conv=fsync oflag=direct"

if [ $FSTYP = "ext3" ]; then
write_opt_list="$ext3_write_opt_list"
else
write_opt_list="$ext4_write_opt_list"
fi

.....
> diff --git a/272 b/272
> index 26dfa3b..e39c52b 100755
> --- a/272
> +++ b/272
> @@ -52,6 +52,12 @@ _workout()
>
> echo "OP write_opt: $write_opt 4M, \
> chattr_opt: $chattr_opt"
> + if [ "$write_opt" = "oflag=direct" ]; then
> + # Some filesystems don't support direct IO
> + # in some cases, check for that
> + xfs_io -F -d -f -c "" $SCRATCH_MNT/file.$idx \
> + >> $seq.full 2>&1 || continue
> + fi

That only fixes one of the two places that does direct IO on a
jounralled file. The above will fix both...

Cheers,

Dave.
--
Dave Chinner
[email protected]

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

2012-08-20 16:22:15

by Jan Kara

[permalink] [raw]
Subject: Re: Test 272 fails for ext3

On Fri 17-08-12 08:48:58, Dave Chinner wrote:
> On Thu, Aug 16, 2012 at 11:37:57AM +0200, Jan Kara wrote:
> > Hello,
> >
> > I was looking into why test 272 from xfstests fails for ext3 and the
> > reason is that ext3 does not support direct IO to file with enabled data
> > journalling (open returns EINVAL because ->direct_IO callback is not
> > specified). So I was thinking how to accomodate this fact in the test -
> > the best I found was to just check using xfs_io whether O_DIRECT open
> > succeeds and perform the test only in that case. Attached patch does this
> > or do people have other ideas?
>
>
> # ext3 doesn't support direct IO in journalling mode
> ext3_write_opt_list="iflag=noatime conv=notrunc conv=fsync"
> ext4_write_opt_list="iflag=noatime conv=notrunc conv=fsync oflag=direct"
>
> if [ $FSTYP = "ext3" ]; then
> write_opt_list="$ext3_write_opt_list"
> else
> write_opt_list="$ext4_write_opt_list"
> fi
Yeah, this is probably simpler. Thanks for suggestion. BTW, ext4 also
does not support direct IO and data journalling but it silently falls back
to buffered IO. Anyway new patch is attached.

Honza
--
Jan Kara <[email protected]>
SUSE Labs, CR


Attachments:
(No filename) (1.16 kB)
0001-Make-test-272-work-for-ext3.patch (1.22 kB)
Download all attachments

2012-08-20 21:06:41

by Jan Kara

[permalink] [raw]
Subject: Re: Test 272 fails for ext3

On Mon 20-08-12 18:22:12, Jan Kara wrote:
> On Fri 17-08-12 08:48:58, Dave Chinner wrote:
> > On Thu, Aug 16, 2012 at 11:37:57AM +0200, Jan Kara wrote:
> > > Hello,
> > >
> > > I was looking into why test 272 from xfstests fails for ext3 and the
> > > reason is that ext3 does not support direct IO to file with enabled data
> > > journalling (open returns EINVAL because ->direct_IO callback is not
> > > specified). So I was thinking how to accomodate this fact in the test -
> > > the best I found was to just check using xfs_io whether O_DIRECT open
> > > succeeds and perform the test only in that case. Attached patch does this
> > > or do people have other ideas?
> >
> >
> > # ext3 doesn't support direct IO in journalling mode
> > ext3_write_opt_list="iflag=noatime conv=notrunc conv=fsync"
> > ext4_write_opt_list="iflag=noatime conv=notrunc conv=fsync oflag=direct"
> >
> > if [ $FSTYP = "ext3" ]; then
> > write_opt_list="$ext3_write_opt_list"
> > else
> > write_opt_list="$ext4_write_opt_list"
> > fi
> Yeah, this is probably simpler. Thanks for suggestion. BTW, ext4 also
> does not support direct IO and data journalling but it silently falls back
> to buffered IO. Anyway new patch is attached.
Hum, after testing this I realized why I did the things the original way.
The test writes a message with title for each test so when direct IO tests
are skipped outputs don't match.

After some thought I decided to change the test to output titles only to
full output and keep compared output (almost) empty. Result is attached.

Honza


Attachments:
(No filename) (1.53 kB)
0001-Make-test-272-work-for-ext3.patch (3.63 kB)
Download all attachments

2012-08-20 22:49:44

by Dave Chinner

[permalink] [raw]
Subject: Re: Test 272 fails for ext3

On Mon, Aug 20, 2012 at 11:06:38PM +0200, Jan Kara wrote:
> On Mon 20-08-12 18:22:12, Jan Kara wrote:
> > On Fri 17-08-12 08:48:58, Dave Chinner wrote:
> > > On Thu, Aug 16, 2012 at 11:37:57AM +0200, Jan Kara wrote:
> > > > Hello,
> > > >
> > > > I was looking into why test 272 from xfstests fails for ext3 and the
> > > > reason is that ext3 does not support direct IO to file with enabled data
> > > > journalling (open returns EINVAL because ->direct_IO callback is not
> > > > specified). So I was thinking how to accomodate this fact in the test -
> > > > the best I found was to just check using xfs_io whether O_DIRECT open
> > > > succeeds and perform the test only in that case. Attached patch does this
> > > > or do people have other ideas?
> > >
> > >
> > > # ext3 doesn't support direct IO in journalling mode
> > > ext3_write_opt_list="iflag=noatime conv=notrunc conv=fsync"
> > > ext4_write_opt_list="iflag=noatime conv=notrunc conv=fsync oflag=direct"
> > >
> > > if [ $FSTYP = "ext3" ]; then
> > > write_opt_list="$ext3_write_opt_list"
> > > else
> > > write_opt_list="$ext4_write_opt_list"
> > > fi
> > Yeah, this is probably simpler. Thanks for suggestion. BTW, ext4 also
> > does not support direct IO and data journalling but it silently falls back
> > to buffered IO. Anyway new patch is attached.
> Hum, after testing this I realized why I did the things the original way.
> The test writes a message with title for each test so when direct IO tests
> are skipped outputs don't match.
>
> After some thought I decided to change the test to output titles only to
> full output and keep compared output (almost) empty. Result is attached.

....
> --- a/272
> +++ b/272
> @@ -32,15 +32,18 @@ tmp=/tmp/$$
> status=1 # failure is the default!
> trap "rm -f $tmp.*; exit \$status" 0 1 2 3 15
>
> +write_opt_list="iflag=noatime conv=notrunc conv=fsync oflag=direct"
> +if [ $FSTYP = "ext3" ]; then
> + # ext3 doesn't support direct IO in journalling mode
> + write_opt_list="iflag=noatime conv=notrunc conv=fsync"
> +fi

Seems like asking for trouble duplicating the common options.

# ext3 doesn't support direct IO in journalling mode
write_opt_list="iflag=noatime conv=notrunc conv=fsync
[ $FSTYP = "ext4" ] && write_opt_list="$write_opt_list oflag=direct"

Otherwise looks fine.

Reviewed-by: Dave Chinner <[email protected]>

Cheers,

Dave.
--
Dave Chinner
[email protected]

2012-08-21 08:03:54

by Jan Kara

[permalink] [raw]
Subject: Re: Test 272 fails for ext3

On Tue 21-08-12 08:49:41, Dave Chinner wrote:
> On Mon, Aug 20, 2012 at 11:06:38PM +0200, Jan Kara wrote:
> ....
> > --- a/272
> > +++ b/272
> > @@ -32,15 +32,18 @@ tmp=/tmp/$$
> > status=1 # failure is the default!
> > trap "rm -f $tmp.*; exit \$status" 0 1 2 3 15
> >
> > +write_opt_list="iflag=noatime conv=notrunc conv=fsync oflag=direct"
> > +if [ $FSTYP = "ext3" ]; then
> > + # ext3 doesn't support direct IO in journalling mode
> > + write_opt_list="iflag=noatime conv=notrunc conv=fsync"
> > +fi
>
> Seems like asking for trouble duplicating the common options.
>
> # ext3 doesn't support direct IO in journalling mode
> write_opt_list="iflag=noatime conv=notrunc conv=fsync
> [ $FSTYP = "ext4" ] && write_opt_list="$write_opt_list oflag=direct"
>
> Otherwise looks fine.
>
> Reviewed-by: Dave Chinner <[email protected]>
OK, I've updated the test as you suggested. Result is attached.

Honza

--
Jan Kara <[email protected]>
SUSE Labs, CR


Attachments:
(No filename) (965.00 B)
0001-Make-test-272-work-for-ext3.patch (3.70 kB)
Download all attachments

2012-09-18 21:26:44

by Ben Myers

[permalink] [raw]
Subject: Re: Test 272 fails for ext3

On Tue, Aug 21, 2012 at 10:03:39AM +0200, Jan Kara wrote:
> On Tue 21-08-12 08:49:41, Dave Chinner wrote:
> > On Mon, Aug 20, 2012 at 11:06:38PM +0200, Jan Kara wrote:
> > ....
> > > --- a/272
> > > +++ b/272
> > > @@ -32,15 +32,18 @@ tmp=/tmp/$$
> > > status=1 # failure is the default!
> > > trap "rm -f $tmp.*; exit \$status" 0 1 2 3 15
> > >
> > > +write_opt_list="iflag=noatime conv=notrunc conv=fsync oflag=direct"
> > > +if [ $FSTYP = "ext3" ]; then
> > > + # ext3 doesn't support direct IO in journalling mode
> > > + write_opt_list="iflag=noatime conv=notrunc conv=fsync"
> > > +fi
> >
> > Seems like asking for trouble duplicating the common options.
> >
> > # ext3 doesn't support direct IO in journalling mode
> > write_opt_list="iflag=noatime conv=notrunc conv=fsync
> > [ $FSTYP = "ext4" ] && write_opt_list="$write_opt_list oflag=direct"
> >
> > Otherwise looks fine.
> >
> > Reviewed-by: Dave Chinner <[email protected]>
> OK, I've updated the test as you suggested. Result is attached.

Committed to git://oss.sgi.com/xfs/cmds/xfstests.git, master branch.