All of lore.kernel.org
 help / color / mirror / Atom feed
From: Brian Foster <bfoster@redhat.com>
To: Dave Chinner <david@fromorbit.com>
Cc: clm@fb.com, xfs@oss.sgi.com
Subject: Re: [PATCH 2/6] xfs: don't zero partial page cache pages during O_DIRECT writes
Date: Thu, 21 Aug 2014 09:08:47 -0400	[thread overview]
Message-ID: <20140821130847.GC64112@bfoster.bfoster> (raw)
In-Reply-To: <1408597754-13526-3-git-send-email-david@fromorbit.com>

On Thu, Aug 21, 2014 at 03:09:10PM +1000, Dave Chinner wrote:
> From: Chris Mason <clm@fb.com>
> 
> xfs is using truncate_pagecache_range to invalidate the page cache
> during DIO reads.  This is different from the other filesystems who
> only invalidate pages during DIO writes.
> 
> truncate_pagecache_range is meant to be used when we are freeing the
> underlying data structs from disk, so it will zero any partial
> ranges in the page.  This means a DIO read can zero out part of the
> page cache page, and it is possible the page will stay in cache.
> 
> buffered reads will find an up to date page with zeros instead of
> the data actually on disk.
> 
> This patch fixes things by using invalidate_inode_pages2_range
> instead.  It preserves the page cache invalidation, but won't zero
> any pages.
> 
> [dchinner: catch error and warn if it fails. Comment.]
> 
> cc: stable@vger.kernel.org
> Signed-off-by: Chris Mason <clm@fb.com>
> Reviewed-by: Dave Chinner <dchinner@redhat.com>
> Signed-off-by: Dave Chinner <david@fromorbit.com>
> ---

Reviewed-by: Brian Foster <bfoster@redhat.com>

>  fs/xfs/xfs_file.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
> index 076b170..827cfb2 100644
> --- a/fs/xfs/xfs_file.c
> +++ b/fs/xfs/xfs_file.c
> @@ -296,7 +296,16 @@ xfs_file_read_iter(
>  				xfs_rw_iunlock(ip, XFS_IOLOCK_EXCL);
>  				return ret;
>  			}
> -			truncate_pagecache_range(VFS_I(ip), pos, -1);
> +
> +			/*
> +			 * Invalidate whole pages. This can return an error if
> +			 * we fail to invalidate a page, but this should never
> +			 * happen on XFS. Warn if it does fail.
> +			 */
> +			ret = invalidate_inode_pages2_range(VFS_I(ip)->i_mapping,
> +						pos >> PAGE_CACHE_SHIFT, -1);
> +			WARN_ON_ONCE(ret);
> +			ret = 0;
>  		}
>  		xfs_rw_ilock_demote(ip, XFS_IOLOCK_EXCL);
>  	}
> -- 
> 2.0.0
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs

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

  reply	other threads:[~2014-08-21 13:08 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-08-21  5:09 [PATCH 0/6] xfs: direct IO invalidation and related fixes Dave Chinner
2014-08-21  5:09 ` [PATCH 1/6] xfs: mmap write/read leaves bad state on pages Dave Chinner
2014-08-21 12:48   ` Brian Foster
2014-08-21 22:38     ` Dave Chinner
2014-08-21 13:08   ` Christoph Hellwig
2014-08-21 13:54     ` Anton Altaparmakov
2014-08-21 15:21     ` Chris Mason
2014-08-21 19:56   ` Jan Kara
2014-08-21 22:33     ` Dave Chinner
2014-08-26 16:06       ` Jan Kara
2014-08-26 21:38         ` Dave Chinner
2014-08-21  5:09 ` [PATCH 2/6] xfs: don't zero partial page cache pages during O_DIRECT writes Dave Chinner
2014-08-21 13:08   ` Brian Foster [this message]
2014-08-21  5:09 ` [PATCH 3/6] " Dave Chinner
2014-08-21 13:09   ` Brian Foster
2014-08-21  5:09 ` [PATCH 4/6] xfs: use ranged writeback and invalidation for direct IO Dave Chinner
2014-08-21 13:09   ` Brian Foster
2014-08-21  5:09 ` [PATCH 5/6] xfs: don't log inode unless extent shift makes extent modifications Dave Chinner
2014-08-21  5:09 ` [PATCH 6/6] xfs: xfs_file_collapse_range is delalloc challenged Dave Chinner
2014-08-21 13:09   ` Brian Foster

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20140821130847.GC64112@bfoster.bfoster \
    --to=bfoster@redhat.com \
    --cc=clm@fb.com \
    --cc=david@fromorbit.com \
    --cc=xfs@oss.sgi.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.