linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
* oh the joy of swap files
@ 2016-05-26 13:02 Christoph Hellwig
  2016-05-26 13:02 ` [PATCH] xfs: fail ->bmap for reflink inodes Christoph Hellwig
  0 siblings, 1 reply; 5+ messages in thread
From: Christoph Hellwig @ 2016-05-26 13:02 UTC (permalink / raw)
  To: darrick.wong; +Cc: xfs, linux-mm

Quirk out of ->bmap for reflink inodes because the swap code still
believes it's a good idea to blindly bypass the fs if it exists.

This is a bit of a crude hack, but without redesigning how swapfiles
work we can't really do very much better.  Except for a real error
code from ->bmap, but that's a bit like putting lipstick on a pig..

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

* [PATCH] xfs: fail ->bmap for reflink inodes
  2016-05-26 13:02 oh the joy of swap files Christoph Hellwig
@ 2016-05-26 13:02 ` Christoph Hellwig
  2016-05-27  4:11   ` Darrick J. Wong
  2016-05-27 17:32   ` Avi Kivity
  0 siblings, 2 replies; 5+ messages in thread
From: Christoph Hellwig @ 2016-05-26 13:02 UTC (permalink / raw)
  To: darrick.wong; +Cc: xfs, linux-mm

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=a, Size: 1083 bytes --]

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

diff --git a/fs/xfs/xfs_aops.c b/fs/xfs/xfs_aops.c
index a955552..d053a9e 100644
--- a/fs/xfs/xfs_aops.c
+++ b/fs/xfs/xfs_aops.c
@@ -1829,6 +1829,17 @@ xfs_vm_bmap(
 
 	trace_xfs_vm_bmap(XFS_I(inode));
 	xfs_ilock(ip, XFS_IOLOCK_SHARED);
+
+	/*
+	 * The swap code (ab-)uses ->bmap to get a block mapping and then
+	 * bypasseN? the file system for actual I/O.  We really can't allow
+	 * that on reflinks inodes, so we have to skip out here.  And yes,
+	 * 0 is the magic code for a bmap error..
+	 */
+	if (xfs_is_reflink_inode(ip)) {
+		xfs_iunlock(ip, XFS_IOLOCK_SHARED);
+		return 0;
+	}
 	filemap_write_and_wait(mapping);
 	xfs_iunlock(ip, XFS_IOLOCK_SHARED);
 	return generic_block_bmap(mapping, block, xfs_get_blocks);
-- 
2.1.4

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

* Re: [PATCH] xfs: fail ->bmap for reflink inodes
  2016-05-26 13:02 ` [PATCH] xfs: fail ->bmap for reflink inodes Christoph Hellwig
@ 2016-05-27  4:11   ` Darrick J. Wong
  2016-05-27 17:32   ` Avi Kivity
  1 sibling, 0 replies; 5+ messages in thread
From: Darrick J. Wong @ 2016-05-27  4:11 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: linux-mm, xfs

On Thu, May 26, 2016 at 03:02:04PM +0200, Christoph Hellwig wrote:
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/xfs/xfs_aops.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/fs/xfs/xfs_aops.c b/fs/xfs/xfs_aops.c
> index a955552..d053a9e 100644
> --- a/fs/xfs/xfs_aops.c
> +++ b/fs/xfs/xfs_aops.c
> @@ -1829,6 +1829,17 @@ xfs_vm_bmap(
>  
>  	trace_xfs_vm_bmap(XFS_I(inode));
>  	xfs_ilock(ip, XFS_IOLOCK_SHARED);
> +
> +	/*
> +	 * The swap code (ab-)uses ->bmap to get a block mapping and then
> +	 * bypasseN? the file system for actual I/O.  We really can't allow
> +	 * that on reflinks inodes, so we have to skip out here.  And yes,
> +	 * 0 is the magic code for a bmap error..
> +	 */
> +	if (xfs_is_reflink_inode(ip)) {
> +		xfs_iunlock(ip, XFS_IOLOCK_SHARED);
> +		return 0;
> +	}

/me adds to the reflink patchpile, thanks.

Just poking at mm/swapfile.c it looks like iomap might work well
as a replacement for repeated bmap() calls, once the iomap vfs
bits get in.

--D

>  	filemap_write_and_wait(mapping);
>  	xfs_iunlock(ip, XFS_IOLOCK_SHARED);
>  	return generic_block_bmap(mapping, block, xfs_get_blocks);
> -- 
> 2.1.4
> 

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

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

* Re: [PATCH] xfs: fail ->bmap for reflink inodes
  2016-05-26 13:02 ` [PATCH] xfs: fail ->bmap for reflink inodes Christoph Hellwig
  2016-05-27  4:11   ` Darrick J. Wong
@ 2016-05-27 17:32   ` Avi Kivity
  2016-05-27 17:40     ` Darrick J. Wong
  1 sibling, 1 reply; 5+ messages in thread
From: Avi Kivity @ 2016-05-27 17:32 UTC (permalink / raw)
  To: Christoph Hellwig, darrick.wong; +Cc: linux-mm, xfs

On 05/26/2016 04:02 PM, Christoph Hellwig wrote:
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>   fs/xfs/xfs_aops.c | 11 +++++++++++
>   1 file changed, 11 insertions(+)
>
> diff --git a/fs/xfs/xfs_aops.c b/fs/xfs/xfs_aops.c
> index a955552..d053a9e 100644
> --- a/fs/xfs/xfs_aops.c
> +++ b/fs/xfs/xfs_aops.c
> @@ -1829,6 +1829,17 @@ xfs_vm_bmap(
>   
>   	trace_xfs_vm_bmap(XFS_I(inode));
>   	xfs_ilock(ip, XFS_IOLOCK_SHARED);
> +
> +	/*
> +	 * The swap code (ab-)uses ->bmap to get a block mapping and then
> +	 * bypasseN? the file system for actual I/O.  We really can't allow
> +	 * that on reflinks inodes, so we have to skip out here.  And yes,
> +	 * 0 is the magic code for a bmap error..
> +	 */
> +	if (xfs_is_reflink_inode(ip)) {
> +		xfs_iunlock(ip, XFS_IOLOCK_SHARED);
> +		return 0;
> +	}
>   	filemap_write_and_wait(mapping);
>   	xfs_iunlock(ip, XFS_IOLOCK_SHARED);
>   	return generic_block_bmap(mapping, block, xfs_get_blocks);

Don't you also have to prevent a swapfile from being reflinked after 
it's bmapped?  Or is that already taken care of?

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

* Re: [PATCH] xfs: fail ->bmap for reflink inodes
  2016-05-27 17:32   ` Avi Kivity
@ 2016-05-27 17:40     ` Darrick J. Wong
  0 siblings, 0 replies; 5+ messages in thread
From: Darrick J. Wong @ 2016-05-27 17:40 UTC (permalink / raw)
  To: Avi Kivity; +Cc: Christoph Hellwig, linux-mm, xfs

On Fri, May 27, 2016 at 08:32:18PM +0300, Avi Kivity wrote:
> On 05/26/2016 04:02 PM, Christoph Hellwig wrote:
> >Signed-off-by: Christoph Hellwig <hch@lst.de>
> >---
> >  fs/xfs/xfs_aops.c | 11 +++++++++++
> >  1 file changed, 11 insertions(+)
> >
> >diff --git a/fs/xfs/xfs_aops.c b/fs/xfs/xfs_aops.c
> >index a955552..d053a9e 100644
> >--- a/fs/xfs/xfs_aops.c
> >+++ b/fs/xfs/xfs_aops.c
> >@@ -1829,6 +1829,17 @@ xfs_vm_bmap(
> >  	trace_xfs_vm_bmap(XFS_I(inode));
> >  	xfs_ilock(ip, XFS_IOLOCK_SHARED);
> >+
> >+	/*
> >+	 * The swap code (ab-)uses ->bmap to get a block mapping and then
> >+	 * bypasseN? the file system for actual I/O.  We really can't allow
> >+	 * that on reflinks inodes, so we have to skip out here.  And yes,
> >+	 * 0 is the magic code for a bmap error..
> >+	 */
> >+	if (xfs_is_reflink_inode(ip)) {
> >+		xfs_iunlock(ip, XFS_IOLOCK_SHARED);
> >+		return 0;
> >+	}
> >  	filemap_write_and_wait(mapping);
> >  	xfs_iunlock(ip, XFS_IOLOCK_SHARED);
> >  	return generic_block_bmap(mapping, block, xfs_get_blocks);
> 
> Don't you also have to prevent a swapfile from being reflinked after it's
> bmapped?  Or is that already taken care of?

Already taken care of, at least for XFS.

--D

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

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

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-05-26 13:02 oh the joy of swap files Christoph Hellwig
2016-05-26 13:02 ` [PATCH] xfs: fail ->bmap for reflink inodes Christoph Hellwig
2016-05-27  4:11   ` Darrick J. Wong
2016-05-27 17:32   ` Avi Kivity
2016-05-27 17:40     ` Darrick J. Wong

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).