All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] xfs: don't hand out pNFS layouts for reflink inodes
@ 2016-03-02 16:34 Christoph Hellwig
  2016-03-28 19:51 ` Darrick J. Wong
  0 siblings, 1 reply; 6+ messages in thread
From: Christoph Hellwig @ 2016-03-02 16:34 UTC (permalink / raw)
  To: darrick.wong; +Cc: xfs

Although the protocol allows for it, the Linux NFS server code doesn't
support reflink-like functionality yet.  It's been added to by TODO
list..

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_pnfs.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/fs/xfs/xfs_pnfs.c b/fs/xfs/xfs_pnfs.c
index ade236e..9708fef 100644
--- a/fs/xfs/xfs_pnfs.c
+++ b/fs/xfs/xfs_pnfs.c
@@ -139,6 +139,13 @@ xfs_fs_map_blocks(
 		return -ENXIO;
 
 	/*
+	 * The pNFS block layout spec actually supports reflink like
+	 * functionality, but the Linux pNFS server doesn't implement it yet.
+	 */
+	if (!xfs_is_reflink_inode(ip))
+		return -ENXIO;
+
+	/*
 	 * Lock out any other I/O before we flush and invalidate the pagecache,
 	 * and then hand out a layout to the remote system.  This is very
 	 * similar to direct I/O, except that the synchronization is much more
-- 
2.1.4

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

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] xfs: don't hand out pNFS layouts for reflink inodes
  2016-03-02 16:34 [PATCH] xfs: don't hand out pNFS layouts for reflink inodes Christoph Hellwig
@ 2016-03-28 19:51 ` Darrick J. Wong
  2016-03-29  7:21   ` Christoph Hellwig
  0 siblings, 1 reply; 6+ messages in thread
From: Darrick J. Wong @ 2016-03-28 19:51 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: xfs

[back to XFS after a few weeks' distractions]

On Wed, Mar 02, 2016 at 05:34:45PM +0100, Christoph Hellwig wrote:
> Although the protocol allows for it, the Linux NFS server code doesn't
> support reflink-like functionality yet.  It's been added to by TODO
> list..
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/xfs/xfs_pnfs.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/fs/xfs/xfs_pnfs.c b/fs/xfs/xfs_pnfs.c
> index ade236e..9708fef 100644
> --- a/fs/xfs/xfs_pnfs.c
> +++ b/fs/xfs/xfs_pnfs.c
> @@ -139,6 +139,13 @@ xfs_fs_map_blocks(
>  		return -ENXIO;
>  
>  	/*
> +	 * The pNFS block layout spec actually supports reflink like
> +	 * functionality, but the Linux pNFS server doesn't implement it yet.
> +	 */
> +	if (!xfs_is_reflink_inode(ip))
> +		return -ENXIO;

/me isn't familiar with pNFS, but doesn't this make it so that
xfs_fs_map_blocks /only/ works on reflinked files?  Maybe I'm misreading the
comment, but it sounds to me like Linux doesn't support reflinked files over
pNFS and therefore we want to disable this iomapping functionality any time we
find a reflinked file.

But maybe there's a subtlety here that I'm missing?

--D

> +
> +	/*
>  	 * Lock out any other I/O before we flush and invalidate the pagecache,
>  	 * and then hand out a layout to the remote system.  This is very
>  	 * similar to direct I/O, except that the synchronization is much more
> -- 
> 2.1.4
> 

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] xfs: don't hand out pNFS layouts for reflink inodes
  2016-03-28 19:51 ` Darrick J. Wong
@ 2016-03-29  7:21   ` Christoph Hellwig
  2016-03-30  0:08     ` Darrick J. Wong
  0 siblings, 1 reply; 6+ messages in thread
From: Christoph Hellwig @ 2016-03-29  7:21 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: Christoph Hellwig, xfs

On Mon, Mar 28, 2016 at 12:51:40PM -0700, Darrick J. Wong wrote:
> >  	/*
> > +	 * The pNFS block layout spec actually supports reflink like
> > +	 * functionality, but the Linux pNFS server doesn't implement it yet.
> > +	 */
> > +	if (!xfs_is_reflink_inode(ip))
> > +		return -ENXIO;
> 
> /me isn't familiar with pNFS, but doesn't this make it so that
> xfs_fs_map_blocks /only/ works on reflinked files?  Maybe I'm misreading the
> comment, but it sounds to me like Linux doesn't support reflinked files over
> pNFS and therefore we want to disable this iomapping functionality any time we
> find a reflinked file.
> 
> But maybe there's a subtlety here that I'm missing?

You jsut missed me fat fingering the check :)

Btw, we'll need something similar to prevent swapfiles which just use
->bmap, but unfortunately swap just checks for the existences of method,
so I'll probably have to do some VFS changes to properly support that.

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] xfs: don't hand out pNFS layouts for reflink inodes
  2016-03-29  7:21   ` Christoph Hellwig
@ 2016-03-30  0:08     ` Darrick J. Wong
  2016-03-30 17:26       ` Christoph Hellwig
  0 siblings, 1 reply; 6+ messages in thread
From: Darrick J. Wong @ 2016-03-30  0:08 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: xfs

On Tue, Mar 29, 2016 at 09:21:28AM +0200, Christoph Hellwig wrote:
> On Mon, Mar 28, 2016 at 12:51:40PM -0700, Darrick J. Wong wrote:
> > >  	/*
> > > +	 * The pNFS block layout spec actually supports reflink like
> > > +	 * functionality, but the Linux pNFS server doesn't implement it yet.
> > > +	 */
> > > +	if (!xfs_is_reflink_inode(ip))
> > > +		return -ENXIO;
> > 
> > /me isn't familiar with pNFS, but doesn't this make it so that
> > xfs_fs_map_blocks /only/ works on reflinked files?  Maybe I'm misreading the
> > comment, but it sounds to me like Linux doesn't support reflinked files over
> > pNFS and therefore we want to disable this iomapping functionality any time we
> > find a reflinked file.
> > 
> > But maybe there's a subtlety here that I'm missing?
> 
> You jsut missed me fat fingering the check :)

Fair enough, I'll integrate this patch, but with:

if (xfs_is_reflink_inode(ip))
	return -ENXIO;

> Btw, we'll need something similar to prevent swapfiles which just use
> ->bmap, but unfortunately swap just checks for the existences of method,
> so I'll probably have to do some VFS changes to properly support that.

Yes, please.  ocfs2 will gladly let you call swapon on a reflinked file,
and totally fails to CoW it properly. :)

--D

> 
> _______________________________________________
> 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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] xfs: don't hand out pNFS layouts for reflink inodes
  2016-03-30  0:08     ` Darrick J. Wong
@ 2016-03-30 17:26       ` Christoph Hellwig
  2016-03-30 17:40         ` Darrick J. Wong
  0 siblings, 1 reply; 6+ messages in thread
From: Christoph Hellwig @ 2016-03-30 17:26 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: Christoph Hellwig, xfs

On Tue, Mar 29, 2016 at 05:08:42PM -0700, Darrick J. Wong wrote:
> > Btw, we'll need something similar to prevent swapfiles which just use
> > ->bmap, but unfortunately swap just checks for the existences of method,
> > so I'll probably have to do some VFS changes to properly support that.
> 
> Yes, please.  ocfs2 will gladly let you call swapon on a reflinked file,
> and totally fails to CoW it properly. :)

Ouch.  Time to add a test for that to xfstests..

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] xfs: don't hand out pNFS layouts for reflink inodes
  2016-03-30 17:26       ` Christoph Hellwig
@ 2016-03-30 17:40         ` Darrick J. Wong
  0 siblings, 0 replies; 6+ messages in thread
From: Darrick J. Wong @ 2016-03-30 17:40 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: xfs

On Wed, Mar 30, 2016 at 07:26:34PM +0200, Christoph Hellwig wrote:
> On Tue, Mar 29, 2016 at 05:08:42PM -0700, Darrick J. Wong wrote:
> > > Btw, we'll need something similar to prevent swapfiles which just use
> > > ->bmap, but unfortunately swap just checks for the existences of method,
> > > so I'll probably have to do some VFS changes to properly support that.
> > 
> > Yes, please.  ocfs2 will gladly let you call swapon on a reflinked file,
> > and totally fails to CoW it properly. :)
> 
> Ouch.  Time to add a test for that to xfstests..

Swap/reflink tests will appear as generic/92[34] the next time I post xfstests
patches.

--D

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2016-03-30 17:40 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-03-02 16:34 [PATCH] xfs: don't hand out pNFS layouts for reflink inodes Christoph Hellwig
2016-03-28 19:51 ` Darrick J. Wong
2016-03-29  7:21   ` Christoph Hellwig
2016-03-30  0:08     ` Darrick J. Wong
2016-03-30 17:26       ` Christoph Hellwig
2016-03-30 17:40         ` Darrick J. Wong

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.