[PATCH 0/3] generalize AIO/DIO dmesg filter

[PATCH 0/3] generalize AIO/DIO dmesg filter

[Ilya Dryomov]

With upstream commit 332391a9935d ("fs: Fix page cache inconsistency
when mixing buffered and AIO DIO"), we need to whitelist dio_complete()
for ext4 pretty much everywhere iomap_dio_complete() and friends are
whitelisted for xfs.

Thanks,

                Ilya


Ilya Dryomov (3):
  common/rc: move _filter_xfs_dmesg() to common/rc
  common/rc: turn _filter_xfs_dmesg() into _filter_aiodio_dmesg()
  generic/036,208: whitelist [iomap_]dio_complete() WARNs

 common/rc         | 25 +++++++++++++++++++++++++
 common/xfs        | 22 ----------------------
 tests/generic/036 |  2 ++
 tests/generic/095 |  7 ++-----
 tests/generic/208 |  2 ++
 tests/generic/224 |  9 +++------
 tests/generic/247 |  9 +++------
 tests/generic/446 |  9 +++------
 8 files changed, 40 insertions(+), 45 deletions(-)

-- 
2.4.3

[PATCH 1/3] common/rc: move _filter_xfs_dmesg() to common/rc

[Ilya Dryomov]

Signed-off-by: Ilya Dryomov <idryomov@gmail.com>
---
 common/rc  | 23 +++++++++++++++++++++++
 common/xfs | 22 ----------------------
 2 files changed, 23 insertions(+), 22 deletions(-)

diff --git a/common/rc b/common/rc
index 53bbb1187f81..e15c8fc27405 100644
--- a/common/rc
+++ b/common/rc
@@ -2027,6 +2027,29 @@ _run_aiodio()
     return $status
 }
 
+# xfs generates WARNINGs on purpose when applications mix buffered/mmap IO with
+# direct IO on the same file. This is a helper for _check_dmesg() to filter out
+# such warnings.
+_filter_xfs_dmesg()
+{
+	local warn1="WARNING:.*fs/xfs/xfs_file\.c:.*xfs_file_dio_aio_write.*"
+	local warn2="WARNING:.*fs/xfs/xfs_file\.c:.*xfs_file_dio_aio_read.*"
+	local warn3="WARNING:.*fs/xfs/xfs_file\.c:.*xfs_file_read_iter.*"
+	local warn4="WARNING:.*fs/xfs/xfs_file\.c:.*xfs_file_aio_read.*"
+	local warn5="WARNING:.*fs/iomap\.c:.*iomap_dio_rw.*"
+	local warn6="WARNING:.*fs/xfs/xfs_aops\.c:.*__xfs_get_blocks.*"
+	local warn7="WARNING:.*fs/iomap\.c:.*iomap_dio_actor.*"
+	local warn8="WARNING:.*fs/iomap\.c:.*iomap_dio_complete.*"
+	sed -e "s#$warn1#Intentional warnings in xfs_file_dio_aio_write#" \
+	    -e "s#$warn2#Intentional warnings in xfs_file_dio_aio_read#" \
+	    -e "s#$warn3#Intentional warnings in xfs_file_read_iter#" \
+	    -e "s#$warn4#Intentional warnings in xfs_file_aio_read#" \
+	    -e "s#$warn5#Intentional warnings in iomap_dio_rw#" \
+	    -e "s#$warn6#Intentional warnings in __xfs_get_blocks#" \
+	    -e "s#$warn7#Intentional warnings in iomap_dio_actor#" \
+	    -e "s#$warn8#Intentional warnings in iomap_dio_complete#"
+}
+
 # this test requires y2038 sysfs switch and filesystem
 # timestamp ranges support.
 _require_y2038()
diff --git a/common/xfs b/common/xfs
index 93eb50394984..d4fef941d321 100644
--- a/common/xfs
+++ b/common/xfs
@@ -599,25 +599,3 @@ _require_no_xfs_debug()
 		_notrun "Require XFS built without CONFIG_XFS_DEBUG"
 	fi
 }
-# xfs generates WARNINGs on purpose when applications mix buffered/mmap IO with
-# direct IO on the same file. This is a helper for _check_dmesg() to filter out
-# such warnings.
-_filter_xfs_dmesg()
-{
-	local warn1="WARNING:.*fs/xfs/xfs_file\.c:.*xfs_file_dio_aio_write.*"
-	local warn2="WARNING:.*fs/xfs/xfs_file\.c:.*xfs_file_dio_aio_read.*"
-	local warn3="WARNING:.*fs/xfs/xfs_file\.c:.*xfs_file_read_iter.*"
-	local warn4="WARNING:.*fs/xfs/xfs_file\.c:.*xfs_file_aio_read.*"
-	local warn5="WARNING:.*fs/iomap\.c:.*iomap_dio_rw.*"
-	local warn6="WARNING:.*fs/xfs/xfs_aops\.c:.*__xfs_get_blocks.*"
-	local warn7="WARNING:.*fs/iomap\.c:.*iomap_dio_actor.*"
-	local warn8="WARNING:.*fs/iomap\.c:.*iomap_dio_complete.*"
-	sed -e "s#$warn1#Intentional warnings in xfs_file_dio_aio_write#" \
-	    -e "s#$warn2#Intentional warnings in xfs_file_dio_aio_read#" \
-	    -e "s#$warn3#Intentional warnings in xfs_file_read_iter#" \
-	    -e "s#$warn4#Intentional warnings in xfs_file_aio_read#" \
-	    -e "s#$warn5#Intentional warnings in iomap_dio_rw#" \
-	    -e "s#$warn6#Intentional warnings in __xfs_get_blocks#" \
-	    -e "s#$warn7#Intentional warnings in iomap_dio_actor#" \
-	    -e "s#$warn8#Intentional warnings in iomap_dio_complete#"
-}
-- 
2.4.3

[PATCH 2/3] common/rc: turn _filter_xfs_dmesg() into _filter_aiodio_dmesg()

[Ilya Dryomov]

With upstream commit 332391a9935d ("fs: Fix page cache inconsistency
when mixing buffered and AIO DIO"), ext4 (and probably other non-iomap
based filesystems) need a _check_dmesg() whitelist entry for a new WARN
in dio_complete() -- generic/095 and 247 trigger a dio_complete() splat
pretty reliably for me.

Add dio_complete() entry to _filter_xfs_dmesg(), rename it and use it
unconditionally in generic/095, 224, 247 and 446.

Signed-off-by: Ilya Dryomov <idryomov@gmail.com>
---
 common/rc         | 8 +++++---
 tests/generic/095 | 7 ++-----
 tests/generic/224 | 9 +++------
 tests/generic/247 | 9 +++------
 tests/generic/446 | 9 +++------
 5 files changed, 16 insertions(+), 26 deletions(-)

diff --git a/common/rc b/common/rc
index e15c8fc27405..03d2a9cd2484 100644
--- a/common/rc
+++ b/common/rc
@@ -2027,10 +2027,10 @@ _run_aiodio()
     return $status
 }
 
-# xfs generates WARNINGs on purpose when applications mix buffered/mmap IO with
+# We generate WARNINGs on purpose when applications mix buffered/mmap IO with
 # direct IO on the same file. This is a helper for _check_dmesg() to filter out
 # such warnings.
-_filter_xfs_dmesg()
+_filter_aiodio_dmesg()
 {
 	local warn1="WARNING:.*fs/xfs/xfs_file\.c:.*xfs_file_dio_aio_write.*"
 	local warn2="WARNING:.*fs/xfs/xfs_file\.c:.*xfs_file_dio_aio_read.*"
@@ -2040,6 +2040,7 @@ _filter_xfs_dmesg()
 	local warn6="WARNING:.*fs/xfs/xfs_aops\.c:.*__xfs_get_blocks.*"
 	local warn7="WARNING:.*fs/iomap\.c:.*iomap_dio_actor.*"
 	local warn8="WARNING:.*fs/iomap\.c:.*iomap_dio_complete.*"
+	local warn9="WARNING:.*fs/direct-io\.c:.*dio_complete.*"
 	sed -e "s#$warn1#Intentional warnings in xfs_file_dio_aio_write#" \
 	    -e "s#$warn2#Intentional warnings in xfs_file_dio_aio_read#" \
 	    -e "s#$warn3#Intentional warnings in xfs_file_read_iter#" \
@@ -2047,7 +2048,8 @@ _filter_xfs_dmesg()
 	    -e "s#$warn5#Intentional warnings in iomap_dio_rw#" \
 	    -e "s#$warn6#Intentional warnings in __xfs_get_blocks#" \
 	    -e "s#$warn7#Intentional warnings in iomap_dio_actor#" \
-	    -e "s#$warn8#Intentional warnings in iomap_dio_complete#"
+	    -e "s#$warn8#Intentional warnings in iomap_dio_complete#" \
+	    -e "s#$warn9#Intentional warnings in dio_complete#"
 }
 
 # this test requires y2038 sysfs switch and filesystem
diff --git a/tests/generic/095 b/tests/generic/095
index 050215d41700..fd4f13fd0a1b 100755
--- a/tests/generic/095
+++ b/tests/generic/095
@@ -126,10 +126,7 @@ $FIO_PROG $fio_config >>$seqres.full 2>&1
 # umount before checking dmesg in case umount triggers any WARNING or Oops
 _scratch_unmount
 
-if [ "$FSTYP" == "xfs" ]; then
-	_check_dmesg _filter_xfs_dmesg
-else
-	_check_dmesg
-fi
+_check_dmesg _filter_aiodio_dmesg
+
 status=$?
 exit
diff --git a/tests/generic/224 b/tests/generic/224
index 29b1c05d06df..6b3a81536aa8 100755
--- a/tests/generic/224
+++ b/tests/generic/224
@@ -83,13 +83,10 @@ done
 wait
 echo "*** Silence is golden ***"
 
-# unmount and check dmesg, filtering out expected XFS warnings about mixed
+# unmount and check dmesg, filtering out expected warnings about mixed
 # direct and buffered I/O
 _scratch_unmount
-if [ "$FSTYP" == "xfs" ]; then
-	_check_dmesg _filter_xfs_dmesg
-else
-	_check_dmesg
-fi
+_check_dmesg _filter_aiodio_dmesg
+
 status=$?
 exit
diff --git a/tests/generic/247 b/tests/generic/247
index aac7078045ee..0614100cd7a8 100755
--- a/tests/generic/247
+++ b/tests/generic/247
@@ -80,13 +80,10 @@ wait
 
 echo "Silence is golden."
 
-# unmount and check dmesg, filtering out expected XFS warnings about mixed
+# unmount and check dmesg, filtering out expected warnings about mixed
 # mmap/dio
 _test_unmount
-if [ "$FSTYP" == "xfs" ]; then
-	_check_dmesg _filter_xfs_dmesg
-else
-	_check_dmesg
-fi
+_check_dmesg _filter_aiodio_dmesg
+
 status=$?
 exit
diff --git a/tests/generic/446 b/tests/generic/446
index 6a1b69aa2edc..01228322911e 100755
--- a/tests/generic/446
+++ b/tests/generic/446
@@ -79,13 +79,10 @@ wait $dread_pid > /dev/null 2>&1
 
 echo "Silence is golden"
 
-# check dmesg, filtering out expected XFS warnings about mixed mmap/dio
+# check dmesg, filtering out expected warnings about mixed mmap/dio
 # and umount first in case umount triggers warnings
 _scratch_unmount
-if [ "$FSTYP" == "xfs" ]; then
-	_check_dmesg _filter_xfs_dmesg
-else
-	_check_dmesg
-fi
+_check_dmesg _filter_aiodio_dmesg
+
 status=$?
 exit
-- 
2.4.3

[PATCH 3/3] generic/036,208: whitelist [iomap_]dio_complete() WARNs

[Ilya Dryomov]

These tests appear to mix buffered and O_DIRECT I/O; easily triggered
on both xfs and ext4.

Signed-off-by: Ilya Dryomov <idryomov@gmail.com>
---
 tests/generic/036 | 2 ++
 tests/generic/208 | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/tests/generic/036 b/tests/generic/036
index 0615dad45e37..27e9916bc13a 100755
--- a/tests/generic/036
+++ b/tests/generic/036
@@ -48,4 +48,6 @@ _require_test
 
 _run_aiodio aio-dio-fcntl-race
 
+_check_dmesg _filter_aiodio_dmesg
+
 exit $status
diff --git a/tests/generic/208 b/tests/generic/208
index f57e2bc76f8b..1768e4220e91 100755
--- a/tests/generic/208
+++ b/tests/generic/208
@@ -50,4 +50,6 @@ rm -f $seqres.full
 
 _run_aiodio aio-dio-invalidate-failure
 
+_check_dmesg _filter_aiodio_dmesg
+
 exit $status
-- 
2.4.3

Re: [PATCH 1/3] common/rc: move _filter_xfs_dmesg() to common/rc

[Dave Chinner]

On Tue, Oct 10, 2017 at 04:54:56PM +0200, Ilya Dryomov wrote:
> Signed-off-by: Ilya Dryomov <idryomov@gmail.com>
> ---
>  common/rc  | 23 +++++++++++++++++++++++
>  common/xfs | 22 ----------------------
>  2 files changed, 23 insertions(+), 22 deletions(-)
> 
> diff --git a/common/rc b/common/rc
> index 53bbb1187f81..e15c8fc27405 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -2027,6 +2027,29 @@ _run_aiodio()
>      return $status
>  }
>  
> +# xfs generates WARNINGs on purpose when applications mix buffered/mmap IO with
> +# direct IO on the same file. This is a helper for _check_dmesg() to filter out
> +# such warnings.
> +_filter_xfs_dmesg()

it's a filter, so belongs in common/filter.

-Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH 0/3] generalize AIO/DIO dmesg filter

[Eryu Guan]

On Tue, Oct 10, 2017 at 04:54:55PM +0200, Ilya Dryomov wrote:
> With upstream commit 332391a9935d ("fs: Fix page cache inconsistency
> when mixing buffered and AIO DIO"), we need to whitelist dio_complete()
> for ext4 pretty much everywhere iomap_dio_complete() and friends are
> whitelisted for xfs.

Thanks for fixing this! I agreed with Dave that we should move the
filter to common/filter file, but that's easy to do and I can fix it up
at commit time.

I was thinking adding this filter to _check_dmesg unconditionally, so we
don't have to the filter case by case. But I guess it's better to be
more careful, in case we miss any real bug.

Thanks,
Eryu

Re: [PATCH 0/3] generalize AIO/DIO dmesg filter

[Ilya Dryomov]

On Thu, Oct 12, 2017 at 6:25 AM, Eryu Guan <eguan@redhat.com> wrote:
> On Tue, Oct 10, 2017 at 04:54:55PM +0200, Ilya Dryomov wrote:
>> With upstream commit 332391a9935d ("fs: Fix page cache inconsistency
>> when mixing buffered and AIO DIO"), we need to whitelist dio_complete()
>> for ext4 pretty much everywhere iomap_dio_complete() and friends are
>> whitelisted for xfs.
>
> Thanks for fixing this! I agreed with Dave that we should move the
> filter to common/filter file, but that's easy to do and I can fix it up
> at commit time.

I already fixed it -- just waiting for more comments before reposting.

>
> I was thinking adding this filter to _check_dmesg unconditionally, so we
> don't have to the filter case by case. But I guess it's better to be
> more careful, in case we miss any real bug.

Definitely!

Thanks,

                Ilya

Re: [PATCH 0/3] generalize AIO/DIO dmesg filter

[Eryu Guan]

On Thu, Oct 12, 2017 at 09:19:21AM +0200, Ilya Dryomov wrote:
> On Thu, Oct 12, 2017 at 6:25 AM, Eryu Guan <eguan@redhat.com> wrote:
> > On Tue, Oct 10, 2017 at 04:54:55PM +0200, Ilya Dryomov wrote:
> >> With upstream commit 332391a9935d ("fs: Fix page cache inconsistency
> >> when mixing buffered and AIO DIO"), we need to whitelist dio_complete()
> >> for ext4 pretty much everywhere iomap_dio_complete() and friends are
> >> whitelisted for xfs.
> >
> > Thanks for fixing this! I agreed with Dave that we should move the
> > filter to common/filter file, but that's easy to do and I can fix it up
> > at commit time.
> 
> I already fixed it -- just waiting for more comments before reposting.

Then please repost and I'll take your v2 :)

Thanks,
Eryu

> 
> >
> > I was thinking adding this filter to _check_dmesg unconditionally, so we
> > don't have to the filter case by case. But I guess it's better to be
> > more careful, in case we miss any real bug.
> 
> Definitely!
> 
> Thanks,
> 
>                 Ilya