[PATCH] generic: concurrent IO test with mixed IO types

[PATCH] generic: concurrent IO test with mixed IO types

[Eryu Guan]

Test concurrent buffered I/O, DIO, AIO, mmap I/O and splice I/O on the
same files.

Signed-off-by: Eryu Guan <eguan@redhat.com>
---

This fio job file has been proven to be potent, it triggers WARNINGs on ext4
and xfs with 4.1-rc6 kernel.

ext4: WARNING: at fs/ext4/inode.c:1328
xfs: WARNING: CPU: 7 PID: 3090 at fs/xfs/xfs_file.c:726 xfs_file_dio_aio_write+0x176/0x2a8 [xfs]()

The ext4 issue should be fixed by Lukas's patch
ext4: fix reservation release on invalidatepage for delalloc fs

And it ever paniced kernel in mm code and hung xfs.

I reduced the numjobs and iodepth to reduce the test time(~25s on my test host)
and scale them by $LOAD_FACTOR. And it still could trigger the warning on ext4
and xfs with reduced workload.

 tests/generic/090     | 121 ++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/generic/090.out |   2 +
 tests/generic/group   |   1 +
 3 files changed, 124 insertions(+)
 create mode 100755 tests/generic/090
 create mode 100644 tests/generic/090.out

diff --git a/tests/generic/090 b/tests/generic/090
new file mode 100755
index 0000000..748e935
--- /dev/null
+++ b/tests/generic/090
@@ -0,0 +1,121 @@
+#! /bin/bash
+# FS QA Test generic/090
+#
+# Concurrent mixed I/O (buffer I/O, aiodio, mmap, splice) on the same files
+#
+#-----------------------------------------------------------------------
+# Copyright (c) 2015 Red Hat Inc. All Rights Reserved.
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it would be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write the Free Software Foundation,
+# Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+#-----------------------------------------------------------------------
+#
+
+seq=`basename $0`
+seqres=$RESULT_DIR/$seq
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=/tmp/$$
+fio_config=$tmp.fio
+status=1	# failure is the default!
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+	cd /
+	rm -f $tmp.*
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+
+# real QA test starts here
+_supported_fs generic
+_supported_os Linux
+_require_scratch
+
+iodepth=$((16 * LOAD_FACTOR))
+iodepth_batch=$((8 * LOAD_FACTOR))
+numjobs=$((5 * LOAD_FACTOR))
+cat >$fio_config <<EOF
+[global]
+bs=8k
+iodepth=$iodepth
+iodepth_batch=$iodepth_batch
+randrepeat=1
+size=1m
+directory=$SCRATCH_MNT
+numjobs=$numjobs
+[job1]
+ioengine=sync
+bs=1k
+direct=1
+rw=randread
+filename=file1:file2
+[job2]
+ioengine=libaio
+rw=randwrite
+direct=1
+filename=file1:file2
+[job3]
+bs=1k
+ioengine=posixaio
+rw=randwrite
+direct=1
+filename=file1:file2
+[job4]
+ioengine=splice
+direct=1
+rw=randwrite
+filename=file1:file2
+[job5]
+bs=1k
+ioengine=sync
+rw=randread
+filename=file1:file2
+[job6]
+ioengine=posixaio
+rw=randwrite
+filename=file1:file2
+[job7]
+ioengine=splice
+rw=randwrite
+filename=file1:file2
+[job8]
+ioengine=mmap
+rw=randwrite
+bs=1k
+filename=file1:file2
+[job9]
+ioengine=mmap
+rw=randwrite
+direct=1
+filename=file1:file2
+EOF
+# with ioengine=mmap and direct=1, fio requires bs to be at least pagesize,
+# which is a fio built-in var.
+echo 'bs=$pagesize' >> $fio_config
+
+rm -f $seqres.full
+
+_require_fio $fio_config
+
+echo "Silence is golden"
+$FIO_PROG $fio_config >>$seqres.full 2>&1
+
+# all done, expect no hang no oops no fs corruption,
+# _check_dmesg and _check_filesystems will do the check work for us
+status=0
+exit
diff --git a/tests/generic/090.out b/tests/generic/090.out
new file mode 100644
index 0000000..2b5100d
--- /dev/null
+++ b/tests/generic/090.out
@@ -0,0 +1,2 @@
+QA output created by 090
+Silence is golden
diff --git a/tests/generic/group b/tests/generic/group
index 0c8964c..2e534a5 100644
--- a/tests/generic/group
+++ b/tests/generic/group
@@ -92,6 +92,7 @@
 087 perms auto quick
 088 perms auto quick
 089 metadata auto
+090 auto rw stress
 091 rw auto quick
 092 auto quick prealloc
 093 attr cap udf auto
-- 
1.8.3.1

Re: [PATCH] generic: concurrent IO test with mixed IO types

[Lukáš Czerner]

On Mon, 8 Jun 2015, Eryu Guan wrote:

> Date: Mon,  8 Jun 2015 18:41:41 +0800
> From: Eryu Guan <eguan@redhat.com>
> To: fstests@vger.kernel.org
> Cc: lczerner@redhat.com, Eryu Guan <eguan@redhat.com>
> Subject: [PATCH] generic: concurrent IO test with mixed IO types
> 
> Test concurrent buffered I/O, DIO, AIO, mmap I/O and splice I/O on the
> same files.

Hi Eryu,

Great! Thanks for turning it into a test. Couple of comments below.

> 
> Signed-off-by: Eryu Guan <eguan@redhat.com>
> ---
> 
> This fio job file has been proven to be potent, it triggers WARNINGs on ext4
> and xfs with 4.1-rc6 kernel.
> 
> ext4: WARNING: at fs/ext4/inode.c:1328
> xfs: WARNING: CPU: 7 PID: 3090 at fs/xfs/xfs_file.c:726 xfs_file_dio_aio_write+0x176/0x2a8 [xfs]()
> 
> The ext4 issue should be fixed by Lukas's patch
> ext4: fix reservation release on invalidatepage for delalloc fs
> 
> And it ever paniced kernel in mm code and hung xfs.
> 
> I reduced the numjobs and iodepth to reduce the test time(~25s on my test host)
> and scale them by $LOAD_FACTOR. And it still could trigger the warning on ext4
> and xfs with reduced workload.
> 
>  tests/generic/090     | 121 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  tests/generic/090.out |   2 +
>  tests/generic/group   |   1 +
>  3 files changed, 124 insertions(+)
>  create mode 100755 tests/generic/090
>  create mode 100644 tests/generic/090.out
> 
> diff --git a/tests/generic/090 b/tests/generic/090
> new file mode 100755
> index 0000000..748e935
> --- /dev/null
> +++ b/tests/generic/090
> @@ -0,0 +1,121 @@
> +#! /bin/bash
> +# FS QA Test generic/090
> +#
> +# Concurrent mixed I/O (buffer I/O, aiodio, mmap, splice) on the same files
> +#
> +#-----------------------------------------------------------------------
> +# Copyright (c) 2015 Red Hat Inc. All Rights Reserved.
> +#
> +# This program is free software; you can redistribute it and/or
> +# modify it under the terms of the GNU General Public License as
> +# published by the Free Software Foundation.
> +#
> +# This program is distributed in the hope that it would be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program; if not, write the Free Software Foundation,
> +# Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
> +#-----------------------------------------------------------------------
> +#
> +
> +seq=`basename $0`
> +seqres=$RESULT_DIR/$seq
> +echo "QA output created by $seq"
> +
> +here=`pwd`
> +tmp=/tmp/$$

Not a big deal, but using mktemp woud probably be better.

> +fio_config=$tmp.fio
> +status=1	# failure is the default!
> +trap "_cleanup; exit \$status" 0 1 2 3 15
> +
> +_cleanup()
> +{
> +	cd /
> +	rm -f $tmp.*
> +}
> +
> +# get standard environment, filters and checks
> +. ./common/rc
> +. ./common/filter
> +
> +# real QA test starts here
> +_supported_fs generic
> +_supported_os Linux
> +_require_scratch
> +
> +iodepth=$((16 * LOAD_FACTOR))
> +iodepth_batch=$((8 * LOAD_FACTOR))
> +numjobs=$((5 * LOAD_FACTOR))
> +cat >$fio_config <<EOF
> +[global]
> +bs=8k
> +iodepth=$iodepth
> +iodepth_batch=$iodepth_batch
> +randrepeat=1
> +size=1m
> +directory=$SCRATCH_MNT
> +numjobs=$numjobs
> +[job1]
> +ioengine=sync
> +bs=1k
> +direct=1
> +rw=randread
> +filename=file1:file2
> +[job2]
> +ioengine=libaio
> +rw=randwrite
> +direct=1
> +filename=file1:file2
> +[job3]
> +bs=1k
> +ioengine=posixaio
> +rw=randwrite
> +direct=1
> +filename=file1:file2
> +[job4]
> +ioengine=splice
> +direct=1
> +rw=randwrite
> +filename=file1:file2
> +[job5]
> +bs=1k
> +ioengine=sync
> +rw=randread
> +filename=file1:file2
> +[job6]
> +ioengine=posixaio
> +rw=randwrite
> +filename=file1:file2
> +[job7]
> +ioengine=splice
> +rw=randwrite
> +filename=file1:file2
> +[job8]
> +ioengine=mmap
> +rw=randwrite
> +bs=1k
> +filename=file1:file2
> +[job9]
> +ioengine=mmap
> +rw=randwrite
> +direct=1
> +filename=file1:file2
> +EOF
> +# with ioengine=mmap and direct=1, fio requires bs to be at least pagesize,
> +# which is a fio built-in var.
> +echo 'bs=$pagesize' >> $fio_config

Can't this be part of the the fio script creation above ?

Thanks!
-Lukas

> +
> +rm -f $seqres.full
> +
> +_require_fio $fio_config
> +
> +echo "Silence is golden"
> +$FIO_PROG $fio_config >>$seqres.full 2>&1
> +
> +# all done, expect no hang no oops no fs corruption,
> +# _check_dmesg and _check_filesystems will do the check work for us
> +status=0
> +exit
> diff --git a/tests/generic/090.out b/tests/generic/090.out
> new file mode 100644
> index 0000000..2b5100d
> --- /dev/null
> +++ b/tests/generic/090.out
> @@ -0,0 +1,2 @@
> +QA output created by 090
> +Silence is golden
> diff --git a/tests/generic/group b/tests/generic/group
> index 0c8964c..2e534a5 100644
> --- a/tests/generic/group
> +++ b/tests/generic/group
> @@ -92,6 +92,7 @@
>  087 perms auto quick
>  088 perms auto quick
>  089 metadata auto
> +090 auto rw stress
>  091 rw auto quick
>  092 auto quick prealloc
>  093 attr cap udf auto
> 

Re: [PATCH] generic: concurrent IO test with mixed IO types

[Eryu Guan]

On Mon, Jun 08, 2015 at 01:02:48PM +0200, Lukáš Czerner wrote:
> On Mon, 8 Jun 2015, Eryu Guan wrote:
> 
> > Date: Mon,  8 Jun 2015 18:41:41 +0800
> > From: Eryu Guan <eguan@redhat.com>
> > To: fstests@vger.kernel.org
> > Cc: lczerner@redhat.com, Eryu Guan <eguan@redhat.com>
> > Subject: [PATCH] generic: concurrent IO test with mixed IO types
> > 
> > Test concurrent buffered I/O, DIO, AIO, mmap I/O and splice I/O on the
> > same files.
> 
> Hi Eryu,
> 
> Great! Thanks for turning it into a test. Couple of comments below.
> 
> > 
> > Signed-off-by: Eryu Guan <eguan@redhat.com>
> > ---
> > 
> > This fio job file has been proven to be potent, it triggers WARNINGs on ext4
> > and xfs with 4.1-rc6 kernel.
> > 
> > ext4: WARNING: at fs/ext4/inode.c:1328
> > xfs: WARNING: CPU: 7 PID: 3090 at fs/xfs/xfs_file.c:726 xfs_file_dio_aio_write+0x176/0x2a8 [xfs]()
> > 
> > The ext4 issue should be fixed by Lukas's patch
> > ext4: fix reservation release on invalidatepage for delalloc fs
> > 
> > And it ever paniced kernel in mm code and hung xfs.
> > 
> > I reduced the numjobs and iodepth to reduce the test time(~25s on my test host)
> > and scale them by $LOAD_FACTOR. And it still could trigger the warning on ext4
> > and xfs with reduced workload.
> > 
> >  tests/generic/090     | 121 ++++++++++++++++++++++++++++++++++++++++++++++++++
> >  tests/generic/090.out |   2 +
> >  tests/generic/group   |   1 +
> >  3 files changed, 124 insertions(+)
> >  create mode 100755 tests/generic/090
> >  create mode 100644 tests/generic/090.out
> > 
> > diff --git a/tests/generic/090 b/tests/generic/090
> > new file mode 100755
> > index 0000000..748e935
> > --- /dev/null
> > +++ b/tests/generic/090
> > @@ -0,0 +1,121 @@
> > +#! /bin/bash
> > +# FS QA Test generic/090
> > +#
> > +# Concurrent mixed I/O (buffer I/O, aiodio, mmap, splice) on the same files
> > +#
> > +#-----------------------------------------------------------------------
> > +# Copyright (c) 2015 Red Hat Inc. All Rights Reserved.
> > +#
> > +# This program is free software; you can redistribute it and/or
> > +# modify it under the terms of the GNU General Public License as
> > +# published by the Free Software Foundation.
> > +#
> > +# This program is distributed in the hope that it would be useful,
> > +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > +# GNU General Public License for more details.
> > +#
> > +# You should have received a copy of the GNU General Public License
> > +# along with this program; if not, write the Free Software Foundation,
> > +# Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
> > +#-----------------------------------------------------------------------
> > +#
> > +
> > +seq=`basename $0`
> > +seqres=$RESULT_DIR/$seq
> > +echo "QA output created by $seq"
> > +
> > +here=`pwd`
> > +tmp=/tmp/$$
> 
> Not a big deal, but using mktemp woud probably be better.

Will do in v2.

> 
> > +fio_config=$tmp.fio
> > +status=1	# failure is the default!
> > +trap "_cleanup; exit \$status" 0 1 2 3 15
> > +
> > +_cleanup()
> > +{
> > +	cd /
> > +	rm -f $tmp.*
> > +}
> > +
> > +# get standard environment, filters and checks
> > +. ./common/rc
> > +. ./common/filter
> > +
> > +# real QA test starts here
> > +_supported_fs generic
> > +_supported_os Linux
> > +_require_scratch
> > +
> > +iodepth=$((16 * LOAD_FACTOR))
> > +iodepth_batch=$((8 * LOAD_FACTOR))
> > +numjobs=$((5 * LOAD_FACTOR))
> > +cat >$fio_config <<EOF
> > +[global]
> > +bs=8k
> > +iodepth=$iodepth
> > +iodepth_batch=$iodepth_batch
> > +randrepeat=1
> > +size=1m
> > +directory=$SCRATCH_MNT
> > +numjobs=$numjobs
> > +[job1]
> > +ioengine=sync
> > +bs=1k
> > +direct=1
> > +rw=randread
> > +filename=file1:file2
> > +[job2]
> > +ioengine=libaio
> > +rw=randwrite
> > +direct=1
> > +filename=file1:file2
> > +[job3]
> > +bs=1k
> > +ioengine=posixaio
> > +rw=randwrite
> > +direct=1
> > +filename=file1:file2
> > +[job4]
> > +ioengine=splice
> > +direct=1
> > +rw=randwrite
> > +filename=file1:file2
> > +[job5]
> > +bs=1k
> > +ioengine=sync
> > +rw=randread
> > +filename=file1:file2
> > +[job6]
> > +ioengine=posixaio
> > +rw=randwrite
> > +filename=file1:file2
> > +[job7]
> > +ioengine=splice
> > +rw=randwrite
> > +filename=file1:file2
> > +[job8]
> > +ioengine=mmap
> > +rw=randwrite
> > +bs=1k
> > +filename=file1:file2
> > +[job9]
> > +ioengine=mmap
> > +rw=randwrite
> > +direct=1
> > +filename=file1:file2
> > +EOF
> > +# with ioengine=mmap and direct=1, fio requires bs to be at least pagesize,
> > +# which is a fio built-in var.
> > +echo 'bs=$pagesize' >> $fio_config
> 
> Can't this be part of the the fio script creation above ?

The 'here-document' replaces every var to its value, e.g. $numjobs => 10
(if LOAD_FACTOR is 1), so we don't have $pagesize set, and it'll be
empty in the fio config file, that's not what we want.

"bs=$pagesize" is literally what we want in the config file, $pagesize
is a special var for fio, it will be replaced by real pagesize
internally by fio.

cat >file <<'EOF'
text
EOF

This kind of 'here-document' ('' around EOF) won't do any variable
expansion, but we need $numjobs to be expanded. So they can't be merged
together.

Thanks,
Eryu

> 
> Thanks!
> -Lukas
> 
> > +
> > +rm -f $seqres.full
> > +
> > +_require_fio $fio_config
> > +
> > +echo "Silence is golden"
> > +$FIO_PROG $fio_config >>$seqres.full 2>&1
> > +
> > +# all done, expect no hang no oops no fs corruption,
> > +# _check_dmesg and _check_filesystems will do the check work for us
> > +status=0
> > +exit
> > diff --git a/tests/generic/090.out b/tests/generic/090.out
> > new file mode 100644
> > index 0000000..2b5100d
> > --- /dev/null
> > +++ b/tests/generic/090.out
> > @@ -0,0 +1,2 @@
> > +QA output created by 090
> > +Silence is golden
> > diff --git a/tests/generic/group b/tests/generic/group
> > index 0c8964c..2e534a5 100644
> > --- a/tests/generic/group
> > +++ b/tests/generic/group
> > @@ -92,6 +92,7 @@
> >  087 perms auto quick
> >  088 perms auto quick
> >  089 metadata auto
> > +090 auto rw stress
> >  091 rw auto quick
> >  092 auto quick prealloc
> >  093 attr cap udf auto
> > 

Re: [PATCH] generic: concurrent IO test with mixed IO types

[Lukáš Czerner]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 7205 bytes --]

On Mon, 8 Jun 2015, Eryu Guan wrote:

> Date: Mon, 8 Jun 2015 19:59:47 +0800
> From: Eryu Guan <eguan@redhat.com>
> To: Lukáš Czerner <lczerner@redhat.com>
> Cc: fstests@vger.kernel.org
> Subject: Re: [PATCH] generic: concurrent IO test with mixed IO types
> 
> On Mon, Jun 08, 2015 at 01:02:48PM +0200, Lukáš Czerner wrote:
> > On Mon, 8 Jun 2015, Eryu Guan wrote:
> > 
> > > Date: Mon,  8 Jun 2015 18:41:41 +0800
> > > From: Eryu Guan <eguan@redhat.com>
> > > To: fstests@vger.kernel.org
> > > Cc: lczerner@redhat.com, Eryu Guan <eguan@redhat.com>
> > > Subject: [PATCH] generic: concurrent IO test with mixed IO types
> > > 
> > > Test concurrent buffered I/O, DIO, AIO, mmap I/O and splice I/O on the
> > > same files.
> > 
> > Hi Eryu,
> > 
> > Great! Thanks for turning it into a test. Couple of comments below.
> > 
> > > 
> > > Signed-off-by: Eryu Guan <eguan@redhat.com>
> > > ---
> > > 
> > > This fio job file has been proven to be potent, it triggers WARNINGs on ext4
> > > and xfs with 4.1-rc6 kernel.
> > > 
> > > ext4: WARNING: at fs/ext4/inode.c:1328
> > > xfs: WARNING: CPU: 7 PID: 3090 at fs/xfs/xfs_file.c:726 xfs_file_dio_aio_write+0x176/0x2a8 [xfs]()
> > > 
> > > The ext4 issue should be fixed by Lukas's patch
> > > ext4: fix reservation release on invalidatepage for delalloc fs
> > > 
> > > And it ever paniced kernel in mm code and hung xfs.
> > > 
> > > I reduced the numjobs and iodepth to reduce the test time(~25s on my test host)
> > > and scale them by $LOAD_FACTOR. And it still could trigger the warning on ext4
> > > and xfs with reduced workload.
> > > 
> > >  tests/generic/090     | 121 ++++++++++++++++++++++++++++++++++++++++++++++++++
> > >  tests/generic/090.out |   2 +
> > >  tests/generic/group   |   1 +
> > >  3 files changed, 124 insertions(+)
> > >  create mode 100755 tests/generic/090
> > >  create mode 100644 tests/generic/090.out
> > > 
> > > diff --git a/tests/generic/090 b/tests/generic/090
> > > new file mode 100755
> > > index 0000000..748e935
> > > --- /dev/null
> > > +++ b/tests/generic/090
> > > @@ -0,0 +1,121 @@
> > > +#! /bin/bash
> > > +# FS QA Test generic/090
> > > +#
> > > +# Concurrent mixed I/O (buffer I/O, aiodio, mmap, splice) on the same files
> > > +#
> > > +#-----------------------------------------------------------------------
> > > +# Copyright (c) 2015 Red Hat Inc. All Rights Reserved.
> > > +#
> > > +# This program is free software; you can redistribute it and/or
> > > +# modify it under the terms of the GNU General Public License as
> > > +# published by the Free Software Foundation.
> > > +#
> > > +# This program is distributed in the hope that it would be useful,
> > > +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> > > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > > +# GNU General Public License for more details.
> > > +#
> > > +# You should have received a copy of the GNU General Public License
> > > +# along with this program; if not, write the Free Software Foundation,
> > > +# Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
> > > +#-----------------------------------------------------------------------
> > > +#
> > > +
> > > +seq=`basename $0`
> > > +seqres=$RESULT_DIR/$seq
> > > +echo "QA output created by $seq"
> > > +
> > > +here=`pwd`
> > > +tmp=/tmp/$$
> > 
> > Not a big deal, but using mktemp woud probably be better.
> 
> Will do in v2.
> 
> > 
> > > +fio_config=$tmp.fio
> > > +status=1	# failure is the default!
> > > +trap "_cleanup; exit \$status" 0 1 2 3 15
> > > +
> > > +_cleanup()
> > > +{
> > > +	cd /
> > > +	rm -f $tmp.*
> > > +}
> > > +
> > > +# get standard environment, filters and checks
> > > +. ./common/rc
> > > +. ./common/filter
> > > +
> > > +# real QA test starts here
> > > +_supported_fs generic
> > > +_supported_os Linux
> > > +_require_scratch
> > > +
> > > +iodepth=$((16 * LOAD_FACTOR))
> > > +iodepth_batch=$((8 * LOAD_FACTOR))
> > > +numjobs=$((5 * LOAD_FACTOR))
> > > +cat >$fio_config <<EOF
> > > +[global]
> > > +bs=8k
> > > +iodepth=$iodepth
> > > +iodepth_batch=$iodepth_batch
> > > +randrepeat=1
> > > +size=1m
> > > +directory=$SCRATCH_MNT
> > > +numjobs=$numjobs
> > > +[job1]
> > > +ioengine=sync
> > > +bs=1k
> > > +direct=1
> > > +rw=randread
> > > +filename=file1:file2
> > > +[job2]
> > > +ioengine=libaio
> > > +rw=randwrite
> > > +direct=1
> > > +filename=file1:file2
> > > +[job3]
> > > +bs=1k
> > > +ioengine=posixaio
> > > +rw=randwrite
> > > +direct=1
> > > +filename=file1:file2
> > > +[job4]
> > > +ioengine=splice
> > > +direct=1
> > > +rw=randwrite
> > > +filename=file1:file2
> > > +[job5]
> > > +bs=1k
> > > +ioengine=sync
> > > +rw=randread
> > > +filename=file1:file2
> > > +[job6]
> > > +ioengine=posixaio
> > > +rw=randwrite
> > > +filename=file1:file2
> > > +[job7]
> > > +ioengine=splice
> > > +rw=randwrite
> > > +filename=file1:file2
> > > +[job8]
> > > +ioengine=mmap
> > > +rw=randwrite
> > > +bs=1k
> > > +filename=file1:file2
> > > +[job9]
> > > +ioengine=mmap
> > > +rw=randwrite
> > > +direct=1
> > > +filename=file1:file2
> > > +EOF
> > > +# with ioengine=mmap and direct=1, fio requires bs to be at least pagesize,
> > > +# which is a fio built-in var.
> > > +echo 'bs=$pagesize' >> $fio_config
> > 
> > Can't this be part of the the fio script creation above ?
> 
> The 'here-document' replaces every var to its value, e.g. $numjobs => 10
> (if LOAD_FACTOR is 1), so we don't have $pagesize set, and it'll be
> empty in the fio config file, that's not what we want.
> 
> "bs=$pagesize" is literally what we want in the config file, $pagesize
> is a special var for fio, it will be replaced by real pagesize
> internally by fio.
> 
> cat >file <<'EOF'
> text
> EOF
> 
> This kind of 'here-document' ('' around EOF) won't do any variable
> expansion, but we need $numjobs to be expanded. So they can't be merged
> together.
> 
> Thanks,
> Eryu

Ah right :) I missed the quotes, sorry.

Thanks!
-Lukas


> 
> > 
> > Thanks!
> > -Lukas
> > 
> > > +
> > > +rm -f $seqres.full
> > > +
> > > +_require_fio $fio_config
> > > +
> > > +echo "Silence is golden"
> > > +$FIO_PROG $fio_config >>$seqres.full 2>&1
> > > +
> > > +# all done, expect no hang no oops no fs corruption,
> > > +# _check_dmesg and _check_filesystems will do the check work for us
> > > +status=0
> > > +exit
> > > diff --git a/tests/generic/090.out b/tests/generic/090.out
> > > new file mode 100644
> > > index 0000000..2b5100d
> > > --- /dev/null
> > > +++ b/tests/generic/090.out
> > > @@ -0,0 +1,2 @@
> > > +QA output created by 090
> > > +Silence is golden
> > > diff --git a/tests/generic/group b/tests/generic/group
> > > index 0c8964c..2e534a5 100644
> > > --- a/tests/generic/group
> > > +++ b/tests/generic/group
> > > @@ -92,6 +92,7 @@
> > >  087 perms auto quick
> > >  088 perms auto quick
> > >  089 metadata auto
> > > +090 auto rw stress
> > >  091 rw auto quick
> > >  092 auto quick prealloc
> > >  093 attr cap udf auto
> > > 
> 

Re: [PATCH] generic: concurrent IO test with mixed IO types

[Dave Chinner]

On Mon, Jun 08, 2015 at 01:02:48PM +0200, Lukáš Czerner wrote:
> On Mon, 8 Jun 2015, Eryu Guan wrote:
> 
> > Date: Mon,  8 Jun 2015 18:41:41 +0800 From: Eryu Guan
> > <eguan@redhat.com> To: fstests@vger.kernel.org Cc:
> > lczerner@redhat.com, Eryu Guan <eguan@redhat.com> Subject:
> > [PATCH] generic: concurrent IO test with mixed IO types
> > 
> > Test concurrent buffered I/O, DIO, AIO, mmap I/O and splice I/O
> > on the same files.
> 
> Hi Eryu,
> 
> Great! Thanks for turning it into a test. Couple of comments
> below.
....
> > + +seq=`basename $0` +seqres=$RESULT_DIR/$seq +echo "QA output
> > created by $seq" + +here=`pwd` +tmp=/tmp/$$
> 
> Not a big deal, but using mktemp woud probably be better.

Not better at all - this reduces the easy of debugging problems.
e.g.  if a test is hung, I know where it's temporary output files
are just by looking at the process listing and grabbing the pid of
the test process being run. IOWs, we *want* deterministic temporary
filenames here - obscuring the temporary file names that tests use
doesn't gain us anything.

Yes, I see that some btrfs tests have used mktemp  and a couple of
generic tests use $TEST_DIR/$$ - they should be reverted to /tmp/$$
so they are consistent with the majority of tests and the new test
template....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

[PATCH v2] generic: concurrent IO test with mixed IO types

[Eryu Guan]

Test concurrent buffered I/O, DIO, AIO, mmap I/O and splice I/O on the
same files.

Signed-off-by: Eryu Guan <eguan@redhat.com>
---

This fio job file has been proven to be potent, it triggers WARNINGs on ext4
and xfs with 4.1-rc6 kernel.

ext4: WARNING: at fs/ext4/inode.c:1328
xfs: WARNING: CPU: 7 PID: 3090 at fs/xfs/xfs_file.c:726 xfs_file_dio_aio_write+0x176/0x2a8 [xfs]()

The ext4 issue should be fixed by Lukas's patch
ext4: fix reservation release on invalidatepage for delalloc fs

And it ever paniced kernel in mm code and hung xfs.

I reduced the numjobs and iodepth to reduce the test time(~25s on my test host)
and scale them by $LOAD_FACTOR. And it still could trigger the warning on ext4
and xfs with reduced workload.

v2:
- use mktemp to create tmp fio job file

 tests/generic/090     | 120 ++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/generic/090.out |   2 +
 tests/generic/group   |   1 +
 3 files changed, 123 insertions(+)
 create mode 100755 tests/generic/090
 create mode 100644 tests/generic/090.out

diff --git a/tests/generic/090 b/tests/generic/090
new file mode 100755
index 0000000..3521cd4
--- /dev/null
+++ b/tests/generic/090
@@ -0,0 +1,120 @@
+#! /bin/bash
+# FS QA Test generic/090
+#
+# Concurrent mixed I/O (buffer I/O, aiodio, mmap, splice) on the same files
+#
+#-----------------------------------------------------------------------
+# Copyright (c) 2015 Red Hat Inc. All Rights Reserved.
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it would be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write the Free Software Foundation,
+# Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+#-----------------------------------------------------------------------
+#
+
+seq=`basename $0`
+seqres=$RESULT_DIR/$seq
+echo "QA output created by $seq"
+
+here=`pwd`
+fio_config=`mktemp`
+status=1	# failure is the default!
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+	cd /
+	rm -f $fio_config
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+
+# real QA test starts here
+_supported_fs generic
+_supported_os Linux
+_require_scratch
+
+iodepth=$((16 * LOAD_FACTOR))
+iodepth_batch=$((8 * LOAD_FACTOR))
+numjobs=$((5 * LOAD_FACTOR))
+cat >$fio_config <<EOF
+[global]
+bs=8k
+iodepth=$iodepth
+iodepth_batch=$iodepth_batch
+randrepeat=1
+size=1m
+directory=$SCRATCH_MNT
+numjobs=$numjobs
+[job1]
+ioengine=sync
+bs=1k
+direct=1
+rw=randread
+filename=file1:file2
+[job2]
+ioengine=libaio
+rw=randwrite
+direct=1
+filename=file1:file2
+[job3]
+bs=1k
+ioengine=posixaio
+rw=randwrite
+direct=1
+filename=file1:file2
+[job4]
+ioengine=splice
+direct=1
+rw=randwrite
+filename=file1:file2
+[job5]
+bs=1k
+ioengine=sync
+rw=randread
+filename=file1:file2
+[job6]
+ioengine=posixaio
+rw=randwrite
+filename=file1:file2
+[job7]
+ioengine=splice
+rw=randwrite
+filename=file1:file2
+[job8]
+ioengine=mmap
+rw=randwrite
+bs=1k
+filename=file1:file2
+[job9]
+ioengine=mmap
+rw=randwrite
+direct=1
+filename=file1:file2
+EOF
+# with ioengine=mmap and direct=1, fio requires bs to be at least pagesize,
+# which is a fio built-in var.
+echo 'bs=$pagesize' >> $fio_config
+
+rm -f $seqres.full
+
+_require_fio $fio_config
+
+echo "Silence is golden"
+$FIO_PROG $fio_config >>$seqres.full 2>&1
+
+# all done, expect no hang no oops no fs corruption,
+# _check_dmesg and _check_filesystems will do the check work for us
+status=0
+exit
diff --git a/tests/generic/090.out b/tests/generic/090.out
new file mode 100644
index 0000000..2b5100d
--- /dev/null
+++ b/tests/generic/090.out
@@ -0,0 +1,2 @@
+QA output created by 090
+Silence is golden
diff --git a/tests/generic/group b/tests/generic/group
index 0c8964c..2e534a5 100644
--- a/tests/generic/group
+++ b/tests/generic/group
@@ -92,6 +92,7 @@
 087 perms auto quick
 088 perms auto quick
 089 metadata auto
+090 auto rw stress
 091 rw auto quick
 092 auto quick prealloc
 093 attr cap udf auto
-- 
2.4.2

Re: [PATCH v2] generic: concurrent IO test with mixed IO types

[Lukáš Czerner]

On Mon, 8 Jun 2015, Eryu Guan wrote:

> Date: Mon,  8 Jun 2015 20:41:11 +0800
> From: Eryu Guan <eguan@redhat.com>
> To: fstests@vger.kernel.org
> Cc: lczerner@redhat.com, Eryu Guan <eguan@redhat.com>
> Subject: [PATCH v2] generic: concurrent IO test with mixed IO types
> 
> Test concurrent buffered I/O, DIO, AIO, mmap I/O and splice I/O on the
> same files.

Great, it looks good to me. Thanks!

Reviewed-by: Lukas Czerner <lczerner@redhat.com>


> 
> Signed-off-by: Eryu Guan <eguan@redhat.com>
> ---
> 
> This fio job file has been proven to be potent, it triggers WARNINGs on ext4
> and xfs with 4.1-rc6 kernel.
> 
> ext4: WARNING: at fs/ext4/inode.c:1328
> xfs: WARNING: CPU: 7 PID: 3090 at fs/xfs/xfs_file.c:726 xfs_file_dio_aio_write+0x176/0x2a8 [xfs]()
> 
> The ext4 issue should be fixed by Lukas's patch
> ext4: fix reservation release on invalidatepage for delalloc fs
> 
> And it ever paniced kernel in mm code and hung xfs.
> 
> I reduced the numjobs and iodepth to reduce the test time(~25s on my test host)
> and scale them by $LOAD_FACTOR. And it still could trigger the warning on ext4
> and xfs with reduced workload.
> 
> v2:
> - use mktemp to create tmp fio job file
> 
>  tests/generic/090     | 120 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  tests/generic/090.out |   2 +
>  tests/generic/group   |   1 +
>  3 files changed, 123 insertions(+)
>  create mode 100755 tests/generic/090
>  create mode 100644 tests/generic/090.out
> 
> diff --git a/tests/generic/090 b/tests/generic/090
> new file mode 100755
> index 0000000..3521cd4
> --- /dev/null
> +++ b/tests/generic/090
> @@ -0,0 +1,120 @@
> +#! /bin/bash
> +# FS QA Test generic/090
> +#
> +# Concurrent mixed I/O (buffer I/O, aiodio, mmap, splice) on the same files
> +#
> +#-----------------------------------------------------------------------
> +# Copyright (c) 2015 Red Hat Inc. All Rights Reserved.
> +#
> +# This program is free software; you can redistribute it and/or
> +# modify it under the terms of the GNU General Public License as
> +# published by the Free Software Foundation.
> +#
> +# This program is distributed in the hope that it would be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program; if not, write the Free Software Foundation,
> +# Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
> +#-----------------------------------------------------------------------
> +#
> +
> +seq=`basename $0`
> +seqres=$RESULT_DIR/$seq
> +echo "QA output created by $seq"
> +
> +here=`pwd`
> +fio_config=`mktemp`
> +status=1	# failure is the default!
> +trap "_cleanup; exit \$status" 0 1 2 3 15
> +
> +_cleanup()
> +{
> +	cd /
> +	rm -f $fio_config
> +}
> +
> +# get standard environment, filters and checks
> +. ./common/rc
> +. ./common/filter
> +
> +# real QA test starts here
> +_supported_fs generic
> +_supported_os Linux
> +_require_scratch
> +
> +iodepth=$((16 * LOAD_FACTOR))
> +iodepth_batch=$((8 * LOAD_FACTOR))
> +numjobs=$((5 * LOAD_FACTOR))
> +cat >$fio_config <<EOF
> +[global]
> +bs=8k
> +iodepth=$iodepth
> +iodepth_batch=$iodepth_batch
> +randrepeat=1
> +size=1m
> +directory=$SCRATCH_MNT
> +numjobs=$numjobs
> +[job1]
> +ioengine=sync
> +bs=1k
> +direct=1
> +rw=randread
> +filename=file1:file2
> +[job2]
> +ioengine=libaio
> +rw=randwrite
> +direct=1
> +filename=file1:file2
> +[job3]
> +bs=1k
> +ioengine=posixaio
> +rw=randwrite
> +direct=1
> +filename=file1:file2
> +[job4]
> +ioengine=splice
> +direct=1
> +rw=randwrite
> +filename=file1:file2
> +[job5]
> +bs=1k
> +ioengine=sync
> +rw=randread
> +filename=file1:file2
> +[job6]
> +ioengine=posixaio
> +rw=randwrite
> +filename=file1:file2
> +[job7]
> +ioengine=splice
> +rw=randwrite
> +filename=file1:file2
> +[job8]
> +ioengine=mmap
> +rw=randwrite
> +bs=1k
> +filename=file1:file2
> +[job9]
> +ioengine=mmap
> +rw=randwrite
> +direct=1
> +filename=file1:file2
> +EOF
> +# with ioengine=mmap and direct=1, fio requires bs to be at least pagesize,
> +# which is a fio built-in var.
> +echo 'bs=$pagesize' >> $fio_config
> +
> +rm -f $seqres.full
> +
> +_require_fio $fio_config
> +
> +echo "Silence is golden"
> +$FIO_PROG $fio_config >>$seqres.full 2>&1
> +
> +# all done, expect no hang no oops no fs corruption,
> +# _check_dmesg and _check_filesystems will do the check work for us
> +status=0
> +exit
> diff --git a/tests/generic/090.out b/tests/generic/090.out
> new file mode 100644
> index 0000000..2b5100d
> --- /dev/null
> +++ b/tests/generic/090.out
> @@ -0,0 +1,2 @@
> +QA output created by 090
> +Silence is golden
> diff --git a/tests/generic/group b/tests/generic/group
> index 0c8964c..2e534a5 100644
> --- a/tests/generic/group
> +++ b/tests/generic/group
> @@ -92,6 +92,7 @@
>  087 perms auto quick
>  088 perms auto quick
>  089 metadata auto
> +090 auto rw stress
>  091 rw auto quick
>  092 auto quick prealloc
>  093 attr cap udf auto
> 

Re: [PATCH v2] generic: concurrent IO test with mixed IO types

[Dave Chinner]

On Mon, Jun 08, 2015 at 08:41:11PM +0800, Eryu Guan wrote:
> Test concurrent buffered I/O, DIO, AIO, mmap I/O and splice I/O on the
> same files.
> 
> Signed-off-by: Eryu Guan <eguan@redhat.com>
> ---
> 
> This fio job file has been proven to be potent, it triggers WARNINGs on ext4
> and xfs with 4.1-rc6 kernel.
> 
> ext4: WARNING: at fs/ext4/inode.c:1328
> xfs: WARNING: CPU: 7 PID: 3090 at fs/xfs/xfs_file.c:726 xfs_file_dio_aio_write+0x176/0x2a8 [xfs]()
> 
> The ext4 issue should be fixed by Lukas's patch
> ext4: fix reservation release on invalidatepage for delalloc fs
> 
> And it ever paniced kernel in mm code and hung xfs.
> 
> I reduced the numjobs and iodepth to reduce the test time(~25s on my test host)
> and scale them by $LOAD_FACTOR. And it still could trigger the warning on ext4
> and xfs with reduced workload.
> 
> v2:
> - use mktemp to create tmp fio job file
....
> +seq=`basename $0`
> +seqres=$RESULT_DIR/$seq
> +echo "QA output created by $seq"
> +
> +here=`pwd`
> +fio_config=`mktemp`
> +status=1	# failure is the default!
> +trap "_cleanup; exit \$status" 0 1 2 3 15

By removing the definition of $tmp, you are now dumping all
the temporary files the test harnes creates in /.

> +_cleanup()
> +{
> +	cd /
> +	rm -f $fio_config
> +}

And now not removing them when the test complests...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH v2] generic: concurrent IO test with mixed IO types

[Eryu Guan]

On Wed, Jun 10, 2015 at 08:29:33AM +1000, Dave Chinner wrote:
> On Mon, Jun 08, 2015 at 08:41:11PM +0800, Eryu Guan wrote:
> > Test concurrent buffered I/O, DIO, AIO, mmap I/O and splice I/O on the
> > same files.
> > 
> > Signed-off-by: Eryu Guan <eguan@redhat.com>
> > ---
> > 
> > This fio job file has been proven to be potent, it triggers WARNINGs on ext4
> > and xfs with 4.1-rc6 kernel.
> > 
> > ext4: WARNING: at fs/ext4/inode.c:1328
> > xfs: WARNING: CPU: 7 PID: 3090 at fs/xfs/xfs_file.c:726 xfs_file_dio_aio_write+0x176/0x2a8 [xfs]()
> > 
> > The ext4 issue should be fixed by Lukas's patch
> > ext4: fix reservation release on invalidatepage for delalloc fs
> > 
> > And it ever paniced kernel in mm code and hung xfs.
> > 
> > I reduced the numjobs and iodepth to reduce the test time(~25s on my test host)
> > and scale them by $LOAD_FACTOR. And it still could trigger the warning on ext4
> > and xfs with reduced workload.
> > 
> > v2:
> > - use mktemp to create tmp fio job file
> ....
> > +seq=`basename $0`
> > +seqres=$RESULT_DIR/$seq
> > +echo "QA output created by $seq"
> > +
> > +here=`pwd`
> > +fio_config=`mktemp`
> > +status=1	# failure is the default!
> > +trap "_cleanup; exit \$status" 0 1 2 3 15
> 
> By removing the definition of $tmp, you are now dumping all
> the temporary files the test harnes creates in /.

You're right, I see /fsck.log on my test host.

I checked 'check' and common/rc and chech has its "tmp=/tmp/$$"
definition and I thought I didn't use any functions from common/rc in my
test directly that depends on $tmp definition. I must have missed
something.

Should I send a v3 and roll back to v1? or you can just take v1?

Thanks,
Eryu

Re: [PATCH v2] generic: concurrent IO test with mixed IO types

[Dave Chinner]

On Wed, Jun 10, 2015 at 03:07:44PM +0800, Eryu Guan wrote:
> On Wed, Jun 10, 2015 at 08:29:33AM +1000, Dave Chinner wrote:
> > On Mon, Jun 08, 2015 at 08:41:11PM +0800, Eryu Guan wrote:
> > > Test concurrent buffered I/O, DIO, AIO, mmap I/O and splice I/O on the
> > > same files.
> > > 
> > > Signed-off-by: Eryu Guan <eguan@redhat.com>
> > > ---
> > > 
> > > This fio job file has been proven to be potent, it triggers WARNINGs on ext4
> > > and xfs with 4.1-rc6 kernel.
> > > 
> > > ext4: WARNING: at fs/ext4/inode.c:1328
> > > xfs: WARNING: CPU: 7 PID: 3090 at fs/xfs/xfs_file.c:726 xfs_file_dio_aio_write+0x176/0x2a8 [xfs]()
> > > 
> > > The ext4 issue should be fixed by Lukas's patch
> > > ext4: fix reservation release on invalidatepage for delalloc fs
> > > 
> > > And it ever paniced kernel in mm code and hung xfs.
> > > 
> > > I reduced the numjobs and iodepth to reduce the test time(~25s on my test host)
> > > and scale them by $LOAD_FACTOR. And it still could trigger the warning on ext4
> > > and xfs with reduced workload.
> > > 
> > > v2:
> > > - use mktemp to create tmp fio job file
> > ....
> > > +seq=`basename $0`
> > > +seqres=$RESULT_DIR/$seq
> > > +echo "QA output created by $seq"
> > > +
> > > +here=`pwd`
> > > +fio_config=`mktemp`
> > > +status=1	# failure is the default!
> > > +trap "_cleanup; exit \$status" 0 1 2 3 15
> > 
> > By removing the definition of $tmp, you are now dumping all
> > the temporary files the test harnes creates in /.
> 
> You're right, I see /fsck.log on my test host.
> 
> I checked 'check' and common/rc and chech has its "tmp=/tmp/$$"
> definition and I thought I didn't use any functions from common/rc in my
> test directly that depends on $tmp definition. I must have missed
> something.
> 
> Should I send a v3 and roll back to v1? or you can just take v1?

You'll need a v3 to fix the problem with using an uninitialised,
unmounted SCRATCH_DEV in the test...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH v2] generic: concurrent IO test with mixed IO types

[Eryu Guan]

On Wed, Jun 10, 2015 at 09:12:37PM +1000, Dave Chinner wrote:
> On Wed, Jun 10, 2015 at 03:07:44PM +0800, Eryu Guan wrote:
> > On Wed, Jun 10, 2015 at 08:29:33AM +1000, Dave Chinner wrote:
> > > On Mon, Jun 08, 2015 at 08:41:11PM +0800, Eryu Guan wrote:
> > > > Test concurrent buffered I/O, DIO, AIO, mmap I/O and splice I/O on the
> > > > same files.
> > > > 
> > > > Signed-off-by: Eryu Guan <eguan@redhat.com>
> > > > ---
> > > > 
> > > > This fio job file has been proven to be potent, it triggers WARNINGs on ext4
> > > > and xfs with 4.1-rc6 kernel.
> > > > 
> > > > ext4: WARNING: at fs/ext4/inode.c:1328
> > > > xfs: WARNING: CPU: 7 PID: 3090 at fs/xfs/xfs_file.c:726 xfs_file_dio_aio_write+0x176/0x2a8 [xfs]()
> > > > 
> > > > The ext4 issue should be fixed by Lukas's patch
> > > > ext4: fix reservation release on invalidatepage for delalloc fs
> > > > 
> > > > And it ever paniced kernel in mm code and hung xfs.
> > > > 
> > > > I reduced the numjobs and iodepth to reduce the test time(~25s on my test host)
> > > > and scale them by $LOAD_FACTOR. And it still could trigger the warning on ext4
> > > > and xfs with reduced workload.
> > > > 
> > > > v2:
> > > > - use mktemp to create tmp fio job file
> > > ....
> > > > +seq=`basename $0`
> > > > +seqres=$RESULT_DIR/$seq
> > > > +echo "QA output created by $seq"
> > > > +
> > > > +here=`pwd`
> > > > +fio_config=`mktemp`
> > > > +status=1	# failure is the default!
> > > > +trap "_cleanup; exit \$status" 0 1 2 3 15
> > > 
> > > By removing the definition of $tmp, you are now dumping all
> > > the temporary files the test harnes creates in /.
> > 
> > You're right, I see /fsck.log on my test host.
> > 
> > I checked 'check' and common/rc and chech has its "tmp=/tmp/$$"
> > definition and I thought I didn't use any functions from common/rc in my
> > test directly that depends on $tmp definition. I must have missed
> > something.
> > 
> > Should I send a v3 and roll back to v1? or you can just take v1?
> 
> You'll need a v3 to fix the problem with using an uninitialised,
> unmounted SCRATCH_DEV in the test...

How can I miss that.. Sorry and thanks!

Eryu

Re: [PATCH v2] generic: concurrent IO test with mixed IO types

[Lukáš Czerner]

On Wed, 10 Jun 2015, Dave Chinner wrote:

> Date: Wed, 10 Jun 2015 08:29:33 +1000
> From: Dave Chinner <david@fromorbit.com>
> To: Eryu Guan <eguan@redhat.com>
> Cc: fstests@vger.kernel.org, lczerner@redhat.com
> Subject: Re: [PATCH v2] generic: concurrent IO test with mixed IO types
> 
> On Mon, Jun 08, 2015 at 08:41:11PM +0800, Eryu Guan wrote:
> > Test concurrent buffered I/O, DIO, AIO, mmap I/O and splice I/O on the
> > same files.
> > 
> > Signed-off-by: Eryu Guan <eguan@redhat.com>
> > ---
> > 
> > This fio job file has been proven to be potent, it triggers WARNINGs on ext4
> > and xfs with 4.1-rc6 kernel.
> > 
> > ext4: WARNING: at fs/ext4/inode.c:1328
> > xfs: WARNING: CPU: 7 PID: 3090 at fs/xfs/xfs_file.c:726 xfs_file_dio_aio_write+0x176/0x2a8 [xfs]()
> > 
> > The ext4 issue should be fixed by Lukas's patch
> > ext4: fix reservation release on invalidatepage for delalloc fs
> > 
> > And it ever paniced kernel in mm code and hung xfs.
> > 
> > I reduced the numjobs and iodepth to reduce the test time(~25s on my test host)
> > and scale them by $LOAD_FACTOR. And it still could trigger the warning on ext4
> > and xfs with reduced workload.
> > 
> > v2:
> > - use mktemp to create tmp fio job file
> ....
> > +seq=`basename $0`
> > +seqres=$RESULT_DIR/$seq
> > +echo "QA output created by $seq"
> > +
> > +here=`pwd`
> > +fio_config=`mktemp`
> > +status=1	# failure is the default!
> > +trap "_cleanup; exit \$status" 0 1 2 3 15
> 
> By removing the definition of $tmp, you are now dumping all
> the temporary files the test harnes creates in /.

What temp files ? Yes we're sometimes using $tmp even though there
is no obvious definition and if we want to rely on the existence of
this variable we better define it as environment variable in 'check'
script. It may be enough to simply add

export tmp

to the 'check' script

But regardless of this bug it does not affect this test in any way
since it's not calling any of those functions and there are other
tests that does not define $tmp as well.

So I think that what Eryu did here is fine and we need to fix the
'check' script.

> 
> > +_cleanup()
> > +{
> > +	cd /
> > +	rm -f $fio_config
> > +}
> 
> And now not removing them when the test complests...

No, it only removes our $fio_config temp file.

Thanks!
-Lukas

> 
> Cheers,
> 
> Dave.
> 

Re: [PATCH v2] generic: concurrent IO test with mixed IO types

[Dave Chinner]

On Wed, Jun 10, 2015 at 11:01:57AM +0200, Lukáš Czerner wrote:
> On Wed, 10 Jun 2015, Dave Chinner wrote:
> 
> > Date: Wed, 10 Jun 2015 08:29:33 +1000
> > From: Dave Chinner <david@fromorbit.com>
> > To: Eryu Guan <eguan@redhat.com>
> > Cc: fstests@vger.kernel.org, lczerner@redhat.com
> > Subject: Re: [PATCH v2] generic: concurrent IO test with mixed IO types
> > 
> > On Mon, Jun 08, 2015 at 08:41:11PM +0800, Eryu Guan wrote:
> > > Test concurrent buffered I/O, DIO, AIO, mmap I/O and splice I/O on the
> > > same files.
> > > 
> > > Signed-off-by: Eryu Guan <eguan@redhat.com>
> > > ---
> > > 
> > > This fio job file has been proven to be potent, it triggers WARNINGs on ext4
> > > and xfs with 4.1-rc6 kernel.
> > > 
> > > ext4: WARNING: at fs/ext4/inode.c:1328
> > > xfs: WARNING: CPU: 7 PID: 3090 at fs/xfs/xfs_file.c:726 xfs_file_dio_aio_write+0x176/0x2a8 [xfs]()
> > > 
> > > The ext4 issue should be fixed by Lukas's patch
> > > ext4: fix reservation release on invalidatepage for delalloc fs
> > > 
> > > And it ever paniced kernel in mm code and hung xfs.
> > > 
> > > I reduced the numjobs and iodepth to reduce the test time(~25s on my test host)
> > > and scale them by $LOAD_FACTOR. And it still could trigger the warning on ext4
> > > and xfs with reduced workload.
> > > 
> > > v2:
> > > - use mktemp to create tmp fio job file
> > ....
> > > +seq=`basename $0`
> > > +seqres=$RESULT_DIR/$seq
> > > +echo "QA output created by $seq"
> > > +
> > > +here=`pwd`
> > > +fio_config=`mktemp`
> > > +status=1	# failure is the default!
> > > +trap "_cleanup; exit \$status" 0 1 2 3 15
> > 
> > By removing the definition of $tmp, you are now dumping all
> > the temporary files the test harnes creates in /.
> 
> What temp files ? Yes we're sometimes using $tmp even though there
> is no obvious definition and if we want to rely on the existence of
> this variable we better define it as environment variable in 'check'
> script.

About 80% of the files in the repository use $tmp in some way.
And it's used all over the place in common/*, too. e.g mkfs
and check functions for storing output for parsing....

> It may be enough to simply add
>
> export tmp
> 
> to the 'check' script

Then everything uses the same tmp file prefix (i.e. the pid of the
check script) rather than a test specific pid so we lose out on
debugging capability there, not to mention that "rm -f $tmp*" in a
test (like the majority of tests do in their cleanup() routine) will
remove all the tmp files that the test harness needs to do it's
stuff....

> But regardless of this bug it does not affect this test in any way
> since it's not calling any of those functions and there are other
> tests that does not define $tmp as well.

Which points out a couple more problems with the test to me. It uses
SCRATCH_MNT without calling _scratch_mkfs - which uses $tmp when
FSTYP=xfs - and it doesn't call _scratch_mount, either, so it's
running on the underlying filesystem rather than the filesystem it
is supposed to test.

And because it uses _require_scratch(), then ./check will call
_check_scratch_fs() to check the filesystem, and if FSTYP=xfs then
_check_xfs_filesystem is called and that uses $tmp....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH v2] generic: concurrent IO test with mixed IO types

[Lukáš Czerner]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 5730 bytes --]

On Wed, 10 Jun 2015, Dave Chinner wrote:

> Date: Wed, 10 Jun 2015 21:11:53 +1000
> From: Dave Chinner <david@fromorbit.com>
> To: Lukáš Czerner <lczerner@redhat.com>
> Cc: Eryu Guan <eguan@redhat.com>, fstests@vger.kernel.org
> Subject: Re: [PATCH v2] generic: concurrent IO test with mixed IO types
> 
> On Wed, Jun 10, 2015 at 11:01:57AM +0200, Lukáš Czerner wrote:
> > On Wed, 10 Jun 2015, Dave Chinner wrote:
> > 
> > > Date: Wed, 10 Jun 2015 08:29:33 +1000
> > > From: Dave Chinner <david@fromorbit.com>
> > > To: Eryu Guan <eguan@redhat.com>
> > > Cc: fstests@vger.kernel.org, lczerner@redhat.com
> > > Subject: Re: [PATCH v2] generic: concurrent IO test with mixed IO types
> > > 
> > > On Mon, Jun 08, 2015 at 08:41:11PM +0800, Eryu Guan wrote:
> > > > Test concurrent buffered I/O, DIO, AIO, mmap I/O and splice I/O on the
> > > > same files.
> > > > 
> > > > Signed-off-by: Eryu Guan <eguan@redhat.com>
> > > > ---
> > > > 
> > > > This fio job file has been proven to be potent, it triggers WARNINGs on ext4
> > > > and xfs with 4.1-rc6 kernel.
> > > > 
> > > > ext4: WARNING: at fs/ext4/inode.c:1328
> > > > xfs: WARNING: CPU: 7 PID: 3090 at fs/xfs/xfs_file.c:726 xfs_file_dio_aio_write+0x176/0x2a8 [xfs]()
> > > > 
> > > > The ext4 issue should be fixed by Lukas's patch
> > > > ext4: fix reservation release on invalidatepage for delalloc fs
> > > > 
> > > > And it ever paniced kernel in mm code and hung xfs.
> > > > 
> > > > I reduced the numjobs and iodepth to reduce the test time(~25s on my test host)
> > > > and scale them by $LOAD_FACTOR. And it still could trigger the warning on ext4
> > > > and xfs with reduced workload.
> > > > 
> > > > v2:
> > > > - use mktemp to create tmp fio job file
> > > ....
> > > > +seq=`basename $0`
> > > > +seqres=$RESULT_DIR/$seq
> > > > +echo "QA output created by $seq"
> > > > +
> > > > +here=`pwd`
> > > > +fio_config=`mktemp`
> > > > +status=1	# failure is the default!
> > > > +trap "_cleanup; exit \$status" 0 1 2 3 15
> > > 
> > > By removing the definition of $tmp, you are now dumping all
> > > the temporary files the test harnes creates in /.
> > 
> > What temp files ? Yes we're sometimes using $tmp even though there
> > is no obvious definition and if we want to rely on the existence of
> > this variable we better define it as environment variable in 'check'
> > script.
> 
> About 80% of the files in the repository use $tmp in some way.
> And it's used all over the place in common/*, too. e.g mkfs
> and check functions for storing output for parsing....

Ah, come on. There are not that many functions using the $tmp and
some of them even removes the file immediately.

Removes immediatelly
common/attr	_acl_get_max
common/filter	_within_tolerance
common/rc	_check_generic_filesystem
common/rc	_check_xfs_filesystem
common/rc	_check_btrfs_filesystem

Does not remove
common/dump	_check_onl
		_create_dumpdir_stress_num
		_mk_fillconfig1
		_mk_fillconfig2
		_mk_fillconfig_ea
		_mk_fillconfig_xattr
		_mk_fillconfig_multi
		_append_dumpdir_fill
		_do_create_dump_symlinks
		_mk_symlink_config
		_do_dump_restore
		_ls_compare_sub
		_ls_nodate_compare_sub
		_create_dumpdir_stress_num
common/quota	_check_quota_usage
common/rc	_do

Most of the users are in the common/dump, however it's broken anyway
since on _cleanup() it does "rm -f $tmp.*" that'll remove all the
tmp file xfstests supposedly use.

> 
> > It may be enough to simply add
> >
> > export tmp
> > 
> > to the 'check' script
> 
> Then everything uses the same tmp file prefix (i.e. the pid of the
> check script) rather than a test specific pid so we lose out on
> debugging capability there, not to mention that "rm -f $tmp*" in a
> test (like the majority of tests do in their cleanup() routine) will
> remove all the tmp files that the test harness needs to do it's
> stuff....

What debugging ? First of all, all of the $tmp.* files are removed either
immediately or when the script ends and some of them are actually
dumped to the .full report anyway.

But regardless, the "rm -f $tmp*" called from the test _cleanup()
routine will _not_ remove tmp files that the test harness needs as
long as it defined it's own $tmp, so really using 'export' is the
right way for test that does not need their own tmp, or creates a
different name for they temp file, or directory.

Not even mentioning that there is not safety check to prevent us
from overwriting each other tmp files, or even temp files of
completely different unrelated process ( that's why we should use
mktemp).

> 
> > But regardless of this bug it does not affect this test in any way
> > since it's not calling any of those functions and there are other
> > tests that does not define $tmp as well.
> 
> Which points out a couple more problems with the test to me. It uses
> SCRATCH_MNT without calling _scratch_mkfs - which uses $tmp when
> FSTYP=xfs - and it doesn't call _scratch_mount, either, so it's
> running on the underlying filesystem rather than the filesystem it
> is supposed to test.

Right, that's a bug. Moreover if the test is the first test, or the
only test you run everything will be ok becuase 'check' script will
_scratch_mkfs and _scratch_mount for you for some reason ...

> 
> And because it uses _require_scratch(), then ./check will call
> _check_scratch_fs() to check the filesystem, and if FSTYP=xfs then
> _check_xfs_filesystem is called and that uses $tmp....

Not really. 'check' script calls _check_scratch_fs() and the $tmp is
defined there regardless of what the test itself is doing.

Regards,
-Lukas

> 
> Cheers,
> 
> Dave.
> 

Re: [PATCH v2] generic: concurrent IO test with mixed IO types

[Dave Chinner]

On Wed, Jun 10, 2015 at 02:22:55PM +0200, Lukáš Czerner wrote:
> On Wed, 10 Jun 2015, Dave Chinner wrote:
> 
> > Date: Wed, 10 Jun 2015 21:11:53 +1000
> > From: Dave Chinner <david@fromorbit.com>
> > To: Lukáš Czerner <lczerner@redhat.com>
> > Cc: Eryu Guan <eguan@redhat.com>, fstests@vger.kernel.org
> > Subject: Re: [PATCH v2] generic: concurrent IO test with mixed IO types
> > 
> > On Wed, Jun 10, 2015 at 11:01:57AM +0200, Lukáš Czerner wrote:
> > > On Wed, 10 Jun 2015, Dave Chinner wrote:
> > > 
> > > > Date: Wed, 10 Jun 2015 08:29:33 +1000
> > > > From: Dave Chinner <david@fromorbit.com>
> > > > To: Eryu Guan <eguan@redhat.com>
> > > > Cc: fstests@vger.kernel.org, lczerner@redhat.com
> > > > Subject: Re: [PATCH v2] generic: concurrent IO test with mixed IO types
> > > > 
> > > > On Mon, Jun 08, 2015 at 08:41:11PM +0800, Eryu Guan wrote:
> > > > > Test concurrent buffered I/O, DIO, AIO, mmap I/O and splice I/O on the
> > > > > same files.
> > > > > 
> > > > > Signed-off-by: Eryu Guan <eguan@redhat.com>
> > > > > ---
> > > > > 
> > > > > This fio job file has been proven to be potent, it triggers WARNINGs on ext4
> > > > > and xfs with 4.1-rc6 kernel.
> > > > > 
> > > > > ext4: WARNING: at fs/ext4/inode.c:1328
> > > > > xfs: WARNING: CPU: 7 PID: 3090 at fs/xfs/xfs_file.c:726 xfs_file_dio_aio_write+0x176/0x2a8 [xfs]()
> > > > > 
> > > > > The ext4 issue should be fixed by Lukas's patch
> > > > > ext4: fix reservation release on invalidatepage for delalloc fs
> > > > > 
> > > > > And it ever paniced kernel in mm code and hung xfs.
> > > > > 
> > > > > I reduced the numjobs and iodepth to reduce the test time(~25s on my test host)
> > > > > and scale them by $LOAD_FACTOR. And it still could trigger the warning on ext4
> > > > > and xfs with reduced workload.
> > > > > 
> > > > > v2:
> > > > > - use mktemp to create tmp fio job file
> > > > ....
> > > > > +seq=`basename $0`
> > > > > +seqres=$RESULT_DIR/$seq
> > > > > +echo "QA output created by $seq"
> > > > > +
> > > > > +here=`pwd`
> > > > > +fio_config=`mktemp`
> > > > > +status=1	# failure is the default!
> > > > > +trap "_cleanup; exit \$status" 0 1 2 3 15
> > > > 
> > > > By removing the definition of $tmp, you are now dumping all
> > > > the temporary files the test harnes creates in /.
> > > 
> > > What temp files ? Yes we're sometimes using $tmp even though there
> > > is no obvious definition and if we want to rely on the existence of
> > > this variable we better define it as environment variable in 'check'
> > > script.
> > 
> > About 80% of the files in the repository use $tmp in some way.
> > And it's used all over the place in common/*, too. e.g mkfs
> > and check functions for storing output for parsing....
> 
> Ah, come on. There are not that many functions using the $tmp and
> some of them even removes the file immediately.

$ git grep -lw "^tmp=" |wc -l
484
$ git grep -lw "\$tmp" |wc -l
417

That's roughly 80% of files that use $tmp /in some way/. The tests
are all suppose dto define $tmp the same way, and they are all
expected to clean up after themselves, thereby making it possible to
safely use $tmp in linrary functions, whether they clean up after
tehmselves or not.

You're spending way more time and effort than is necessary on basic
infrastructure that has worked for 15 years and, quite frankly,
*doesn't need fixing because it's not broken*.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH v2] generic: concurrent IO test with mixed IO types

[Lukáš Czerner]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 4121 bytes --]

On Wed, 10 Jun 2015, Dave Chinner wrote:

> Date: Wed, 10 Jun 2015 23:59:28 +1000
> From: Dave Chinner <david@fromorbit.com>
> To: Lukáš Czerner <lczerner@redhat.com>
> Cc: Eryu Guan <eguan@redhat.com>, fstests@vger.kernel.org
> Subject: Re: [PATCH v2] generic: concurrent IO test with mixed IO types
> 
> On Wed, Jun 10, 2015 at 02:22:55PM +0200, Lukáš Czerner wrote:
> > On Wed, 10 Jun 2015, Dave Chinner wrote:
> > 
> > > Date: Wed, 10 Jun 2015 21:11:53 +1000
> > > From: Dave Chinner <david@fromorbit.com>
> > > To: Lukáš Czerner <lczerner@redhat.com>
> > > Cc: Eryu Guan <eguan@redhat.com>, fstests@vger.kernel.org
> > > Subject: Re: [PATCH v2] generic: concurrent IO test with mixed IO types
> > > 
> > > On Wed, Jun 10, 2015 at 11:01:57AM +0200, Lukáš Czerner wrote:
> > > > On Wed, 10 Jun 2015, Dave Chinner wrote:
> > > > 
> > > > > Date: Wed, 10 Jun 2015 08:29:33 +1000
> > > > > From: Dave Chinner <david@fromorbit.com>
> > > > > To: Eryu Guan <eguan@redhat.com>
> > > > > Cc: fstests@vger.kernel.org, lczerner@redhat.com
> > > > > Subject: Re: [PATCH v2] generic: concurrent IO test with mixed IO types
> > > > > 
> > > > > On Mon, Jun 08, 2015 at 08:41:11PM +0800, Eryu Guan wrote:
> > > > > > Test concurrent buffered I/O, DIO, AIO, mmap I/O and splice I/O on the
> > > > > > same files.
> > > > > > 
> > > > > > Signed-off-by: Eryu Guan <eguan@redhat.com>
> > > > > > ---
> > > > > > 
> > > > > > This fio job file has been proven to be potent, it triggers WARNINGs on ext4
> > > > > > and xfs with 4.1-rc6 kernel.
> > > > > > 
> > > > > > ext4: WARNING: at fs/ext4/inode.c:1328
> > > > > > xfs: WARNING: CPU: 7 PID: 3090 at fs/xfs/xfs_file.c:726 xfs_file_dio_aio_write+0x176/0x2a8 [xfs]()
> > > > > > 
> > > > > > The ext4 issue should be fixed by Lukas's patch
> > > > > > ext4: fix reservation release on invalidatepage for delalloc fs
> > > > > > 
> > > > > > And it ever paniced kernel in mm code and hung xfs.
> > > > > > 
> > > > > > I reduced the numjobs and iodepth to reduce the test time(~25s on my test host)
> > > > > > and scale them by $LOAD_FACTOR. And it still could trigger the warning on ext4
> > > > > > and xfs with reduced workload.
> > > > > > 
> > > > > > v2:
> > > > > > - use mktemp to create tmp fio job file
> > > > > ....
> > > > > > +seq=`basename $0`
> > > > > > +seqres=$RESULT_DIR/$seq
> > > > > > +echo "QA output created by $seq"
> > > > > > +
> > > > > > +here=`pwd`
> > > > > > +fio_config=`mktemp`
> > > > > > +status=1	# failure is the default!
> > > > > > +trap "_cleanup; exit \$status" 0 1 2 3 15
> > > > > 
> > > > > By removing the definition of $tmp, you are now dumping all
> > > > > the temporary files the test harnes creates in /.
> > > > 
> > > > What temp files ? Yes we're sometimes using $tmp even though there
> > > > is no obvious definition and if we want to rely on the existence of
> > > > this variable we better define it as environment variable in 'check'
> > > > script.
> > > 
> > > About 80% of the files in the repository use $tmp in some way.
> > > And it's used all over the place in common/*, too. e.g mkfs
> > > and check functions for storing output for parsing....
> > 
> > Ah, come on. There are not that many functions using the $tmp and
> > some of them even removes the file immediately.
> 
> $ git grep -lw "^tmp=" |wc -l
> 484
> $ git grep -lw "\$tmp" |wc -l
> 417
> 
> That's roughly 80% of files that use $tmp /in some way/. The tests
> are all suppose dto define $tmp the same way, and they are all
> expected to clean up after themselves, thereby making it possible to
> safely use $tmp in linrary functions, whether they clean up after
> tehmselves or not.
> 
> You're spending way more time and effort than is necessary on basic
> infrastructure that has worked for 15 years and, quite frankly,
> *doesn't need fixing because it's not broken*.

Strongly disagree and patch like that just proves that it is
broken. But frankly you're right in that I am spending too much time
and effort arguing with you.

Regards,
-Lukas

[PATCH v3] generic: concurrent IO test with mixed IO types

[Eryu Guan]

Test concurrent buffered I/O, DIO, AIO, mmap I/O and splice I/O on the
same files.

Signed-off-by: Eryu Guan <eguan@redhat.com>
---

This fio job file has been proven to be potent, it triggers WARNINGs on ext4
and xfs with 4.1-rc6 kernel.

ext4: WARNING: at fs/ext4/inode.c:1328
xfs: WARNING: CPU: 7 PID: 3090 at fs/xfs/xfs_file.c:726 xfs_file_dio_aio_write+0x176/0x2a8 [xfs]()

The ext4 issue should be fixed by Lukas's patch
ext4: fix reservation release on invalidatepage for delalloc fs

And it ever paniced kernel in mm code and hung xfs.

I reduced the numjobs and iodepth to reduce the test time(~25s on my test host)
and scale them by $LOAD_FACTOR. And it still could trigger the warning on ext4
and xfs with reduced workload.

v2:
- use mktemp to create tmp fio job file
v3:
- initialize scratch dev and mount it correctly before test
- roll back to use /tmp/$$ as tmp

 tests/generic/090     | 122 ++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/generic/090.out |   2 +
 tests/generic/group   |   1 +
 3 files changed, 125 insertions(+)
 create mode 100755 tests/generic/090
 create mode 100644 tests/generic/090.out

diff --git a/tests/generic/090 b/tests/generic/090
new file mode 100755
index 0000000..e7cca52
--- /dev/null
+++ b/tests/generic/090
@@ -0,0 +1,122 @@
+#! /bin/bash
+# FS QA Test generic/090
+#
+# Concurrent mixed I/O (buffer I/O, aiodio, mmap, splice) on the same files
+#
+#-----------------------------------------------------------------------
+# Copyright (c) 2015 Red Hat Inc. All Rights Reserved.
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it would be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write the Free Software Foundation,
+# Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+#-----------------------------------------------------------------------
+#
+
+seq=`basename $0`
+seqres=$RESULT_DIR/$seq
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=/tmp/$$
+status=1	# failure is the default!
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+	cd /
+	rm -f $tmp.*
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+
+# real QA test starts here
+_supported_fs generic
+_supported_os Linux
+_require_scratch
+
+iodepth=$((16 * LOAD_FACTOR))
+iodepth_batch=$((8 * LOAD_FACTOR))
+numjobs=$((5 * LOAD_FACTOR))
+fio_config=$tmp.fio
+cat >$fio_config <<EOF
+[global]
+bs=8k
+iodepth=$iodepth
+iodepth_batch=$iodepth_batch
+randrepeat=1
+size=1m
+directory=$SCRATCH_MNT
+numjobs=$numjobs
+[job1]
+ioengine=sync
+bs=1k
+direct=1
+rw=randread
+filename=file1:file2
+[job2]
+ioengine=libaio
+rw=randwrite
+direct=1
+filename=file1:file2
+[job3]
+bs=1k
+ioengine=posixaio
+rw=randwrite
+direct=1
+filename=file1:file2
+[job4]
+ioengine=splice
+direct=1
+rw=randwrite
+filename=file1:file2
+[job5]
+bs=1k
+ioengine=sync
+rw=randread
+filename=file1:file2
+[job6]
+ioengine=posixaio
+rw=randwrite
+filename=file1:file2
+[job7]
+ioengine=splice
+rw=randwrite
+filename=file1:file2
+[job8]
+ioengine=mmap
+rw=randwrite
+bs=1k
+filename=file1:file2
+[job9]
+ioengine=mmap
+rw=randwrite
+direct=1
+filename=file1:file2
+EOF
+# with ioengine=mmap and direct=1, fio requires bs to be at least pagesize,
+# which is a fio built-in var.
+echo 'bs=$pagesize' >> $fio_config
+
+rm -f $seqres.full
+_require_fio $fio_config
+_scratch_mkfs >>$seqres.full 2>&1
+_scratch_mount
+
+echo "Silence is golden"
+$FIO_PROG $fio_config >>$seqres.full 2>&1
+
+# all done, expect no hang no oops no fs corruption,
+# _check_dmesg and _check_filesystems will do the check work for us
+status=0
+exit
diff --git a/tests/generic/090.out b/tests/generic/090.out
new file mode 100644
index 0000000..2b5100d
--- /dev/null
+++ b/tests/generic/090.out
@@ -0,0 +1,2 @@
+QA output created by 090
+Silence is golden
diff --git a/tests/generic/group b/tests/generic/group
index 0c8964c..2e534a5 100644
--- a/tests/generic/group
+++ b/tests/generic/group
@@ -92,6 +92,7 @@
 087 perms auto quick
 088 perms auto quick
 089 metadata auto
+090 auto rw stress
 091 rw auto quick
 092 auto quick prealloc
 093 attr cap udf auto
-- 
2.4.2

Re: [PATCH v3] generic: concurrent IO test with mixed IO types

[Dave Chinner]

On Thu, Jun 11, 2015 at 05:17:53PM +0800, Eryu Guan wrote:
> Test concurrent buffered I/O, DIO, AIO, mmap I/O and splice I/O on the
> same files.
> 
> Signed-off-by: Eryu Guan <eguan@redhat.com>
> ---
> 
> This fio job file has been proven to be potent, it triggers WARNINGs on ext4
> and xfs with 4.1-rc6 kernel.
> 
> ext4: WARNING: at fs/ext4/inode.c:1328
> xfs: WARNING: CPU: 7 PID: 3090 at fs/xfs/xfs_file.c:726 xfs_file_dio_aio_write+0x176/0x2a8 [xfs]()

Ok, so that warning is expected on XFS - that's intentional,
WARN_ONCE() output indicating a data coherency problem has occurred
because of the because the application is mixing buffered/mmap IO
with direct IO on the same file and direct Io has been unable to
cleanly invalidate the cache. i.e.  it's information to us
developers explaining why the user is complaining about data
corruption....

So this test is never going to pass on XFS unless you tell the test
harness to ignore the dmesg output...

> And it ever paniced kernel in mm code and hung xfs.

The "hung XFS" case will probably be the pipe mutex inversion
problem in the generic splice code. i.e.

.splice_read -> xfs_file_splice_read -> IOLOCK_SHARED ->
generic_file_splice_read -> splice_to_pipe -> pipe_lock()

vs:

iter_file_splice_write -> pipe_lock() -> vfs_iter_write ->
xfs_file_write_iter -> xfs_file_buffered_aio_write -> IOLOCK_EXCL

Can you confirm this? If so, there's not much we can actually do
about this - the recent big splice rewrite replaced the
pipe_lock/i_mutex inversion deadlock with a different pipe_lock
inversion deadlock....

> diff --git a/tests/generic/group b/tests/generic/group
> index 0c8964c..2e534a5 100644
> --- a/tests/generic/group
> +++ b/tests/generic/group
> @@ -92,6 +92,7 @@
>  087 perms auto quick
>  088 perms auto quick
>  089 metadata auto
> +090 auto rw stress

Hence I'm not sure "auto" is the correct group here. "dangerous" is
more likely because it is exercising a problem we can't fix and will
prevent the auto test group from making progress past this test.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH v3] generic: concurrent IO test with mixed IO types

[Eryu Guan]

On Thu, Jun 18, 2015 at 08:15:25AM +1000, Dave Chinner wrote:
> On Thu, Jun 11, 2015 at 05:17:53PM +0800, Eryu Guan wrote:
> > Test concurrent buffered I/O, DIO, AIO, mmap I/O and splice I/O on the
> > same files.
> > 
> > Signed-off-by: Eryu Guan <eguan@redhat.com>
> > ---
> > 
> > This fio job file has been proven to be potent, it triggers WARNINGs on ext4
> > and xfs with 4.1-rc6 kernel.
> > 
> > ext4: WARNING: at fs/ext4/inode.c:1328
> > xfs: WARNING: CPU: 7 PID: 3090 at fs/xfs/xfs_file.c:726 xfs_file_dio_aio_write+0x176/0x2a8 [xfs]()
> 
> Ok, so that warning is expected on XFS - that's intentional,
> WARN_ONCE() output indicating a data coherency problem has occurred
> because of the because the application is mixing buffered/mmap IO
> with direct IO on the same file and direct Io has been unable to
> cleanly invalidate the cache. i.e.  it's information to us
> developers explaining why the user is complaining about data
> corruption....
> 
> So this test is never going to pass on XFS unless you tell the test
> harness to ignore the dmesg output...

I can send a v4 to disable dmesg check if FSTYP is xfs, but that will
ignore any other WARNINGs/Oops too, which seems not ideal to me either.

I'm fine to go either way here(disable the dmesg check or not). But I
personally prefer changing the WARN_ON_ONCE to something like xfs_warn()
or xfs_warn_ratelimited() to give out the warning.

> 
> > And it ever paniced kernel in mm code and hung xfs.
> 
> The "hung XFS" case will probably be the pipe mutex inversion
> problem in the generic splice code. i.e.
> 
> .splice_read -> xfs_file_splice_read -> IOLOCK_SHARED ->
> generic_file_splice_read -> splice_to_pipe -> pipe_lock()
> 
> vs:
> 
> iter_file_splice_write -> pipe_lock() -> vfs_iter_write ->
> xfs_file_write_iter -> xfs_file_buffered_aio_write -> IOLOCK_EXCL
> 
> Can you confirm this? If so, there's not much we can actually do
> about this - the recent big splice rewrite replaced the
> pipe_lock/i_mutex inversion deadlock with a different pipe_lock
> inversion deadlock....

Yes, XFS deadlocks in the splice code with RHEL7.1 kernel but doesn't
deadlock with 4.1-rc[567] kernels (I only confirmed on these kernels
just now), so ...

> 
> > diff --git a/tests/generic/group b/tests/generic/group
> > index 0c8964c..2e534a5 100644
> > --- a/tests/generic/group
> > +++ b/tests/generic/group
> > @@ -92,6 +92,7 @@
> >  087 perms auto quick
> >  088 perms auto quick
> >  089 metadata auto
> > +090 auto rw stress
> 
> Hence I'm not sure "auto" is the correct group here. "dangerous" is
> more likely because it is exercising a problem we can't fix and will
> prevent the auto test group from making progress past this test.

I think the auto group should be fine here.

Thanks,
Eryu

Re: [PATCH v3] generic: concurrent IO test with mixed IO types

[Dave Chinner]

On Thu, Jun 18, 2015 at 11:04:31AM +0800, Eryu Guan wrote:
> On Thu, Jun 18, 2015 at 08:15:25AM +1000, Dave Chinner wrote:
> > On Thu, Jun 11, 2015 at 05:17:53PM +0800, Eryu Guan wrote:
> > > Test concurrent buffered I/O, DIO, AIO, mmap I/O and splice I/O on the
> > > same files.
> > > 
> > > Signed-off-by: Eryu Guan <eguan@redhat.com>
> > > ---
> > > 
> > > This fio job file has been proven to be potent, it triggers WARNINGs on ext4
> > > and xfs with 4.1-rc6 kernel.
> > > 
> > > ext4: WARNING: at fs/ext4/inode.c:1328
> > > xfs: WARNING: CPU: 7 PID: 3090 at fs/xfs/xfs_file.c:726 xfs_file_dio_aio_write+0x176/0x2a8 [xfs]()
> > 
> > Ok, so that warning is expected on XFS - that's intentional,
> > WARN_ONCE() output indicating a data coherency problem has occurred
> > because of the because the application is mixing buffered/mmap IO
> > with direct IO on the same file and direct Io has been unable to
> > cleanly invalidate the cache. i.e.  it's information to us
> > developers explaining why the user is complaining about data
> > corruption....
> > 
> > So this test is never going to pass on XFS unless you tell the test
> > harness to ignore the dmesg output...
> 
> I can send a v4 to disable dmesg check if FSTYP is xfs, but that will
> ignore any other WARNINGs/Oops too, which seems not ideal to me either.

Such conditional output issues are dealt with by adding filters to
the output....

> I'm fine to go either way here(disable the dmesg check or not). But I
> personally prefer changing the WARN_ON_ONCE to something like xfs_warn()
> or xfs_warn_ratelimited() to give out the warning.

History tells us that such warnings get ignored and not reported,
and we lose lots of hair before we find out that the bug reporter
thought it "wasn't important" and so "didn't include it" in any of
the bug reports.

Data coherency problems are important enough that we WARN_ON_ONCE is
justified - it's something we need to know about sooner rather than
later, and it's something that application developers also need to
be aware of. They won't notice an xfs warning in the logs, but they
will notice abort() or some other syslog monitor telling them
there's been a kernel warning....

> > > And it ever paniced kernel in mm code and hung xfs.
> > 
> > The "hung XFS" case will probably be the pipe mutex inversion
> > problem in the generic splice code. i.e.
> > 
> > .splice_read -> xfs_file_splice_read -> IOLOCK_SHARED ->
> > generic_file_splice_read -> splice_to_pipe -> pipe_lock()
> > 
> > vs:
> > 
> > iter_file_splice_write -> pipe_lock() -> vfs_iter_write ->
> > xfs_file_write_iter -> xfs_file_buffered_aio_write -> IOLOCK_EXCL
> > 
> > Can you confirm this? If so, there's not much we can actually do
> > about this - the recent big splice rewrite replaced the
> > pipe_lock/i_mutex inversion deadlock with a different pipe_lock
> > inversion deadlock....
> 
> Yes, XFS deadlocks in the splice code with RHEL7.1 kernel but doesn't
> deadlock with 4.1-rc[567] kernels (I only confirmed on these kernels
> just now), so ...

Oh, ok, so the current upstream is fine; RHEL7 has the
pre-write_iter rewrite of the splice code, so the deadlock must be
of the older variety. We can ignore that, then.

> > > diff --git a/tests/generic/group b/tests/generic/group
> > > index 0c8964c..2e534a5 100644
> > > --- a/tests/generic/group
> > > +++ b/tests/generic/group
> > > @@ -92,6 +92,7 @@
> > >  087 perms auto quick
> > >  088 perms auto quick
> > >  089 metadata auto
> > > +090 auto rw stress
> > 
> > Hence I'm not sure "auto" is the correct group here. "dangerous" is
> > more likely because it is exercising a problem we can't fix and will
> > prevent the auto test group from making progress past this test.
> 
> I think the auto group should be fine here.

If it doesn't fail on current upstream kernels, that will be fine.
If it fails, and there is no likely resolution of the failure in the
forseeable future, then it does not belong in the auto group.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com