[PATCH 0/3] fstests: various fixes

[PATCH 0/3] fstests: various fixes

[Darrick J. Wong]

Hi all,

Fix the mixed buffered/directio filtering function to pick up the new
locations of the iomap directio code, fix the bogus description in
xfs/504, and reduce the time generic/561 waits for duperemove to finish.

If you're going to start using this mess, you probably ought to just
pull from my git trees, which are linked below.

This is an extraordinary way to destroy everything.  Enjoy!
Comments and questions are, as always, welcome.

--D

fstests git tree:
https://git.kernel.org/cgit/linux/kernel/git/djwong/xfstests-dev.git/log/?h=random-fixes

[PATCH 1/3] common: filter aiodio dmesg after fs/iomap.c to fs/iomap/ move

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

Since the iomap code are moving to fs/iomap/ we have to add new entries
to the aiodio dmesg filter to reflect this.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 common/filter |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)


diff --git a/common/filter b/common/filter
index ed082d24..26fc2132 100644
--- a/common/filter
+++ b/common/filter
@@ -555,6 +555,7 @@ _filter_aiodio_dmesg()
 	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.*"
+	local warn10="WARNING:.*fs/iomap/direct-io\.c:.*iomap_dio_actor.*"
 	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#" \
@@ -563,7 +564,8 @@ _filter_aiodio_dmesg()
 	    -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#$warn9#Intentional warnings in dio_complete#"
+	    -e "s#$warn9#Intentional warnings in dio_complete#" \
+	    -e "s#$warn10#Intentional warnings in iomap_dio_actor#"
 }
 
 # We generate assert related WARNINGs on purpose and make sure test doesn't fail

Re: [PATCH 1/3] common: filter aiodio dmesg after fs/iomap.c to fs/iomap/ move

[Dave Chinner]

On Tue, Jul 23, 2019 at 09:12:54PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Since the iomap code are moving to fs/iomap/ we have to add new entries
> to the aiodio dmesg filter to reflect this.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  common/filter |    4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> 
> diff --git a/common/filter b/common/filter
> index ed082d24..26fc2132 100644
> --- a/common/filter
> +++ b/common/filter
> @@ -555,6 +555,7 @@ _filter_aiodio_dmesg()
>  	local warn7="WARNING:.*fs/iomap\.c:.*iomap_dio_actor.*"
>  	local warn8="WARNING:.*fs/iomap\.c:.*iomap_dio_complete.*"

There's two different warnings that need capturing here.

>  	local warn9="WARNING:.*fs/direct-io\.c:.*dio_complete.*"
> +	local warn10="WARNING:.*fs/iomap/direct-io\.c:.*iomap_dio_actor.*"
>  	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#" \
> @@ -563,7 +564,8 @@ _filter_aiodio_dmesg()
>  	    -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#$warn9#Intentional warnings in dio_complete#"
> +	    -e "s#$warn9#Intentional warnings in dio_complete#" \
> +	    -e "s#$warn10#Intentional warnings in iomap_dio_actor#"

Why not just change the regex in warn7/warn8 just to catch anything
under fs/iomap rather than explictly specifying fs/iomap.c?

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH 1/3] common: filter aiodio dmesg after fs/iomap.c to fs/iomap/ move

[Darrick J. Wong]

On Thu, Jul 25, 2019 at 09:21:17AM +1000, Dave Chinner wrote:
> On Tue, Jul 23, 2019 at 09:12:54PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > Since the iomap code are moving to fs/iomap/ we have to add new entries
> > to the aiodio dmesg filter to reflect this.
> > 
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > ---
> >  common/filter |    4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > 
> > diff --git a/common/filter b/common/filter
> > index ed082d24..26fc2132 100644
> > --- a/common/filter
> > +++ b/common/filter
> > @@ -555,6 +555,7 @@ _filter_aiodio_dmesg()
> >  	local warn7="WARNING:.*fs/iomap\.c:.*iomap_dio_actor.*"
> >  	local warn8="WARNING:.*fs/iomap\.c:.*iomap_dio_complete.*"
> 
> There's two different warnings that need capturing here.

DOH, missed that.

> >  	local warn9="WARNING:.*fs/direct-io\.c:.*dio_complete.*"
> > +	local warn10="WARNING:.*fs/iomap/direct-io\.c:.*iomap_dio_actor.*"
> >  	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#" \
> > @@ -563,7 +564,8 @@ _filter_aiodio_dmesg()
> >  	    -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#$warn9#Intentional warnings in dio_complete#"
> > +	    -e "s#$warn9#Intentional warnings in dio_complete#" \
> > +	    -e "s#$warn10#Intentional warnings in iomap_dio_actor#"
> 
> Why not just change the regex in warn7/warn8 just to catch anything
> under fs/iomap rather than explictly specifying fs/iomap.c?

Ok I'll do that.

--D

> Cheers,
> 
> Dave.
> -- 
> Dave Chinner
> david@fromorbit.com

[PATCH v2 1/3] common: filter aiodio dmesg after fs/iomap.c to fs/iomap/ move

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

Since the iomap code are moving to fs/iomap/ we have to add new entries
to the aiodio dmesg filter to reflect this.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
v2: fix all the iomap regexes
---
 common/filter |    9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/common/filter b/common/filter
index ed082d24..2e32ab10 100644
--- a/common/filter
+++ b/common/filter
@@ -550,10 +550,10 @@ _filter_aiodio_dmesg()
 	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 warn5="WARNING:.*fs/iomap.*:.*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.*"
+	local warn7="WARNING:.*fs/iomap.*:.*iomap_dio_actor.*"
+	local warn8="WARNING:.*fs/iomap.*:.*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#" \
@@ -563,7 +563,8 @@ _filter_aiodio_dmesg()
 	    -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#$warn9#Intentional warnings in dio_complete#"
+	    -e "s#$warn9#Intentional warnings in dio_complete#" \
+	    -e "s#$warn10#Intentional warnings in iomap_dio_actor#"
 }
 
 # We generate assert related WARNINGs on purpose and make sure test doesn't fail

Re: [PATCH v2 1/3] common: filter aiodio dmesg after fs/iomap.c to fs/iomap/ move

[Eryu Guan]

On Thu, Jul 25, 2019 at 11:03:30AM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Since the iomap code are moving to fs/iomap/ we have to add new entries
> to the aiodio dmesg filter to reflect this.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
> v2: fix all the iomap regexes
> ---
>  common/filter |    9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/common/filter b/common/filter
> index ed082d24..2e32ab10 100644
> --- a/common/filter
> +++ b/common/filter
> @@ -550,10 +550,10 @@ _filter_aiodio_dmesg()
>  	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 warn5="WARNING:.*fs/iomap.*:.*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.*"
> +	local warn7="WARNING:.*fs/iomap.*:.*iomap_dio_actor.*"
> +	local warn8="WARNING:.*fs/iomap.*:.*iomap_dio_complete.*"

I don't think we need new filters anymore, as commit 5a9d929d6e13
("iomap: report collisions between directio and buffered writes to
userspace") replaced the WARN_ON with a pr_crit(). These filters are
there only for old kernels.

Thanks,
Eryu

>  	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#" \
> @@ -563,7 +563,8 @@ _filter_aiodio_dmesg()
>  	    -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#$warn9#Intentional warnings in dio_complete#"
> +	    -e "s#$warn9#Intentional warnings in dio_complete#" \
> +	    -e "s#$warn10#Intentional warnings in iomap_dio_actor#"
>  }
>  
>  # We generate assert related WARNINGs on purpose and make sure test doesn't fail

Re: [PATCH v2 1/3] common: filter aiodio dmesg after fs/iomap.c to fs/iomap/ move

[Darrick J. Wong]

On Sun, Jul 28, 2019 at 07:30:36PM +0800, Eryu Guan wrote:
> On Thu, Jul 25, 2019 at 11:03:30AM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > Since the iomap code are moving to fs/iomap/ we have to add new entries
> > to the aiodio dmesg filter to reflect this.
> > 
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > ---
> > v2: fix all the iomap regexes
> > ---
> >  common/filter |    9 +++++----
> >  1 file changed, 5 insertions(+), 4 deletions(-)
> > 
> > diff --git a/common/filter b/common/filter
> > index ed082d24..2e32ab10 100644
> > --- a/common/filter
> > +++ b/common/filter
> > @@ -550,10 +550,10 @@ _filter_aiodio_dmesg()
> >  	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 warn5="WARNING:.*fs/iomap.*:.*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.*"
> > +	local warn7="WARNING:.*fs/iomap.*:.*iomap_dio_actor.*"
> > +	local warn8="WARNING:.*fs/iomap.*:.*iomap_dio_complete.*"
> 
> I don't think we need new filters anymore, as commit 5a9d929d6e13
> ("iomap: report collisions between directio and buffered writes to
> userspace") replaced the WARN_ON with a pr_crit(). These filters are
> there only for old kernels.

Aaaaahh... but I /did/ write this patch because I kept hitting a WARNING
somewhere in the iomap directio code, and you know what?  It's one of the
warnings about a bogus iomap type in iomap_dio_actor.

I /think/ this is what happens when a buffered write sneaks in and
creates a delalloc reservation after the directio write has zapped the
page cache but before it actually starts iterating extents.
Consequently iomap_dio_actor sees the delalloc extent and WARNs.

Will have to recheck this, but maybe the kernel needs to deploy that
helper that 5a9d929d6e13 for that case.

--D

> Thanks,
> Eryu
> 
> >  	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#" \
> > @@ -563,7 +563,8 @@ _filter_aiodio_dmesg()
> >  	    -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#$warn9#Intentional warnings in dio_complete#"
> > +	    -e "s#$warn9#Intentional warnings in dio_complete#" \
> > +	    -e "s#$warn10#Intentional warnings in iomap_dio_actor#"
> >  }
> >  
> >  # We generate assert related WARNINGs on purpose and make sure test doesn't fail

Re: [PATCH v2 1/3] common: filter aiodio dmesg after fs/iomap.c to fs/iomap/ move

[Darrick J. Wong]

On Mon, Jul 29, 2019 at 05:55:06PM -0700, Darrick J. Wong wrote:
> On Sun, Jul 28, 2019 at 07:30:36PM +0800, Eryu Guan wrote:
> > On Thu, Jul 25, 2019 at 11:03:30AM -0700, Darrick J. Wong wrote:
> > > From: Darrick J. Wong <darrick.wong@oracle.com>
> > > 
> > > Since the iomap code are moving to fs/iomap/ we have to add new entries
> > > to the aiodio dmesg filter to reflect this.
> > > 
> > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > > ---
> > > v2: fix all the iomap regexes
> > > ---
> > >  common/filter |    9 +++++----
> > >  1 file changed, 5 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/common/filter b/common/filter
> > > index ed082d24..2e32ab10 100644
> > > --- a/common/filter
> > > +++ b/common/filter
> > > @@ -550,10 +550,10 @@ _filter_aiodio_dmesg()
> > >  	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 warn5="WARNING:.*fs/iomap.*:.*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.*"
> > > +	local warn7="WARNING:.*fs/iomap.*:.*iomap_dio_actor.*"
> > > +	local warn8="WARNING:.*fs/iomap.*:.*iomap_dio_complete.*"
> > 
> > I don't think we need new filters anymore, as commit 5a9d929d6e13
> > ("iomap: report collisions between directio and buffered writes to
> > userspace") replaced the WARN_ON with a pr_crit(). These filters are
> > there only for old kernels.
> 
> Aaaaahh... but I /did/ write this patch because I kept hitting a WARNING
> somewhere in the iomap directio code, and you know what?  It's one of the
> warnings about a bogus iomap type in iomap_dio_actor.
> 
> I /think/ this is what happens when a buffered write sneaks in and
> creates a delalloc reservation after the directio write has zapped the
> page cache but before it actually starts iterating extents.
> Consequently iomap_dio_actor sees the delalloc extent and WARNs.
> 
> Will have to recheck this, but maybe the kernel needs to deploy that
> helper that 5a9d929d6e13 for that case.

Aha, I found it again.  The patch fixes failures in generic/446 when a
directio write through iomap encounters a delalloc extent and triggers
the WARN_ON_ONCE at the bottom of iomap_dio_actor:

WARNING: CPU: 2 PID: 1710922 at fs/iomap/direct-io.c:383 iomap_dio_actor+0x144/0x1a0

This can happen if a buffered write and a directio write race to fill a
hole and the buffered write manages to stuff a delalloc reservation into
the data mapping after the dio write has cleared the page cache.

We don't need the dio_warn_stale_pagecache() warning here because we
fail the direct write and therefore do not write anything to disk.

--D

> 
> --D
> 
> > Thanks,
> > Eryu
> > 
> > >  	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#" \
> > > @@ -563,7 +563,8 @@ _filter_aiodio_dmesg()
> > >  	    -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#$warn9#Intentional warnings in dio_complete#"
> > > +	    -e "s#$warn9#Intentional warnings in dio_complete#" \
> > > +	    -e "s#$warn10#Intentional warnings in iomap_dio_actor#"
> > >  }
> > >  
> > >  # We generate assert related WARNINGs on purpose and make sure test doesn't fail

Re: [PATCH v2 1/3] common: filter aiodio dmesg after fs/iomap.c to fs/iomap/ move

[Eryu Guan]

On Tue, Aug 06, 2019 at 06:44:54PM -0700, Darrick J. Wong wrote:
> On Mon, Jul 29, 2019 at 05:55:06PM -0700, Darrick J. Wong wrote:
> > On Sun, Jul 28, 2019 at 07:30:36PM +0800, Eryu Guan wrote:
> > > On Thu, Jul 25, 2019 at 11:03:30AM -0700, Darrick J. Wong wrote:
> > > > From: Darrick J. Wong <darrick.wong@oracle.com>
> > > > 
> > > > Since the iomap code are moving to fs/iomap/ we have to add new entries
> > > > to the aiodio dmesg filter to reflect this.
> > > > 
> > > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > > > ---
> > > > v2: fix all the iomap regexes
> > > > ---
> > > >  common/filter |    9 +++++----
> > > >  1 file changed, 5 insertions(+), 4 deletions(-)
> > > > 
> > > > diff --git a/common/filter b/common/filter
> > > > index ed082d24..2e32ab10 100644
> > > > --- a/common/filter
> > > > +++ b/common/filter
> > > > @@ -550,10 +550,10 @@ _filter_aiodio_dmesg()
> > > >  	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 warn5="WARNING:.*fs/iomap.*:.*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.*"
> > > > +	local warn7="WARNING:.*fs/iomap.*:.*iomap_dio_actor.*"
> > > > +	local warn8="WARNING:.*fs/iomap.*:.*iomap_dio_complete.*"
> > > 
> > > I don't think we need new filters anymore, as commit 5a9d929d6e13
> > > ("iomap: report collisions between directio and buffered writes to
> > > userspace") replaced the WARN_ON with a pr_crit(). These filters are
> > > there only for old kernels.
> > 
> > Aaaaahh... but I /did/ write this patch because I kept hitting a WARNING
> > somewhere in the iomap directio code, and you know what?  It's one of the
> > warnings about a bogus iomap type in iomap_dio_actor.
> > 
> > I /think/ this is what happens when a buffered write sneaks in and
> > creates a delalloc reservation after the directio write has zapped the
> > page cache but before it actually starts iterating extents.
> > Consequently iomap_dio_actor sees the delalloc extent and WARNs.
> > 
> > Will have to recheck this, but maybe the kernel needs to deploy that
> > helper that 5a9d929d6e13 for that case.
> 
> Aha, I found it again.  The patch fixes failures in generic/446 when a
> directio write through iomap encounters a delalloc extent and triggers
> the WARN_ON_ONCE at the bottom of iomap_dio_actor:
> 
> WARNING: CPU: 2 PID: 1710922 at fs/iomap/direct-io.c:383 iomap_dio_actor+0x144/0x1a0

Yeah, I can hit it too when run generic/446 on XFS as well.

> 
> This can happen if a buffered write and a directio write race to fill a
> hole and the buffered write manages to stuff a delalloc reservation into
> the data mapping after the dio write has cleared the page cache.
> 
> We don't need the dio_warn_stale_pagecache() warning here because we
> fail the direct write and therefore do not write anything to disk.

IMHO, the v1 patch makes more sense in this case, as iomap_dio_actor is
the only place that could generate to-be-filtered warning now, warnings
in iomap_dio_rw and iomap_dio_complete are suppressed by commit
5a9d929d6e13.

Thanks,
Eryu

> 
> --D
> 
> > 
> > --D
> > 
> > > Thanks,
> > > Eryu
> > > 
> > > >  	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#" \
> > > > @@ -563,7 +563,8 @@ _filter_aiodio_dmesg()
> > > >  	    -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#$warn9#Intentional warnings in dio_complete#"
> > > > +	    -e "s#$warn9#Intentional warnings in dio_complete#" \
> > > > +	    -e "s#$warn10#Intentional warnings in iomap_dio_actor#"
> > > >  }
> > > >  
> > > >  # We generate assert related WARNINGs on purpose and make sure test doesn't fail

[PATCH 2/3] xfs/504: fix bogus test description

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

Fix the description of this test to reflect what it actually checks.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 tests/xfs/504 |    8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)


diff --git a/tests/xfs/504 b/tests/xfs/504
index cc6088d1..c6435117 100755
--- a/tests/xfs/504
+++ b/tests/xfs/504
@@ -4,10 +4,10 @@
 #
 # FS QA Test No. 504
 #
-# Create a directory with multiple filenames that all appear the same
-# (in unicode, anyway) but point to different inodes.  In theory all
-# Linux filesystems should allow this (filenames are a sequence of
-# arbitrary bytes) even if the user implications are horrifying.
+# Create a filesystem label with emoji and confusing unicode characters
+# to make sure that these special things actually work on xfs.  In
+# theory it should allow this (labels are a sequence of arbitrary bytes)
+# even if the user implications are horrifying.
 #
 seq=`basename "$0"`
 seqres="$RESULT_DIR/$seq"

[PATCH 3/3] generic/561: kill duperemove after sleep_time

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

generic/561 can take a very long time to run on XFS (45+ minutes)
because it kicks off fsstress and a lot of duperemove processes, waits
50 seconds, and then waits for the duperemove processes to finish.
duperemove, however, fights with fsstress for file locks and can take
a very long time to make even a single pass over the filesystem and
exit, which means the test just takes forever to run.

Once we've decided to tear down the duperemove processes let's just send
them SIGINT and then wait for them to exit.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 tests/generic/561 |    1 +
 1 file changed, 1 insertion(+)


diff --git a/tests/generic/561 b/tests/generic/561
index c11d5bfa..2f3eff3c 100755
--- a/tests/generic/561
+++ b/tests/generic/561
@@ -47,6 +47,7 @@ function end_test()
 	# stop duperemove running
 	if [ -e $dupe_run ]; then
 		rm -f $dupe_run
+		kill -INT $dedup_pids
 		wait $dedup_pids
 	fi
 

[PATCH 4/3] generic/506: mount scratch fs before testing for prjquota presence

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

On XFS, the _require_prjquota helper takes a path to a block device,
but (unintuitively) requires the block device to be mounted for the
detection to work properly.  Fix the detection code in generic/506.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 common/quota      |    3 ++-
 tests/generic/506 |    2 ++
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/common/quota b/common/quota
index f19f81a1..8a22f815 100644
--- a/common/quota
+++ b/common/quota
@@ -68,7 +68,8 @@ _require_xfs_quota_foreign()
 }
 
 #
-# checks that the project quota support in the kernel is enabled.
+# Checks that the project quota support in the kernel is enabled.
+# The device must be mounted for detection to work properly.
 #
 _require_prjquota()
 {
diff --git a/tests/generic/506 b/tests/generic/506
index 7002c00c..e8d0ca24 100755
--- a/tests/generic/506
+++ b/tests/generic/506
@@ -51,7 +51,9 @@ _require_scratch_shutdown
 
 _scratch_mkfs >/dev/null 2>&1
 _require_metadata_journaling $SCRATCH_DEV
+_scratch_mount
 _require_prjquota $SCRATCH_DEV
+_scratch_unmount
 
 testfile=$SCRATCH_MNT/testfile
 

[PATCH 5/3] various: disable quotas before running test

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

For all the tests which require that quotas be disabled, remove the
quota mount options before mounting the scratch filesystem.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 tests/generic/384 |    4 ++--
 tests/xfs/030     |    2 ++
 tests/xfs/033     |    2 ++
 tests/xfs/065     |    2 ++
 4 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/tests/generic/384 b/tests/generic/384
index b7c940d7..33098a38 100755
--- a/tests/generic/384
+++ b/tests/generic/384
@@ -37,8 +37,8 @@ _require_quota
 _require_xfs_quota_foreign
 _require_xfs_io_command "chproj"
 
-# we can't run with group quotas
-_exclude_scratch_mount_option "gquota" "grpquota"
+# we can't run with group quotas (on v4 xfs); the mount options for group
+# quotas will be filtered out by _qmount_option below.
 
 dir=$SCRATCH_MNT/project
 
diff --git a/tests/xfs/030 b/tests/xfs/030
index 5ed99628..10854c8a 100755
--- a/tests/xfs/030
+++ b/tests/xfs/030
@@ -28,6 +28,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 . ./common/rc
 . ./common/filter
 . ./common/repair
+. ./common/quota
 
 # nuke the superblock, AGI, AGF, AGFL; then try repair the damage
 #
@@ -65,6 +66,7 @@ if [ $? -ne 0 ]		# probably don't have a big enough scratch
 then
 	_notrun "SCRATCH_DEV too small, results would be non-deterministic"
 else
+	_qmount_option noquota
 	_scratch_mount
 	src/feature -U $SCRATCH_DEV && \
 		_notrun "UQuota are enabled, test needs controlled sb recovery"
diff --git a/tests/xfs/033 b/tests/xfs/033
index 5af0aefc..75b44f38 100755
--- a/tests/xfs/033
+++ b/tests/xfs/033
@@ -28,6 +28,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 . ./common/rc
 . ./common/filter
 . ./common/repair
+. ./common/quota
 
 # nuke the root, rt bitmap, and rt summary inodes
 # 
@@ -80,6 +81,7 @@ _link_out_file_named $seqfull.out "$FEATURES"
 	sed -e 's/ //g' -e 's/^/export /'`
 
 # check we won't get any quota inodes setup on mount
+_qmount_option noquota
 _scratch_mount
 src/feature -U $SCRATCH_DEV && \
 	_notrun "UQuota are enabled, test needs controlled sb recovery"
diff --git a/tests/xfs/065 b/tests/xfs/065
index f09bd947..b1533666 100755
--- a/tests/xfs/065
+++ b/tests/xfs/065
@@ -29,6 +29,7 @@ _cleanup()
 . ./common/rc
 . ./common/filter
 . ./common/dump
+. ./common/quota
 
 #
 # list recursively the directory
@@ -57,6 +58,7 @@ _require_scratch
 # so don't run it
 #
 _scratch_mkfs_xfs >> $seqres.full
+_qmount_option noquota
 _scratch_mount
 $here/src/feature -U $SCRATCH_DEV && \
 	_notrun "UQuota enabled, test needs controlled xfsdump output"

[PATCH 6/3] xfs/033: filter out root inode nlink repair

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

A couple of releases ago, xfs_repair was patched to set the root inode
link count correctly when messing around with lost inodes.  However, the
old xfs_repair remains in the golden output, so remove it and filter the
line so that we don't cause 'new' regressions on old software.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 tests/xfs/033             |   12 ++++++++++--
 tests/xfs/033.out.crc     |    2 --
 tests/xfs/033.out.default |    2 --
 3 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/tests/xfs/033 b/tests/xfs/033
index 75b44f38..0ce67a9c 100755
--- a/tests/xfs/033
+++ b/tests/xfs/033
@@ -91,9 +91,17 @@ src/feature -P $SCRATCH_DEV && \
 	_notrun "PQuota are enabled, test needs controlled sb recovery"
 _scratch_unmount
 
+# We fixed some bugs in xfs_repair that caused us to have to reset the inode
+# link counts on the root inode twice.  That wasn't related to what this test
+# is checking, so remove the nlink reset line from the golden output and filter
+# old xfsprogs to avoid introducing new regressions.
+filter_repair() {
+	sed -e '/resetting inode INO nlinks from 1 to 2/d'
+}
+
 # rootino, rbmino, and rsumino are now set (lets blow em away!)
-_check_root_inos 0
-_check_root_inos -1 | _filter_bad_ids
+_check_root_inos 0 | filter_repair
+_check_root_inos -1 | filter_repair | _filter_bad_ids
 
 # success, all done
 status=0
diff --git a/tests/xfs/033.out.crc b/tests/xfs/033.out.crc
index 2ab4c432..594060f0 100644
--- a/tests/xfs/033.out.crc
+++ b/tests/xfs/033.out.crc
@@ -30,7 +30,6 @@ reinitializing root directory
         - traversal finished ...
         - moving disconnected inodes to lost+found ...
 Phase 7 - verify and correct link counts...
-resetting inode INO nlinks from 1 to 2
 done
 Corrupting rt bitmap inode - setting bits to 0
 Wrote X.XXKb (value 0x0)
@@ -125,7 +124,6 @@ reinitializing root directory
         - traversal finished ...
         - moving disconnected inodes to lost+found ...
 Phase 7 - verify and correct link counts...
-resetting inode INO nlinks from 1 to 2
 done
 Corrupting rt bitmap inode - setting bits to -1
 Wrote X.XXKb (value 0xffffffff)
diff --git a/tests/xfs/033.out.default b/tests/xfs/033.out.default
index 68bc7810..be297e5a 100644
--- a/tests/xfs/033.out.default
+++ b/tests/xfs/033.out.default
@@ -29,7 +29,6 @@ reinitializing root directory
         - traversal finished ...
         - moving disconnected inodes to lost+found ...
 Phase 7 - verify and correct link counts...
-resetting inode INO nlinks from 1 to 2
 done
 Corrupting rt bitmap inode - setting bits to 0
 Wrote X.XXKb (value 0x0)
@@ -122,7 +121,6 @@ reinitializing root directory
         - traversal finished ...
         - moving disconnected inodes to lost+found ...
 Phase 7 - verify and correct link counts...
-resetting inode INO nlinks from 1 to 2
 done
 Corrupting rt bitmap inode - setting bits to -1
 Wrote X.XXKb (value 0xffffffff)

[PATCH 7/3] xfs/194: unmount forced v4 fs during cleanup

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

Unmount the V4 filesystem we forcibly created to run this test during
test cleanup so that the post-test wrapup checks won't try to remount
the filesystem with different MOUNT_OPTIONS (specifically, the ones
that get screened out by _force_xfsv4_mount_options) and fail.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 tests/xfs/194 |    1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/xfs/194 b/tests/xfs/194
index 3e186528..1f46d403 100755
--- a/tests/xfs/194
+++ b/tests/xfs/194
@@ -18,6 +18,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 _cleanup()
 {
     cd /
+    _scratch_unmount
     rm -f $tmp.*
 }
 

Re: [PATCH 7/3] xfs/194: unmount forced v4 fs during cleanup

[Dave Chinner]

On Wed, Jul 24, 2019 at 08:56:56AM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Unmount the V4 filesystem we forcibly created to run this test during
> test cleanup so that the post-test wrapup checks won't try to remount
> the filesystem with different MOUNT_OPTIONS (specifically, the ones
> that get screened out by _force_xfsv4_mount_options) and fail.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  tests/xfs/194 |    1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/tests/xfs/194 b/tests/xfs/194
> index 3e186528..1f46d403 100755
> --- a/tests/xfs/194
> +++ b/tests/xfs/194
> @@ -18,6 +18,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
>  _cleanup()
>  {
>      cd /
> +    _scratch_unmount

Comment as to why this is necessary here so we don't go and remove
it because unmounting in cleanup should generally be unnecessary....

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

Re: [PATCH 7/3] xfs/194: unmount forced v4 fs during cleanup

[Darrick J. Wong]

On Thu, Jul 25, 2019 at 09:22:59AM +1000, Dave Chinner wrote:
> On Wed, Jul 24, 2019 at 08:56:56AM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > Unmount the V4 filesystem we forcibly created to run this test during
> > test cleanup so that the post-test wrapup checks won't try to remount
> > the filesystem with different MOUNT_OPTIONS (specifically, the ones
> > that get screened out by _force_xfsv4_mount_options) and fail.
> > 
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > ---
> >  tests/xfs/194 |    1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/tests/xfs/194 b/tests/xfs/194
> > index 3e186528..1f46d403 100755
> > --- a/tests/xfs/194
> > +++ b/tests/xfs/194
> > @@ -18,6 +18,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
> >  _cleanup()
> >  {
> >      cd /
> > +    _scratch_unmount
> 
> Comment as to why this is necessary here so we don't go and remove
> it because unmounting in cleanup should generally be unnecessary....

Ok.

--D

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

[PATCH v2 7/3] xfs/194: unmount forced v4 fs during cleanup

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

Unmount the V4 filesystem we forcibly created to run this test during
test cleanup so that the post-test wrapup checks won't try to remount
the filesystem with different MOUNT_OPTIONS (specifically, the ones
that get screened out by _force_xfsv4_mount_options) and fail.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
v2: add a comment explaining why we unmount the scratch device
---
 tests/xfs/194 |    5 +++++
 1 file changed, 5 insertions(+)

diff --git a/tests/xfs/194 b/tests/xfs/194
index 3e186528..9001a146 100755
--- a/tests/xfs/194
+++ b/tests/xfs/194
@@ -18,6 +18,11 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 _cleanup()
 {
     cd /
+    # Unmount the V4 filesystem we forcibly created to run this test so that
+    # the post-test wrapup checks won't try to remount the filesystem with
+    # different MOUNT_OPTIONS (specifically, the ones that get screened out by
+    # _force_xfsv4_mount_options) and fail.
+    _scratch_unmount
     rm -f $tmp.*
 }