Re: [PATCH 1/3] VFS/ioctl: Add punching-hole support to ioctl().

Re: [PATCH 1/3] VFS/ioctl: Add punching-hole support to ioctl().

[Sunil Mushran]

Frankly I see no point extending the ioctl interface when we have
a syscall interface.

On 03/31/2011 12:33 AM, Tristan Ye wrote:
> We're currently support two paths from VFS to preallocate unwritten
> extents(from FS_IOC_RESVSP, or fallocate()), likewise, behavior of
> punching-hole should be treated as the same, this patch tries to teach
> file_ioctl() to handle FS_IOC_UNRESVSP, underlying filesystem like ocfs2
> is wise enough to do the rest of work;-)
>
> Signed-off-by: Tristan Ye<tristan.ye@oracle.com>
> ---
>   fs/ioctl.c             |   10 +++++++---
>   include/linux/falloc.h |    2 ++
>   2 files changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/fs/ioctl.c b/fs/ioctl.c
> index 1d9b9fc..234e26f 100644
> --- a/fs/ioctl.c
> +++ b/fs/ioctl.c
> @@ -422,7 +422,7 @@ EXPORT_SYMBOL(generic_block_fiemap);
>    * Only the l_start, l_len and l_whence fields of the 'struct space_resv'
>    * are used here, rest are ignored.
>    */
> -int ioctl_preallocate(struct file *filp, void __user *argp)
> +int ioctl_preallocate(struct file *filp, void __user *argp, int mode)
>   {
>   	struct inode *inode = filp->f_path.dentry->d_inode;
>   	struct space_resv sr;
> @@ -443,7 +443,7 @@ int ioctl_preallocate(struct file *filp, void __user *argp)
>   		return -EINVAL;
>   	}
>
> -	return do_fallocate(filp, FALLOC_FL_KEEP_SIZE, sr.l_start, sr.l_len);
> +	return do_fallocate(filp, mode, sr.l_start, sr.l_len);
>   }
>
>   static int file_ioctl(struct file *filp, unsigned int cmd,
> @@ -459,7 +459,11 @@ static int file_ioctl(struct file *filp, unsigned int cmd,
>   		return put_user(i_size_read(inode) - filp->f_pos, p);
>   	case FS_IOC_RESVSP:
>   	case FS_IOC_RESVSP64:
> -		return ioctl_preallocate(filp, p);
> +		return ioctl_preallocate(filp, p, FALLOC_FL_KEEP_SIZE);
> +	case FS_IOC_UNRESVSP:
> +	case FS_IOC_UNRESVSP64:
> +		return ioctl_preallocate(filp, p, FALLOC_FL_KEEP_SIZE |
> +						  FALLOC_FL_PUNCH_HOLE);
>   	}
>
>   	return vfs_ioctl(filp, cmd, arg);
> diff --git a/include/linux/falloc.h b/include/linux/falloc.h
> index 73e0b62..fd1e871 100644
> --- a/include/linux/falloc.h
> +++ b/include/linux/falloc.h
> @@ -21,7 +21,9 @@ struct space_resv {
>   };
>
>   #define FS_IOC_RESVSP		_IOW('X', 40, struct space_resv)
> +#define FS_IOC_UNRESVSP		_IOW('X', 41, struct space_resv)
>   #define FS_IOC_RESVSP64		_IOW('X', 42, struct space_resv)
> +#define FS_IOC_UNRESVSP64	_IOW('X', 43, struct space_resv)
>
>   #endif /* __KERNEL__ */
>

Re: [PATCH 1/3] VFS/ioctl: Add punching-hole support to ioctl().

[Josef Bacik]

On Thu, Mar 31, 2011 at 02:14:43PM -0700, Sunil Mushran wrote:
> Frankly I see no point extending the ioctl interface when we have
> a syscall interface.
>

I'd even go so far as to say we could probably axe the xfs and ocfs2 ioctls
since we have the fallocate interface :).  Thanks,

Josef

Re: [PATCH 1/3] VFS/ioctl: Add punching-hole support to ioctl().

[Joel Becker]

On Thu, Mar 31, 2011 at 06:56:18PM -0400, Josef Bacik wrote:
> On Thu, Mar 31, 2011 at 02:14:43PM -0700, Sunil Mushran wrote:
> > Frankly I see no point extending the ioctl interface when we have
> > a syscall interface.
> >
> 
> I'd even go so far as to say we could probably axe the xfs and ocfs2 ioctls
> since we have the fallocate interface :).  Thanks,

	These ioctls are in long use.  Granted, it is for the small
subset of users that know xfs and ocfs2 can do this, but still.
<venkman>Breaking userspace is *bad*.</venkman>
	More interesting would be to bring the ioctls up to generic code
and have them backended by fallocate.  I'm not sure they map without
looking deeper, but it's at least an idea.

Joel

-- 

 print STDOUT q
 Just another Perl hacker,
 unless $spring
	- Larry Wall

			http://www.jlbec.org/
			jlbec@evilplan.org

Re: [PATCH 1/3] VFS/ioctl: Add punching-hole support to ioctl().

[Josef Bacik]

On Thu, Mar 31, 2011 at 04:44:55PM -0700, Joel Becker wrote:
> On Thu, Mar 31, 2011 at 06:56:18PM -0400, Josef Bacik wrote:
> > On Thu, Mar 31, 2011 at 02:14:43PM -0700, Sunil Mushran wrote:
> > > Frankly I see no point extending the ioctl interface when we have
> > > a syscall interface.
> > >
> > 
> > I'd even go so far as to say we could probably axe the xfs and ocfs2 ioctls
> > since we have the fallocate interface :).  Thanks,
> 
> 	These ioctls are in long use.  Granted, it is for the small
> subset of users that know xfs and ocfs2 can do this, but still.
> <venkman>Breaking userspace is *bad*.</venkman>

Yeah I wasn't serious, though I do wish there was a way to mark these sort of
interfaces deprecated to give us a path to retire old interfaces.

> 	More interesting would be to bring the ioctls up to generic code
> and have them backended by fallocate.  I'm not sure they map without
> looking deeper, but it's at least an idea.
> 

I just did a cursory look and it seems like that would work out ok.  Thanks,

Josef

Re: [Ocfs2-devel] [PATCH 1/3] VFS/ioctl: Add punching-hole support to ioctl().

[Joel Becker]

On Thu, Mar 31, 2011 at 08:34:50PM -0400, Josef Bacik wrote:
> > 	More interesting would be to bring the ioctls up to generic code
> > and have them backended by fallocate.  I'm not sure they map without
> > looking deeper, but it's at least an idea.
> > 
> 
> I just did a cursory look and it seems like that would work out ok.  Thanks,

	Note that ocfs2 and xfs have identical ioctl values (ocfs2
copied xfs's on purpose).

Joel

-- 

Life's Little Instruction Book #306

	"Take a nap on Sunday afternoons."

			http://www.jlbec.org/
			jlbec@evilplan.org