diff mbox series

[1/2] xfs: disable xfs_ioc_space for always COW inodes

Message ID 20191011130316.13373-2-hch@lst.de (mailing list archive)
State Superseded, archived
Headers show
Series [1/2] xfs: disable xfs_ioc_space for always COW inodes | expand

Commit Message

Christoph Hellwig Oct. 11, 2019, 1:03 p.m. UTC
If we always have to write out of place preallocating blocks is
pointless.  We already check for this in the normal falloc path, but
the check was missig in the legacy ALLOCSP path.

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

Comments

Darrick J. Wong Oct. 12, 2019, 12:29 a.m. UTC | #1
On Fri, Oct 11, 2019 at 03:03:15PM +0200, Christoph Hellwig wrote:
> If we always have to write out of place preallocating blocks is
> pointless.  We already check for this in the normal falloc path, but
> the check was missig in the legacy ALLOCSP path.

This function handles other things than preallocation, such as
XFS_IOC_ZERO_RANGE and XFS_IOC_UNRESVSP, which call xfs_zero_file_space
and xfs_free_file_space, respectively.  We don't prohibit fallocate
from calling those two functions on an always_cow inode, so why do that
here?

--D

> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/xfs/xfs_ioctl.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
> index d58f0d6a699e..abf7a102376f 100644
> --- a/fs/xfs/xfs_ioctl.c
> +++ b/fs/xfs/xfs_ioctl.c
> @@ -33,6 +33,7 @@
>  #include "xfs_sb.h"
>  #include "xfs_ag.h"
>  #include "xfs_health.h"
> +#include "xfs_reflink.h"
>  
>  #include <linux/mount.h>
>  #include <linux/namei.h>
> @@ -607,6 +608,9 @@ xfs_ioc_space(
>  	if (!S_ISREG(inode->i_mode))
>  		return -EINVAL;
>  
> +	if (xfs_is_always_cow_inode(ip))
> +		return -EOPNOTSUPP;
> +
>  	if (filp->f_flags & O_DSYNC)
>  		flags |= XFS_PREALLOC_SYNC;
>  	if (filp->f_mode & FMODE_NOCMTIME)
> -- 
> 2.20.1
>
Christoph Hellwig Oct. 14, 2019, 7:18 a.m. UTC | #2
On Fri, Oct 11, 2019 at 05:29:54PM -0700, Darrick J. Wong wrote:
> On Fri, Oct 11, 2019 at 03:03:15PM +0200, Christoph Hellwig wrote:
> > If we always have to write out of place preallocating blocks is
> > pointless.  We already check for this in the normal falloc path, but
> > the check was missig in the legacy ALLOCSP path.
> 
> This function handles other things than preallocation, such as
> XFS_IOC_ZERO_RANGE and XFS_IOC_UNRESVSP, which call xfs_zero_file_space
> and xfs_free_file_space, respectively.  We don't prohibit fallocate
> from calling those two functions on an always_cow inode, so why do that
> here?

True.  I actually have a patch in my tree that switches those to
be handled in the core so that they enter XFS through ->fallocate.
It didn't make any sense to send this patch before that other change,
sorry.
diff mbox series

Patch

diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
index d58f0d6a699e..abf7a102376f 100644
--- a/fs/xfs/xfs_ioctl.c
+++ b/fs/xfs/xfs_ioctl.c
@@ -33,6 +33,7 @@ 
 #include "xfs_sb.h"
 #include "xfs_ag.h"
 #include "xfs_health.h"
+#include "xfs_reflink.h"
 
 #include <linux/mount.h>
 #include <linux/namei.h>
@@ -607,6 +608,9 @@  xfs_ioc_space(
 	if (!S_ISREG(inode->i_mode))
 		return -EINVAL;
 
+	if (xfs_is_always_cow_inode(ip))
+		return -EOPNOTSUPP;
+
 	if (filp->f_flags & O_DSYNC)
 		flags |= XFS_PREALLOC_SYNC;
 	if (filp->f_mode & FMODE_NOCMTIME)