[PATCH] xfs: Fix WARN_ON(delalloc) in xfs_vm_releasepage()

[PATCH] xfs: Fix WARN_ON(delalloc) in xfs_vm_releasepage()

[Jan Kara]

When a dirty page is truncated from a file but reclaim gets to it before
truncate_inode_pages(), we hit WARN_ON(delalloc) in
xfs_vm_releasepage(). This is because reclaim tries to write the page,
xfs_vm_writepage() just bails out (leaving page clean) and thus reclaim
thinks it can continue and calls xfs_vm_releasepage() on page with dirty
buffers.

Fix the issue by redirtying the page in xfs_vm_writepage(). This makes
reclaim stop reclaiming the page and also logically it keeps page in a
more consistent state where page with dirty buffers has PageDirty set.

Signed-off-by: Jan Kara <jack@suse.cz>
---
 fs/xfs/xfs_aops.c |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/fs/xfs/xfs_aops.c b/fs/xfs/xfs_aops.c
index 5f707e5..3244c98 100644
--- a/fs/xfs/xfs_aops.c
+++ b/fs/xfs/xfs_aops.c
@@ -953,13 +953,13 @@ xfs_vm_writepage(
 		unsigned offset_into_page = offset & (PAGE_CACHE_SIZE - 1);
 
 		/*
-		 * Just skip the page if it is fully outside i_size, e.g. due
-		 * to a truncate operation that is in progress.
+		 * Skip the page if it is fully outside i_size, e.g. due to a
+		 * truncate operation that is in progress. We must redirty the
+		 * page so that reclaim stops reclaiming it. Otherwise
+		 * xfs_vm_releasepage() is called on it and gets confused.
 		 */
-		if (page->index >= end_index + 1 || offset_into_page == 0) {
-			unlock_page(page);
-			return 0;
-		}
+		if (page->index >= end_index + 1 || offset_into_page == 0)
+			goto redirty;
 
 		/*
 		 * The page straddles i_size.  It must be zeroed out on each
-- 
1.7.1

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] xfs: Fix WARN_ON(delalloc) in xfs_vm_releasepage()

[Carlos Maiolino]

On Thu, Mar 14, 2013 at 02:30:54PM +0100, Jan Kara wrote:
> When a dirty page is truncated from a file but reclaim gets to it before
> truncate_inode_pages(), we hit WARN_ON(delalloc) in
> xfs_vm_releasepage(). This is because reclaim tries to write the page,
> xfs_vm_writepage() just bails out (leaving page clean) and thus reclaim
> thinks it can continue and calls xfs_vm_releasepage() on page with dirty
> buffers.
> 
> Fix the issue by redirtying the page in xfs_vm_writepage(). This makes
> reclaim stop reclaiming the page and also logically it keeps page in a
> more consistent state where page with dirty buffers has PageDirty set.
> 
> Signed-off-by: Jan Kara <jack@suse.cz>
> ---
>  fs/xfs/xfs_aops.c |   12 ++++++------
>  1 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/fs/xfs/xfs_aops.c b/fs/xfs/xfs_aops.c
> index 5f707e5..3244c98 100644
> --- a/fs/xfs/xfs_aops.c
> +++ b/fs/xfs/xfs_aops.c
> @@ -953,13 +953,13 @@ xfs_vm_writepage(
>  		unsigned offset_into_page = offset & (PAGE_CACHE_SIZE - 1);
>  
>  		/*
> -		 * Just skip the page if it is fully outside i_size, e.g. due
> -		 * to a truncate operation that is in progress.
> +		 * Skip the page if it is fully outside i_size, e.g. due to a
> +		 * truncate operation that is in progress. We must redirty the
> +		 * page so that reclaim stops reclaiming it. Otherwise
> +		 * xfs_vm_releasepage() is called on it and gets confused.
>  		 */
> -		if (page->index >= end_index + 1 || offset_into_page == 0) {
> -			unlock_page(page);
> -			return 0;
> -		}
> +		if (page->index >= end_index + 1 || offset_into_page == 0)
> +			goto redirty;
>  
>  		/*
>  		 * The page straddles i_size.  It must be zeroed out on each
> -- 
> 1.7.1
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs

Looks Good,
Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>

-- 
Carlos

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] xfs: Fix WARN_ON(delalloc) in xfs_vm_releasepage()

[Ben Myers]

Hi Jan,

On Thu, Mar 14, 2013 at 02:30:54PM +0100, Jan Kara wrote:
> When a dirty page is truncated from a file but reclaim gets to it before
> truncate_inode_pages(), we hit WARN_ON(delalloc) in
> xfs_vm_releasepage(). This is because reclaim tries to write the page,
> xfs_vm_writepage() just bails out (leaving page clean) and thus reclaim
> thinks it can continue and calls xfs_vm_releasepage() on page with dirty
> buffers.
> 
> Fix the issue by redirtying the page in xfs_vm_writepage(). This makes
> reclaim stop reclaiming the page and also logically it keeps page in a
> more consistent state where page with dirty buffers has PageDirty set.

Was there an easy way to reproduce this?  I'm testing and reviewing this now
and it might help.

Thanks much,
       Ben

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] xfs: Fix WARN_ON(delalloc) in xfs_vm_releasepage()

[Jan Kara]

[-- Attachment #1: Type: text/plain, Size: 1141 bytes --]

On Fri 15-03-13 15:52:14, Ben Myers wrote:
> Hi Jan,
> 
> On Thu, Mar 14, 2013 at 02:30:54PM +0100, Jan Kara wrote:
> > When a dirty page is truncated from a file but reclaim gets to it before
> > truncate_inode_pages(), we hit WARN_ON(delalloc) in
> > xfs_vm_releasepage(). This is because reclaim tries to write the page,
> > xfs_vm_writepage() just bails out (leaving page clean) and thus reclaim
> > thinks it can continue and calls xfs_vm_releasepage() on page with dirty
> > buffers.
> > 
> > Fix the issue by redirtying the page in xfs_vm_writepage(). This makes
> > reclaim stop reclaiming the page and also logically it keeps page in a
> > more consistent state where page with dirty buffers has PageDirty set.
> 
> Was there an easy way to reproduce this?  I'm testing and reviewing this now
> and it might help.
  I used scripts/run-bash-shared-mapping.sh from the attached tarball - it
fires up several processes beating a file with mmap accesses while
truncating the file and memory stressing the machine. I presume fsx with
some memhog could trigger the issue as well.

									Honza
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

[-- Attachment #2: ext3-tools.tar.gz --]
[-- Type: application/x-compressed-tar, Size: 96501 bytes --]

[-- Attachment #3: Type: text/plain, Size: 121 bytes --]

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] xfs: Fix WARN_ON(delalloc) in xfs_vm_releasepage()

[Dave Chinner]

On Mon, Mar 18, 2013 at 05:05:52PM +0100, Jan Kara wrote:
> On Fri 15-03-13 15:52:14, Ben Myers wrote:
> > Hi Jan,
> > 
> > On Thu, Mar 14, 2013 at 02:30:54PM +0100, Jan Kara wrote:
> > > When a dirty page is truncated from a file but reclaim gets to it before
> > > truncate_inode_pages(), we hit WARN_ON(delalloc) in
> > > xfs_vm_releasepage(). This is because reclaim tries to write the page,
> > > xfs_vm_writepage() just bails out (leaving page clean) and thus reclaim
> > > thinks it can continue and calls xfs_vm_releasepage() on page with dirty
> > > buffers.
> > > 
> > > Fix the issue by redirtying the page in xfs_vm_writepage(). This makes
> > > reclaim stop reclaiming the page and also logically it keeps page in a
> > > more consistent state where page with dirty buffers has PageDirty set.
> > 
> > Was there an easy way to reproduce this?  I'm testing and reviewing this now
> > and it might help.
>   I used scripts/run-bash-shared-mapping.sh from the attached tarball - it
> fires up several processes beating a file with mmap accesses while
> truncating the file and memory stressing the machine. I presume fsx with
> some memhog could trigger the issue as well.

I have seen fsx trip this occasionally. The problem is that it was
never reliable because of the fact that other memory pressure had to
be generated at the same time....

Is the above script something you could turn into an xfstest (I
haven't looked at the script yet)?

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] xfs: Fix WARN_ON(delalloc) in xfs_vm_releasepage()

[Jan Kara]

On Tue 19-03-13 14:47:59, Dave Chinner wrote:
> On Mon, Mar 18, 2013 at 05:05:52PM +0100, Jan Kara wrote:
> > On Fri 15-03-13 15:52:14, Ben Myers wrote:
> > > Hi Jan,
> > > 
> > > On Thu, Mar 14, 2013 at 02:30:54PM +0100, Jan Kara wrote:
> > > > When a dirty page is truncated from a file but reclaim gets to it before
> > > > truncate_inode_pages(), we hit WARN_ON(delalloc) in
> > > > xfs_vm_releasepage(). This is because reclaim tries to write the page,
> > > > xfs_vm_writepage() just bails out (leaving page clean) and thus reclaim
> > > > thinks it can continue and calls xfs_vm_releasepage() on page with dirty
> > > > buffers.
> > > > 
> > > > Fix the issue by redirtying the page in xfs_vm_writepage(). This makes
> > > > reclaim stop reclaiming the page and also logically it keeps page in a
> > > > more consistent state where page with dirty buffers has PageDirty set.
> > > 
> > > Was there an easy way to reproduce this?  I'm testing and reviewing this now
> > > and it might help.
> >   I used scripts/run-bash-shared-mapping.sh from the attached tarball - it
> > fires up several processes beating a file with mmap accesses while
> > truncating the file and memory stressing the machine. I presume fsx with
> > some memhog could trigger the issue as well.
> 
> I have seen fsx trip this occasionally. The problem is that it was
> never reliable because of the fact that other memory pressure had to
> be generated at the same time....
> 
> Is the above script something you could turn into an xfstest (I
> haven't looked at the script yet)?
  Yes, that should be doable. I'll try combining fsx and usemem whether
that will be enough to trigger the race.

								Honza
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] xfs: Fix WARN_ON(delalloc) in xfs_vm_releasepage()

[Dave Chinner]

On Fri, Mar 15, 2013 at 03:52:14PM -0500, Ben Myers wrote:
> Hi Jan,
> 
> On Thu, Mar 14, 2013 at 02:30:54PM +0100, Jan Kara wrote:
> > When a dirty page is truncated from a file but reclaim gets to it before
> > truncate_inode_pages(), we hit WARN_ON(delalloc) in
> > xfs_vm_releasepage(). This is because reclaim tries to write the page,
> > xfs_vm_writepage() just bails out (leaving page clean) and thus reclaim
> > thinks it can continue and calls xfs_vm_releasepage() on page with dirty
> > buffers.
> > 
> > Fix the issue by redirtying the page in xfs_vm_writepage(). This makes
> > reclaim stop reclaiming the page and also logically it keeps page in a
> > more consistent state where page with dirty buffers has PageDirty set.
> 
> Was there an easy way to reproduce this?  I'm testing and reviewing this now
> and it might help.

This patch has been confirmed to fix the report generated by
the DB workload I mentioned in another part of the thread. Hence I
think it should be good to go.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] xfs: Fix WARN_ON(delalloc) in xfs_vm_releasepage()

[Ben Myers]

On Thu, Mar 21, 2013 at 12:37:33PM +1100, Dave Chinner wrote:
> On Fri, Mar 15, 2013 at 03:52:14PM -0500, Ben Myers wrote:
> > Hi Jan,
> > 
> > On Thu, Mar 14, 2013 at 02:30:54PM +0100, Jan Kara wrote:
> > > When a dirty page is truncated from a file but reclaim gets to it before
> > > truncate_inode_pages(), we hit WARN_ON(delalloc) in
> > > xfs_vm_releasepage(). This is because reclaim tries to write the page,
> > > xfs_vm_writepage() just bails out (leaving page clean) and thus reclaim
> > > thinks it can continue and calls xfs_vm_releasepage() on page with dirty
> > > buffers.
> > > 
> > > Fix the issue by redirtying the page in xfs_vm_writepage(). This makes
> > > reclaim stop reclaiming the page and also logically it keeps page in a
> > > more consistent state where page with dirty buffers has PageDirty set.
> > 
> > Was there an easy way to reproduce this?  I'm testing and reviewing this now
> > and it might help.
> 
> This patch has been confirmed to fix the report generated by
> the DB workload I mentioned in another part of the thread. Hence I
> think it should be good to go.

Agreed, Thanks.

-Ben

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] xfs: Fix WARN_ON(delalloc) in xfs_vm_releasepage()

[Dave Chinner]

On Thu, Mar 14, 2013 at 02:30:54PM +0100, Jan Kara wrote:
> When a dirty page is truncated from a file but reclaim gets to it before
> truncate_inode_pages(), we hit WARN_ON(delalloc) in
> xfs_vm_releasepage(). This is because reclaim tries to write the page,
> xfs_vm_writepage() just bails out (leaving page clean) and thus reclaim
> thinks it can continue and calls xfs_vm_releasepage() on page with dirty
> buffers.
> 
> Fix the issue by redirtying the page in xfs_vm_writepage(). This makes
> reclaim stop reclaiming the page and also logically it keeps page in a
> more consistent state where page with dirty buffers has PageDirty set.
> 
> Signed-off-by: Jan Kara <jack@suse.cz>

Definitely look s like a problem taht needs fixing, and it seems to
me to be the correct fix.

I got a bug report recently about this warning being triggered,
but the total report was the warning and a workload description
of "XFS produced a warning at some point during a 28 hour long DB
benchmark." Not particularly helpful, but this looks like a
potential cause. Thanks!

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] xfs: Fix WARN_ON(delalloc) in xfs_vm_releasepage()

[Ben Myers]

On Thu, Mar 14, 2013 at 02:30:54PM +0100, Jan Kara wrote:
> When a dirty page is truncated from a file but reclaim gets to it before
> truncate_inode_pages(), we hit WARN_ON(delalloc) in
> xfs_vm_releasepage(). This is because reclaim tries to write the page,
> xfs_vm_writepage() just bails out (leaving page clean) and thus reclaim
> thinks it can continue and calls xfs_vm_releasepage() on page with dirty
> buffers.
> 
> Fix the issue by redirtying the page in xfs_vm_writepage(). This makes
> reclaim stop reclaiming the page and also logically it keeps page in a
> more consistent state where page with dirty buffers has PageDirty set.
> 
> Signed-off-by: Jan Kara <jack@suse.cz>

Applied.

Regards,
	Ben

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs