All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ankit Jain <me@ankitjain.org>
To: Mark Fasheh <mfasheh@suse.com>
Cc: linux-kernel@vger.kernel.org, joel.becker@oracle.com,
	Christoph Hellwig <hch@infradead.org>,
	xfs-masters@oss.sgi.com, Al Viro <viro@zeniv.linux.org.uk>,
	linux-fsdevel@vger.kernel.org, xfs@oss.sgi.com,
	ocfs2-devel@oss.oracle.com
Subject: Re: [PATCH][RFC] fs: Add new pre-allocation ioctls to vfs for compatibility with legacy xfs ioctls
Date: Thu, 18 Dec 2008 15:14:09 +0530	[thread overview]
Message-ID: <494A1B69.70300@ankitjain.org> (raw)
In-Reply-To: <20081217202815.GE8791@wotan.suse.de>

Mark Fasheh wrote:
>> 2. Should the corresponding ioctls be removed from ocfs2?
> 
> Well, a small amount of the code in fs/ocfs2/ioctl.c can certainly go away.
> Shouldn't we be talking about doing the same for xfs too?

Yep. I'll cook up cleanup patches for these two and post them.

-Ankit

> 
> 
>> Signed-off-by: Ankit Jain <me@ankitjain.org>
>> ---
>>  fs/ioctl.c             |   37 +++++++++++++++++++++++++++
>>  fs/open.c              |   51 ++++++++++++++++++-------------------
>>  include/linux/falloc.h |   19 ++++++++++++++
>>  include/linux/fs.h     |    2 +
>>  4 files changed, 83 insertions(+), 26 deletions(-)
>>
>> diff --git a/fs/ioctl.c b/fs/ioctl.c
>> index 43e8b2c..5e565c8 100644
>> --- a/fs/ioctl.c
>> +++ b/fs/ioctl.c
>> @@ -15,6 +15,7 @@
>>  #include <linux/uaccess.h>
>>  #include <linux/writeback.h>
>>  #include <linux/buffer_head.h>
>> +#include <linux/falloc.h>
>>  
>>  #include <asm/ioctls.h>
>>  
>> @@ -346,6 +347,37 @@ EXPORT_SYMBOL(generic_block_fiemap);
>>  
>>  #endif  /*  CONFIG_BLOCK  */
>>  
>> +/*
>> + * This provides compatibility with legacy XFS pre-allocation ioctls
>> + * which predate the fallocate syscall.
>> + *
>> + * Only the l_start, l_len and l_whence fields of the 'struct space_resv'
>> + * are used here, rest are ignored.
>> + */
>> +static int ioctl_preallocate(struct file *filp, unsigned long arg)
>> +{
>> +	struct inode *inode = filp->f_path.dentry->d_inode;
>> +	struct space_resv sr;
>> +
>> +	if (copy_from_user(&sr, (struct space_resv __user *) arg, sizeof(sr)))
>> +		return -EFAULT;
>> +
>> +	switch (sr.l_whence) {
>> +	case SEEK_SET:
>> +		break;
>> +	case SEEK_CUR:
>> +		sr.l_start += filp->f_pos;
>> +		break;
>> +	case SEEK_END:
>> +		sr.l_start += i_size_read(inode);
>> +		break;
>> +	default:
>> +		return -EINVAL;
>> +	}
>> +
>> +	return do_fallocate(filp, FALLOC_FL_KEEP_SIZE, sr.l_start, sr.l_len);
>> +}
>> +
>>  static int file_ioctl(struct file *filp, unsigned int cmd,
>>  		unsigned long arg)
>>  {
>> @@ -361,6 +393,11 @@ static int file_ioctl(struct file *filp, unsigned int cmd,
>>  		return put_user(inode->i_sb->s_blocksize, p);
>>  	case FIONREAD:
>>  		return put_user(i_size_read(inode) - filp->f_pos, p);
>> +	case F_IOC_RESVSP:
>> +	case F_IOC_RESVSP64:
>> +	case F_IOC_UNRESVSP:
>> +	case F_IOC_UNRESVSP64:
>> +		return ioctl_preallocate(filp, arg);
> 
> This patch is not implementing proper support for F_IOC_UNRESVSP and
> F_IOC_UNRESVSP64, so why are you catching those here? To be more clear,
> those are used for freeing space in a file ("puching holes"), which
> fallocate is not set up to do right now.
> 	--Mark
> 
> --
> Mark Fasheh

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

WARNING: multiple messages have this Message-ID (diff)
From: Ankit Jain <me@ankitjain.org>
To: Mark Fasheh <mfasheh@suse.com>
Cc: Al Viro <viro@zeniv.linux.org.uk>,
	Christoph Hellwig <hch@infradead.org>,
	linux-fsdevel@vger.kernel.org, joel.becker@oracle.com,
	ocfs2-devel@oss.oracle.com, linux-kernel@vger.kernel.org,
	xfs-masters@oss.sgi.com, xfs@oss.sgi.com
Subject: [Ocfs2-devel] [PATCH][RFC] fs: Add new pre-allocation ioctls to vfs for compatibility with legacy xfs ioctls
Date: Thu, 18 Dec 2008 15:14:09 +0530	[thread overview]
Message-ID: <494A1B69.70300@ankitjain.org> (raw)
In-Reply-To: <20081217202815.GE8791@wotan.suse.de>

Mark Fasheh wrote:
>> 2. Should the corresponding ioctls be removed from ocfs2?
> 
> Well, a small amount of the code in fs/ocfs2/ioctl.c can certainly go away.
> Shouldn't we be talking about doing the same for xfs too?

Yep. I'll cook up cleanup patches for these two and post them.

-Ankit

> 
> 
>> Signed-off-by: Ankit Jain <me@ankitjain.org>
>> ---
>>  fs/ioctl.c             |   37 +++++++++++++++++++++++++++
>>  fs/open.c              |   51 ++++++++++++++++++-------------------
>>  include/linux/falloc.h |   19 ++++++++++++++
>>  include/linux/fs.h     |    2 +
>>  4 files changed, 83 insertions(+), 26 deletions(-)
>>
>> diff --git a/fs/ioctl.c b/fs/ioctl.c
>> index 43e8b2c..5e565c8 100644
>> --- a/fs/ioctl.c
>> +++ b/fs/ioctl.c
>> @@ -15,6 +15,7 @@
>>  #include <linux/uaccess.h>
>>  #include <linux/writeback.h>
>>  #include <linux/buffer_head.h>
>> +#include <linux/falloc.h>
>>  
>>  #include <asm/ioctls.h>
>>  
>> @@ -346,6 +347,37 @@ EXPORT_SYMBOL(generic_block_fiemap);
>>  
>>  #endif  /*  CONFIG_BLOCK  */
>>  
>> +/*
>> + * This provides compatibility with legacy XFS pre-allocation ioctls
>> + * which predate the fallocate syscall.
>> + *
>> + * Only the l_start, l_len and l_whence fields of the 'struct space_resv'
>> + * are used here, rest are ignored.
>> + */
>> +static int ioctl_preallocate(struct file *filp, unsigned long arg)
>> +{
>> +	struct inode *inode = filp->f_path.dentry->d_inode;
>> +	struct space_resv sr;
>> +
>> +	if (copy_from_user(&sr, (struct space_resv __user *) arg, sizeof(sr)))
>> +		return -EFAULT;
>> +
>> +	switch (sr.l_whence) {
>> +	case SEEK_SET:
>> +		break;
>> +	case SEEK_CUR:
>> +		sr.l_start += filp->f_pos;
>> +		break;
>> +	case SEEK_END:
>> +		sr.l_start += i_size_read(inode);
>> +		break;
>> +	default:
>> +		return -EINVAL;
>> +	}
>> +
>> +	return do_fallocate(filp, FALLOC_FL_KEEP_SIZE, sr.l_start, sr.l_len);
>> +}
>> +
>>  static int file_ioctl(struct file *filp, unsigned int cmd,
>>  		unsigned long arg)
>>  {
>> @@ -361,6 +393,11 @@ static int file_ioctl(struct file *filp, unsigned int cmd,
>>  		return put_user(inode->i_sb->s_blocksize, p);
>>  	case FIONREAD:
>>  		return put_user(i_size_read(inode) - filp->f_pos, p);
>> +	case F_IOC_RESVSP:
>> +	case F_IOC_RESVSP64:
>> +	case F_IOC_UNRESVSP:
>> +	case F_IOC_UNRESVSP64:
>> +		return ioctl_preallocate(filp, arg);
> 
> This patch is not implementing proper support for F_IOC_UNRESVSP and
> F_IOC_UNRESVSP64, so why are you catching those here? To be more clear,
> those are used for freeing space in a file ("puching holes"), which
> fallocate is not set up to do right now.
> 	--Mark
> 
> --
> Mark Fasheh

WARNING: multiple messages have this Message-ID (diff)
From: Ankit Jain <me@ankitjain.org>
To: Mark Fasheh <mfasheh@suse.com>
Cc: Al Viro <viro@zeniv.linux.org.uk>,
	Christoph Hellwig <hch@infradead.org>,
	linux-fsdevel@vger.kernel.org, joel.becker@oracle.com,
	ocfs2-devel@oss.oracle.com, linux-kernel@vger.kernel.org,
	xfs-masters@oss.sgi.com, xfs@oss.sgi.com
Subject: Re: [PATCH][RFC] fs: Add new pre-allocation ioctls to vfs for compatibility with legacy xfs ioctls
Date: Thu, 18 Dec 2008 15:14:09 +0530	[thread overview]
Message-ID: <494A1B69.70300@ankitjain.org> (raw)
In-Reply-To: <20081217202815.GE8791@wotan.suse.de>

Mark Fasheh wrote:
>> 2. Should the corresponding ioctls be removed from ocfs2?
> 
> Well, a small amount of the code in fs/ocfs2/ioctl.c can certainly go away.
> Shouldn't we be talking about doing the same for xfs too?

Yep. I'll cook up cleanup patches for these two and post them.

-Ankit

> 
> 
>> Signed-off-by: Ankit Jain <me@ankitjain.org>
>> ---
>>  fs/ioctl.c             |   37 +++++++++++++++++++++++++++
>>  fs/open.c              |   51 ++++++++++++++++++-------------------
>>  include/linux/falloc.h |   19 ++++++++++++++
>>  include/linux/fs.h     |    2 +
>>  4 files changed, 83 insertions(+), 26 deletions(-)
>>
>> diff --git a/fs/ioctl.c b/fs/ioctl.c
>> index 43e8b2c..5e565c8 100644
>> --- a/fs/ioctl.c
>> +++ b/fs/ioctl.c
>> @@ -15,6 +15,7 @@
>>  #include <linux/uaccess.h>
>>  #include <linux/writeback.h>
>>  #include <linux/buffer_head.h>
>> +#include <linux/falloc.h>
>>  
>>  #include <asm/ioctls.h>
>>  
>> @@ -346,6 +347,37 @@ EXPORT_SYMBOL(generic_block_fiemap);
>>  
>>  #endif  /*  CONFIG_BLOCK  */
>>  
>> +/*
>> + * This provides compatibility with legacy XFS pre-allocation ioctls
>> + * which predate the fallocate syscall.
>> + *
>> + * Only the l_start, l_len and l_whence fields of the 'struct space_resv'
>> + * are used here, rest are ignored.
>> + */
>> +static int ioctl_preallocate(struct file *filp, unsigned long arg)
>> +{
>> +	struct inode *inode = filp->f_path.dentry->d_inode;
>> +	struct space_resv sr;
>> +
>> +	if (copy_from_user(&sr, (struct space_resv __user *) arg, sizeof(sr)))
>> +		return -EFAULT;
>> +
>> +	switch (sr.l_whence) {
>> +	case SEEK_SET:
>> +		break;
>> +	case SEEK_CUR:
>> +		sr.l_start += filp->f_pos;
>> +		break;
>> +	case SEEK_END:
>> +		sr.l_start += i_size_read(inode);
>> +		break;
>> +	default:
>> +		return -EINVAL;
>> +	}
>> +
>> +	return do_fallocate(filp, FALLOC_FL_KEEP_SIZE, sr.l_start, sr.l_len);
>> +}
>> +
>>  static int file_ioctl(struct file *filp, unsigned int cmd,
>>  		unsigned long arg)
>>  {
>> @@ -361,6 +393,11 @@ static int file_ioctl(struct file *filp, unsigned int cmd,
>>  		return put_user(inode->i_sb->s_blocksize, p);
>>  	case FIONREAD:
>>  		return put_user(i_size_read(inode) - filp->f_pos, p);
>> +	case F_IOC_RESVSP:
>> +	case F_IOC_RESVSP64:
>> +	case F_IOC_UNRESVSP:
>> +	case F_IOC_UNRESVSP64:
>> +		return ioctl_preallocate(filp, arg);
> 
> This patch is not implementing proper support for F_IOC_UNRESVSP and
> F_IOC_UNRESVSP64, so why are you catching those here? To be more clear,
> those are used for freeing space in a file ("puching holes"), which
> fallocate is not set up to do right now.
> 	--Mark
> 
> --
> Mark Fasheh


  parent reply	other threads:[~2008-12-18 13:42 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-12-15  8:04 [PATCH][RFC] fs: Add new pre-allocation ioctls to vfs for compatibility with legacy xfs ioctls Ankit Jain
2008-12-15  8:04 ` Ankit Jain
2008-12-15  8:04 ` [Ocfs2-devel] " Ankit Jain
2008-12-17 20:28 ` Mark Fasheh
2008-12-17 20:28   ` Mark Fasheh
2008-12-17 20:28   ` [Ocfs2-devel] " Mark Fasheh
2008-12-17 21:06   ` [xfs-masters] " Eric Sandeen
2008-12-17 21:06     ` [Ocfs2-devel] " Eric Sandeen
2008-12-17 21:06     ` Eric Sandeen
2008-12-17 21:15     ` Mark Fasheh
2008-12-17 21:16       ` [Ocfs2-devel] " Mark Fasheh
2008-12-17 21:15       ` Mark Fasheh
2008-12-18  9:50       ` Ankit Jain
2008-12-18  9:51         ` [Ocfs2-devel] " Ankit Jain
2008-12-18  9:50         ` Ankit Jain
2008-12-18  9:44   ` Ankit Jain [this message]
2008-12-18  9:44     ` Ankit Jain
2008-12-18  9:44     ` [Ocfs2-devel] " Ankit Jain
2008-12-18 21:01   ` Ankit Jain
2008-12-18 21:01     ` Ankit Jain
2008-12-18 21:01     ` [Ocfs2-devel] " Ankit Jain
2008-12-18  6:49 ` Felix Blyakher
2008-12-18  6:52   ` [Ocfs2-devel] " Felix Blyakher
2008-12-18  6:49   ` Felix Blyakher
2008-12-18  9:54   ` Ankit Jain
2008-12-18  9:55     ` [Ocfs2-devel] " Ankit Jain
2008-12-18  9:54     ` Ankit Jain

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=494A1B69.70300@ankitjain.org \
    --to=me@ankitjain.org \
    --cc=hch@infradead.org \
    --cc=joel.becker@oracle.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mfasheh@suse.com \
    --cc=ocfs2-devel@oss.oracle.com \
    --cc=viro@zeniv.linux.org.uk \
    --cc=xfs-masters@oss.sgi.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.