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

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

[Tristan Ye]

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__ */
 
-- 
1.5.5

[Ocfs2-devel] [PATCH 2/3] Ocfs2: Teach local-mounted ocfs2 to handle unwritten_extents correctly.

[Tristan Ye]

Oops, local-mounted of 'ocfs2_fops_no_plocks' is just missing the support
of unwritten_extents/punching-hole due to no func pointer was given correctly
to '.follocate' field.

Signed-off-by: Tristan Ye <tristan.ye@oracle.com>
---
 fs/ocfs2/file.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c
index 41565ae..cce8c2b 100644
--- a/fs/ocfs2/file.c
+++ b/fs/ocfs2/file.c
@@ -2658,6 +2658,7 @@ const struct file_operations ocfs2_fops_no_plocks = {
 	.flock		= ocfs2_flock,
 	.splice_read	= ocfs2_file_splice_read,
 	.splice_write	= ocfs2_file_splice_write,
+	.fallocate	= ocfs2_fallocate,
 };
 
 const struct file_operations ocfs2_dops_no_plocks = {
-- 
1.5.5

[Ocfs2-devel] [PATCH 3/3] Ocfs2: Cleanup for OCFS2_IOC_RESVSP & OCFS2_IOC_UNRESVSP.

[Tristan Ye]

Given that VFS has alreay supported an entry to handle the unwritten_extents
and punching-hole(is to be supported) by filp->f_op->fallocate(), our path of
OCFS2_IOC_RESVSP becomes a bit redundant somehow, especially given the fact
that ocfs2_fallocate() is working well.

Signed-off-by: Tristan Ye <tristan.ye@oracle.com>
---
 fs/ocfs2/file.c        |   63 ++++++++++++++++++++---------------------------
 fs/ocfs2/file.h        |    3 --
 fs/ocfs2/ioctl.c       |   13 ----------
 fs/ocfs2/ocfs2_ioctl.h |   29 +++-------------------
 4 files changed, 31 insertions(+), 77 deletions(-)

diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c
index cce8c2b..d016322 100644
--- a/fs/ocfs2/file.c
+++ b/fs/ocfs2/file.c
@@ -37,6 +37,7 @@
 #include <linux/falloc.h>
 #include <linux/quotaops.h>
 #include <linux/blkdev.h>
+#include <linux/falloc.h>
 
 #include <cluster/masklog.h>
 
@@ -1804,7 +1805,7 @@ out:
  */
 static int __ocfs2_change_file_space(struct file *file, struct inode *inode,
 				     loff_t f_pos, unsigned int cmd,
-				     struct ocfs2_space_resv *sr,
+				     struct space_resv *sr,
 				     int change_size)
 {
 	int ret;
@@ -1866,7 +1867,7 @@ static int __ocfs2_change_file_space(struct file *file, struct inode *inode,
 	}
 	size = sr->l_start + sr->l_len;
 
-	if (cmd == OCFS2_IOC_RESVSP || cmd == OCFS2_IOC_RESVSP64) {
+	if (cmd == FS_IOC_RESVSP || cmd == FS_IOC_RESVSP64) {
 		if (sr->l_len <= 0) {
 			ret = -EINVAL;
 			goto out_inode_unlock;
@@ -1883,8 +1884,8 @@ static int __ocfs2_change_file_space(struct file *file, struct inode *inode,
 
 	down_write(&OCFS2_I(inode)->ip_alloc_sem);
 	switch (cmd) {
-	case OCFS2_IOC_RESVSP:
-	case OCFS2_IOC_RESVSP64:
+	case FS_IOC_RESVSP:
+	case FS_IOC_RESVSP64:
 		/*
 		 * This takes unsigned offsets, but the signed ones we
 		 * pass have been checked against overflow above.
@@ -1892,8 +1893,8 @@ static int __ocfs2_change_file_space(struct file *file, struct inode *inode,
 		ret = ocfs2_allocate_unwritten_extents(inode, sr->l_start,
 						       sr->l_len);
 		break;
-	case OCFS2_IOC_UNRESVSP:
-	case OCFS2_IOC_UNRESVSP64:
+	case FS_IOC_UNRESVSP:
+	case FS_IOC_UNRESVSP64:
 		ret = ocfs2_remove_inode_range(inode, di_bh, sr->l_start,
 					       sr->l_len);
 		break;
@@ -1937,47 +1938,37 @@ out:
 	return ret;
 }
 
-int ocfs2_change_file_space(struct file *file, unsigned int cmd,
-			    struct ocfs2_space_resv *sr)
-{
-	struct inode *inode = file->f_path.dentry->d_inode;
-	struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
-
-	if ((cmd == OCFS2_IOC_RESVSP || cmd == OCFS2_IOC_RESVSP64) &&
-	    !ocfs2_writes_unwritten_extents(osb))
-		return -ENOTTY;
-	else if ((cmd == OCFS2_IOC_UNRESVSP || cmd == OCFS2_IOC_UNRESVSP64) &&
-		 !ocfs2_sparse_alloc(osb))
-		return -ENOTTY;
-
-	if (!S_ISREG(inode->i_mode))
-		return -EINVAL;
-
-	if (!(file->f_mode & FMODE_WRITE))
-		return -EBADF;
-
-	return __ocfs2_change_file_space(file, inode, file->f_pos, cmd, sr, 0);
-}
-
 static long ocfs2_fallocate(struct file *file, int mode, loff_t offset,
 			    loff_t len)
 {
 	struct inode *inode = file->f_path.dentry->d_inode;
 	struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
-	struct ocfs2_space_resv sr;
+	struct space_resv sr;
 	int change_size = 1;
-	int cmd = OCFS2_IOC_RESVSP64;
+	int cmd = FS_IOC_RESVSP64;
 
 	if (mode & ~(FALLOC_FL_KEEP_SIZE | FALLOC_FL_PUNCH_HOLE))
 		return -EOPNOTSUPP;
-	if (!ocfs2_writes_unwritten_extents(osb))
-		return -EOPNOTSUPP;
 
-	if (mode & FALLOC_FL_KEEP_SIZE)
-		change_size = 0;
+	/*
+	 * unwritten extents
+	 */
+	if ((mode & FALLOC_FL_KEEP_SIZE) && !(mode & FALLOC_FL_PUNCH_HOLE)) {
+		if (ocfs2_writes_unwritten_extents(osb))
+			change_size = 0;
+		else
+			return -EOPNOTSUPP;
+	}
 
-	if (mode & FALLOC_FL_PUNCH_HOLE)
-		cmd = OCFS2_IOC_UNRESVSP64;
+	/*
+	 * punching hole
+	 */
+	if ((mode & FALLOC_FL_KEEP_SIZE) && (mode & FALLOC_FL_PUNCH_HOLE)) {
+		if (ocfs2_sparse_alloc(osb))
+			cmd = FS_IOC_UNRESVSP64;
+		else
+			return -EOPNOTSUPP;
+	}
 
 	sr.l_whence = 0;
 	sr.l_start = (s64)offset;
diff --git a/fs/ocfs2/file.h b/fs/ocfs2/file.h
index f5afbbe..2c9b7a8 100644
--- a/fs/ocfs2/file.h
+++ b/fs/ocfs2/file.h
@@ -68,9 +68,6 @@ int ocfs2_should_update_atime(struct inode *inode,
 int ocfs2_update_inode_atime(struct inode *inode,
 			     struct buffer_head *bh);
 
-int ocfs2_change_file_space(struct file *file, unsigned int cmd,
-			    struct ocfs2_space_resv *sr);
-
 int ocfs2_check_range_for_refcount(struct inode *inode, loff_t pos,
 				   size_t count);
 #endif /* OCFS2_FILE_H */
diff --git a/fs/ocfs2/ioctl.c b/fs/ocfs2/ioctl.c
index 8f13c59..d703210 100644
--- a/fs/ocfs2/ioctl.c
+++ b/fs/ocfs2/ioctl.c
@@ -476,7 +476,6 @@ long ocfs2_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 	unsigned int flags;
 	int new_clusters;
 	int status;
-	struct ocfs2_space_resv sr;
 	struct ocfs2_new_group_input input;
 	struct reflink_arguments args;
 	const char *old_path, *new_path;
@@ -502,14 +501,6 @@ long ocfs2_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 			OCFS2_FL_MODIFIABLE);
 		mnt_drop_write(filp->f_path.mnt);
 		return status;
-	case OCFS2_IOC_RESVSP:
-	case OCFS2_IOC_RESVSP64:
-	case OCFS2_IOC_UNRESVSP:
-	case OCFS2_IOC_UNRESVSP64:
-		if (copy_from_user(&sr, (int __user *) arg, sizeof(sr)))
-			return -EFAULT;
-
-		return ocfs2_change_file_space(filp, cmd, &sr);
 	case OCFS2_IOC_GROUP_EXTEND:
 		if (!capable(CAP_SYS_RESOURCE))
 			return -EPERM;
@@ -562,10 +553,6 @@ long ocfs2_compat_ioctl(struct file *file, unsigned cmd, unsigned long arg)
 	case OCFS2_IOC32_SETFLAGS:
 		cmd = OCFS2_IOC_SETFLAGS;
 		break;
-	case OCFS2_IOC_RESVSP:
-	case OCFS2_IOC_RESVSP64:
-	case OCFS2_IOC_UNRESVSP:
-	case OCFS2_IOC_UNRESVSP64:
 	case OCFS2_IOC_GROUP_EXTEND:
 	case OCFS2_IOC_GROUP_ADD:
 	case OCFS2_IOC_GROUP_ADD64:
diff --git a/fs/ocfs2/ocfs2_ioctl.h b/fs/ocfs2/ocfs2_ioctl.h
index b46f39b..4f9bf28 100644
--- a/fs/ocfs2/ocfs2_ioctl.h
+++ b/fs/ocfs2/ocfs2_ioctl.h
@@ -28,31 +28,10 @@
 #define OCFS2_IOC32_GETFLAGS	FS_IOC32_GETFLAGS
 #define OCFS2_IOC32_SETFLAGS	FS_IOC32_SETFLAGS
 
-/*
- * Space reservation / allocation / free ioctls and argument structure
- * are designed to be compatible with XFS.
- *
- * ALLOCSP* and FREESP* are not and will never be supported, but are
- * included here for completeness.
- */
-struct ocfs2_space_resv {
-	__s16		l_type;
-	__s16		l_whence;
-	__s64		l_start;
-	__s64		l_len;		/* len == 0 means until end of file */
-	__s32		l_sysid;
-	__u32		l_pid;
-	__s32		l_pad[4];	/* reserve area			    */
-};
-
-#define OCFS2_IOC_ALLOCSP		_IOW ('X', 10, struct ocfs2_space_resv)
-#define OCFS2_IOC_FREESP		_IOW ('X', 11, struct ocfs2_space_resv)
-#define OCFS2_IOC_RESVSP		_IOW ('X', 40, struct ocfs2_space_resv)
-#define OCFS2_IOC_UNRESVSP	_IOW ('X', 41, struct ocfs2_space_resv)
-#define OCFS2_IOC_ALLOCSP64	_IOW ('X', 36, struct ocfs2_space_resv)
-#define OCFS2_IOC_FREESP64	_IOW ('X', 37, struct ocfs2_space_resv)
-#define OCFS2_IOC_RESVSP64	_IOW ('X', 42, struct ocfs2_space_resv)
-#define OCFS2_IOC_UNRESVSP64	_IOW ('X', 43, struct ocfs2_space_resv)
+#define OCFS2_IOC_ALLOCSP		_IOW ('X', 10, struct space_resv)
+#define OCFS2_IOC_FREESP		_IOW ('X', 11, struct space_resv)
+#define OCFS2_IOC_ALLOCSP64	_IOW ('X', 36, struct space_resv)
+#define OCFS2_IOC_FREESP64	_IOW ('X', 37, struct space_resv)
 
 /* Used to pass group descriptor data when online resize is done */
 struct ocfs2_new_group_input {
-- 
1.5.5

[Ocfs2-devel] [PATCH 2/3] Ocfs2: Teach local-mounted ocfs2 to handle unwritten_extents correctly.

[Sunil Mushran]

Acked-by: Sunil Mushran <sunil.mushran@oracle.com>

BTW, this affects the o2cb stack too. Not just local mounts.

Also, cc stable at kernel.org as this problem was introduced in
2.6.38 by 2fe17c10.

On 03/31/2011 12:34 AM, Tristan Ye wrote:
> Oops, local-mounted of 'ocfs2_fops_no_plocks' is just missing the support
> of unwritten_extents/punching-hole due to no func pointer was given correctly
> to '.follocate' field.
>
> Signed-off-by: Tristan Ye<tristan.ye@oracle.com>
> ---
>   fs/ocfs2/file.c |    1 +
>   1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c
> index 41565ae..cce8c2b 100644
> --- a/fs/ocfs2/file.c
> +++ b/fs/ocfs2/file.c
> @@ -2658,6 +2658,7 @@ const struct file_operations ocfs2_fops_no_plocks = {
>   	.flock		= ocfs2_flock,
>   	.splice_read	= ocfs2_file_splice_read,
>   	.splice_write	= ocfs2_file_splice_write,
> +	.fallocate	= ocfs2_fallocate,
>   };
>
>   const struct file_operations ocfs2_dops_no_plocks = {

[Ocfs2-devel] [PATCH 3/3] Ocfs2: Cleanup for OCFS2_IOC_RESVSP & OCFS2_IOC_UNRESVSP.

[Sunil Mushran]

On 03/31/2011 12:34 AM, Tristan Ye wrote:
> Given that VFS has alreay supported an entry to handle the unwritten_extents
> and punching-hole(is to be supported) by filp->f_op->fallocate(), our path of
> OCFS2_IOC_RESVSP becomes a bit redundant somehow, especially given the fact
> that ocfs2_fallocate() is working well.
>
> Signed-off-by: Tristan Ye<tristan.ye@oracle.com>
> ---
>   fs/ocfs2/file.c        |   63 ++++++++++++++++++++---------------------------
>   fs/ocfs2/file.h        |    3 --
>   fs/ocfs2/ioctl.c       |   13 ----------
>   fs/ocfs2/ocfs2_ioctl.h |   29 +++-------------------
>   4 files changed, 31 insertions(+), 77 deletions(-)
>
> diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c
> index cce8c2b..d016322 100644
> --- a/fs/ocfs2/file.c
> +++ b/fs/ocfs2/file.c
> @@ -37,6 +37,7 @@
>   #include<linux/falloc.h>
>   #include<linux/quotaops.h>
>   #include<linux/blkdev.h>
> +#include<linux/falloc.h>
>
>   #include<cluster/masklog.h>
>
> @@ -1804,7 +1805,7 @@ out:
>    */
>   static int __ocfs2_change_file_space(struct file *file, struct inode *inode,
>   				     loff_t f_pos, unsigned int cmd,
> -				     struct ocfs2_space_resv *sr,
> +				     struct space_resv *sr,
>   				     int change_size)
>   {
>   	int ret;
> @@ -1866,7 +1867,7 @@ static int __ocfs2_change_file_space(struct file *file, struct inode *inode,
>   	}
>   	size = sr->l_start + sr->l_len;
>
> -	if (cmd == OCFS2_IOC_RESVSP || cmd == OCFS2_IOC_RESVSP64) {
> +	if (cmd == FS_IOC_RESVSP || cmd == FS_IOC_RESVSP64) {
>   		if (sr->l_len<= 0) {
>   			ret = -EINVAL;
>   			goto out_inode_unlock;
> @@ -1883,8 +1884,8 @@ static int __ocfs2_change_file_space(struct file *file, struct inode *inode,
>
>   	down_write(&OCFS2_I(inode)->ip_alloc_sem);
>   	switch (cmd) {
> -	case OCFS2_IOC_RESVSP:
> -	case OCFS2_IOC_RESVSP64:
> +	case FS_IOC_RESVSP:
> +	case FS_IOC_RESVSP64:
>   		/*
>   		 * This takes unsigned offsets, but the signed ones we
>   		 * pass have been checked against overflow above.
> @@ -1892,8 +1893,8 @@ static int __ocfs2_change_file_space(struct file *file, struct inode *inode,
>   		ret = ocfs2_allocate_unwritten_extents(inode, sr->l_start,
>   						       sr->l_len);
>   		break;
> -	case OCFS2_IOC_UNRESVSP:
> -	case OCFS2_IOC_UNRESVSP64:
> +	case FS_IOC_UNRESVSP:
> +	case FS_IOC_UNRESVSP64:
>   		ret = ocfs2_remove_inode_range(inode, di_bh, sr->l_start,
>   					       sr->l_len);
>   		break;
> @@ -1937,47 +1938,37 @@ out:
>   	return ret;
>   }
>
> -int ocfs2_change_file_space(struct file *file, unsigned int cmd,
> -			    struct ocfs2_space_resv *sr)
> -{
> -	struct inode *inode = file->f_path.dentry->d_inode;
> -	struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
> -
> -	if ((cmd == OCFS2_IOC_RESVSP || cmd == OCFS2_IOC_RESVSP64)&&
> -	    !ocfs2_writes_unwritten_extents(osb))
> -		return -ENOTTY;
> -	else if ((cmd == OCFS2_IOC_UNRESVSP || cmd == OCFS2_IOC_UNRESVSP64)&&
> -		 !ocfs2_sparse_alloc(osb))
> -		return -ENOTTY;
> -
> -	if (!S_ISREG(inode->i_mode))
> -		return -EINVAL;
> -
> -	if (!(file->f_mode&  FMODE_WRITE))
> -		return -EBADF;
> -
> -	return __ocfs2_change_file_space(file, inode, file->f_pos, cmd, sr, 0);
> -}
> -
>   static long ocfs2_fallocate(struct file *file, int mode, loff_t offset,
>   			    loff_t len)
>   {
>   	struct inode *inode = file->f_path.dentry->d_inode;
>   	struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
> -	struct ocfs2_space_resv sr;
> +	struct space_resv sr;
>   	int change_size = 1;
> -	int cmd = OCFS2_IOC_RESVSP64;
> +	int cmd = FS_IOC_RESVSP64;
>
>   	if (mode&  ~(FALLOC_FL_KEEP_SIZE | FALLOC_FL_PUNCH_HOLE))
>   		return -EOPNOTSUPP;
> -	if (!ocfs2_writes_unwritten_extents(osb))
> -		return -EOPNOTSUPP;
>
> -	if (mode&  FALLOC_FL_KEEP_SIZE)
> -		change_size = 0;
> +	/*
> +	 * unwritten extents
> +	 */
> +	if ((mode&  FALLOC_FL_KEEP_SIZE)&&  !(mode&  FALLOC_FL_PUNCH_HOLE)) {
> +		if (ocfs2_writes_unwritten_extents(osb))
> +			change_size = 0;
> +		else
> +			return -EOPNOTSUPP;
> +	}

I think you have made the code harder to read that it was previously.


>
> -	if (mode&  FALLOC_FL_PUNCH_HOLE)
> -		cmd = OCFS2_IOC_UNRESVSP64;
> +	/*
> +	 * punching hole
> +	 */
> +	if ((mode&  FALLOC_FL_KEEP_SIZE)&&  (mode&  FALLOC_FL_PUNCH_HOLE)) {
> +		if (ocfs2_sparse_alloc(osb))
> +			cmd = FS_IOC_UNRESVSP64;
> +		else
> +			return -EOPNOTSUPP;
> +	}
>
>   	sr.l_whence = 0;
>   	sr.l_start = (s64)offset;
> diff --git a/fs/ocfs2/file.h b/fs/ocfs2/file.h
> index f5afbbe..2c9b7a8 100644
> --- a/fs/ocfs2/file.h
> +++ b/fs/ocfs2/file.h
> @@ -68,9 +68,6 @@ int ocfs2_should_update_atime(struct inode *inode,
>   int ocfs2_update_inode_atime(struct inode *inode,
>   			     struct buffer_head *bh);
>
> -int ocfs2_change_file_space(struct file *file, unsigned int cmd,
> -			    struct ocfs2_space_resv *sr);
> -
>   int ocfs2_check_range_for_refcount(struct inode *inode, loff_t pos,
>   				   size_t count);
>   #endif /* OCFS2_FILE_H */
> diff --git a/fs/ocfs2/ioctl.c b/fs/ocfs2/ioctl.c
> index 8f13c59..d703210 100644
> --- a/fs/ocfs2/ioctl.c
> +++ b/fs/ocfs2/ioctl.c
> @@ -476,7 +476,6 @@ long ocfs2_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
>   	unsigned int flags;
>   	int new_clusters;
>   	int status;
> -	struct ocfs2_space_resv sr;
>   	struct ocfs2_new_group_input input;
>   	struct reflink_arguments args;
>   	const char *old_path, *new_path;
> @@ -502,14 +501,6 @@ long ocfs2_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
>   			OCFS2_FL_MODIFIABLE);
>   		mnt_drop_write(filp->f_path.mnt);
>   		return status;
> -	case OCFS2_IOC_RESVSP:
> -	case OCFS2_IOC_RESVSP64:
> -	case OCFS2_IOC_UNRESVSP:
> -	case OCFS2_IOC_UNRESVSP64:
> -		if (copy_from_user(&sr, (int __user *) arg, sizeof(sr)))
> -			return -EFAULT;
> -
> -		return ocfs2_change_file_space(filp, cmd,&sr);
>   	case OCFS2_IOC_GROUP_EXTEND:
>   		if (!capable(CAP_SYS_RESOURCE))
>   			return -EPERM;
> @@ -562,10 +553,6 @@ long ocfs2_compat_ioctl(struct file *file, unsigned cmd, unsigned long arg)
>   	case OCFS2_IOC32_SETFLAGS:
>   		cmd = OCFS2_IOC_SETFLAGS;
>   		break;
> -	case OCFS2_IOC_RESVSP:
> -	case OCFS2_IOC_RESVSP64:
> -	case OCFS2_IOC_UNRESVSP:
> -	case OCFS2_IOC_UNRESVSP64:
>   	case OCFS2_IOC_GROUP_EXTEND:
>   	case OCFS2_IOC_GROUP_ADD:
>   	case OCFS2_IOC_GROUP_ADD64:
> diff --git a/fs/ocfs2/ocfs2_ioctl.h b/fs/ocfs2/ocfs2_ioctl.h
> index b46f39b..4f9bf28 100644
> --- a/fs/ocfs2/ocfs2_ioctl.h
> +++ b/fs/ocfs2/ocfs2_ioctl.h
> @@ -28,31 +28,10 @@
>   #define OCFS2_IOC32_GETFLAGS	FS_IOC32_GETFLAGS
>   #define OCFS2_IOC32_SETFLAGS	FS_IOC32_SETFLAGS
>
> -/*
> - * Space reservation / allocation / free ioctls and argument structure
> - * are designed to be compatible with XFS.
> - *
> - * ALLOCSP* and FREESP* are not and will never be supported, but are
> - * included here for completeness.
> - */
> -struct ocfs2_space_resv {
> -	__s16		l_type;
> -	__s16		l_whence;
> -	__s64		l_start;
> -	__s64		l_len;		/* len == 0 means until end of file */
> -	__s32		l_sysid;
> -	__u32		l_pid;
> -	__s32		l_pad[4];	/* reserve area			    */
> -};
> -
> -#define OCFS2_IOC_ALLOCSP		_IOW ('X', 10, struct ocfs2_space_resv)
> -#define OCFS2_IOC_FREESP		_IOW ('X', 11, struct ocfs2_space_resv)
> -#define OCFS2_IOC_RESVSP		_IOW ('X', 40, struct ocfs2_space_resv)
> -#define OCFS2_IOC_UNRESVSP	_IOW ('X', 41, struct ocfs2_space_resv)
> -#define OCFS2_IOC_ALLOCSP64	_IOW ('X', 36, struct ocfs2_space_resv)
> -#define OCFS2_IOC_FREESP64	_IOW ('X', 37, struct ocfs2_space_resv)
> -#define OCFS2_IOC_RESVSP64	_IOW ('X', 42, struct ocfs2_space_resv)
> -#define OCFS2_IOC_UNRESVSP64	_IOW ('X', 43, struct ocfs2_space_resv)
> +#define OCFS2_IOC_ALLOCSP		_IOW ('X', 10, struct space_resv)
> +#define OCFS2_IOC_FREESP		_IOW ('X', 11, struct space_resv)
> +#define OCFS2_IOC_ALLOCSP64	_IOW ('X', 36, struct space_resv)
> +#define OCFS2_IOC_FREESP64	_IOW ('X', 37, struct space_resv)
>
>   /* Used to pass group descriptor data when online resize is done */
>   struct ocfs2_new_group_input {

[Ocfs2-devel] [PATCH 3/3] Ocfs2: Cleanup for OCFS2_IOC_RESVSP & OCFS2_IOC_UNRESVSP.

[Tristan Ye]

Sunil Mushran wrote:
> On 03/31/2011 12:34 AM, Tristan Ye wrote:
>> Given that VFS has alreay supported an entry to handle the
>> unwritten_extents
>> and punching-hole(is to be supported) by filp->f_op->fallocate(), our
>> path of
>> OCFS2_IOC_RESVSP becomes a bit redundant somehow, especially given the
>> fact
>> that ocfs2_fallocate() is working well.
>>
>> Signed-off-by: Tristan Ye<tristan.ye@oracle.com>
>> ---
>>   fs/ocfs2/file.c        |   63
>> ++++++++++++++++++++---------------------------
>>   fs/ocfs2/file.h        |    3 --
>>   fs/ocfs2/ioctl.c       |   13 ----------
>>   fs/ocfs2/ocfs2_ioctl.h |   29 +++-------------------
>>   4 files changed, 31 insertions(+), 77 deletions(-)
>>
>> diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c
>> index cce8c2b..d016322 100644
>> --- a/fs/ocfs2/file.c
>> +++ b/fs/ocfs2/file.c
>> @@ -37,6 +37,7 @@
>>   #include<linux/falloc.h>
>>   #include<linux/quotaops.h>
>>   #include<linux/blkdev.h>
>> +#include<linux/falloc.h>
>>
>>   #include<cluster/masklog.h>
>>
>> @@ -1804,7 +1805,7 @@ out:
>>    */
>>   static int __ocfs2_change_file_space(struct file *file, struct inode
>> *inode,
>>                        loff_t f_pos, unsigned int cmd,
>> -                     struct ocfs2_space_resv *sr,
>> +                     struct space_resv *sr,
>>                        int change_size)
>>   {
>>       int ret;
>> @@ -1866,7 +1867,7 @@ static int __ocfs2_change_file_space(struct file
>> *file, struct inode *inode,
>>       }
>>       size = sr->l_start + sr->l_len;
>>
>> -    if (cmd == OCFS2_IOC_RESVSP || cmd == OCFS2_IOC_RESVSP64) {
>> +    if (cmd == FS_IOC_RESVSP || cmd == FS_IOC_RESVSP64) {
>>           if (sr->l_len<= 0) {
>>               ret = -EINVAL;
>>               goto out_inode_unlock;
>> @@ -1883,8 +1884,8 @@ static int __ocfs2_change_file_space(struct file
>> *file, struct inode *inode,
>>
>>       down_write(&OCFS2_I(inode)->ip_alloc_sem);
>>       switch (cmd) {
>> -    case OCFS2_IOC_RESVSP:
>> -    case OCFS2_IOC_RESVSP64:
>> +    case FS_IOC_RESVSP:
>> +    case FS_IOC_RESVSP64:
>>           /*
>>            * This takes unsigned offsets, but the signed ones we
>>            * pass have been checked against overflow above.
>> @@ -1892,8 +1893,8 @@ static int __ocfs2_change_file_space(struct file
>> *file, struct inode *inode,
>>           ret = ocfs2_allocate_unwritten_extents(inode, sr->l_start,
>>                                  sr->l_len);
>>           break;
>> -    case OCFS2_IOC_UNRESVSP:
>> -    case OCFS2_IOC_UNRESVSP64:
>> +    case FS_IOC_UNRESVSP:
>> +    case FS_IOC_UNRESVSP64:
>>           ret = ocfs2_remove_inode_range(inode, di_bh, sr->l_start,
>>                              sr->l_len);
>>           break;
>> @@ -1937,47 +1938,37 @@ out:
>>       return ret;
>>   }
>>
>> -int ocfs2_change_file_space(struct file *file, unsigned int cmd,
>> -                struct ocfs2_space_resv *sr)
>> -{
>> -    struct inode *inode = file->f_path.dentry->d_inode;
>> -    struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
>> -
>> -    if ((cmd == OCFS2_IOC_RESVSP || cmd == OCFS2_IOC_RESVSP64)&&
>> -        !ocfs2_writes_unwritten_extents(osb))
>> -        return -ENOTTY;
>> -    else if ((cmd == OCFS2_IOC_UNRESVSP || cmd ==
>> OCFS2_IOC_UNRESVSP64)&&
>> -         !ocfs2_sparse_alloc(osb))
>> -        return -ENOTTY;
>> -
>> -    if (!S_ISREG(inode->i_mode))
>> -        return -EINVAL;
>> -
>> -    if (!(file->f_mode&  FMODE_WRITE))
>> -        return -EBADF;
>> -
>> -    return __ocfs2_change_file_space(file, inode, file->f_pos, cmd,
>> sr, 0);
>> -}
>> -
>>   static long ocfs2_fallocate(struct file *file, int mode, loff_t offset,
>>                   loff_t len)
>>   {
>>       struct inode *inode = file->f_path.dentry->d_inode;
>>       struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
>> -    struct ocfs2_space_resv sr;
>> +    struct space_resv sr;
>>       int change_size = 1;
>> -    int cmd = OCFS2_IOC_RESVSP64;
>> +    int cmd = FS_IOC_RESVSP64;
>>
>>       if (mode&  ~(FALLOC_FL_KEEP_SIZE | FALLOC_FL_PUNCH_HOLE))
>>           return -EOPNOTSUPP;
>> -    if (!ocfs2_writes_unwritten_extents(osb))
>> -        return -EOPNOTSUPP;
>>
>> -    if (mode&  FALLOC_FL_KEEP_SIZE)
>> -        change_size = 0;
>> +    /*
>> +     * unwritten extents
>> +     */
>> +    if ((mode&  FALLOC_FL_KEEP_SIZE)&&  !(mode& 
>> FALLOC_FL_PUNCH_HOLE)) {
>> +        if (ocfs2_writes_unwritten_extents(osb))
>> +            change_size = 0;
>> +        else
>> +            return -EOPNOTSUPP;
>> +    }
> 
> I think you have made the code harder to read that it was previously.

	Those changes were a bit coherent to previous patch of adding punching-hole
support to VFS ioctl(), which needs punching-hole to have both
FALLOC_FL_KEEP_SIZE and FALLOC_FL_PUNCH_HOLE set, on the other hand, however
you're right, I may have to make those changes in a more neat way.

	Given the fact that VFS already have two paths doing
preallocation/punching-hole(maybe they're going to fire one), ways from ocfs2's
specific ioctl() to accomplish this really can be deprecated.

Tristan.

> 
> 
>>
>> -    if (mode&  FALLOC_FL_PUNCH_HOLE)
>> -        cmd = OCFS2_IOC_UNRESVSP64;
>> +    /*
>> +     * punching hole
>> +     */
>> +    if ((mode&  FALLOC_FL_KEEP_SIZE)&&  (mode&  FALLOC_FL_PUNCH_HOLE)) {
>> +        if (ocfs2_sparse_alloc(osb))
>> +            cmd = FS_IOC_UNRESVSP64;
>> +        else
>> +            return -EOPNOTSUPP;
>> +    }
>>
>>       sr.l_whence = 0;
>>       sr.l_start = (s64)offset;
>> diff --git a/fs/ocfs2/file.h b/fs/ocfs2/file.h
>> index f5afbbe..2c9b7a8 100644
>> --- a/fs/ocfs2/file.h
>> +++ b/fs/ocfs2/file.h
>> @@ -68,9 +68,6 @@ int ocfs2_should_update_atime(struct inode *inode,
>>   int ocfs2_update_inode_atime(struct inode *inode,
>>                    struct buffer_head *bh);
>>
>> -int ocfs2_change_file_space(struct file *file, unsigned int cmd,
>> -                struct ocfs2_space_resv *sr);
>> -
>>   int ocfs2_check_range_for_refcount(struct inode *inode, loff_t pos,
>>                      size_t count);
>>   #endif /* OCFS2_FILE_H */
>> diff --git a/fs/ocfs2/ioctl.c b/fs/ocfs2/ioctl.c
>> index 8f13c59..d703210 100644
>> --- a/fs/ocfs2/ioctl.c
>> +++ b/fs/ocfs2/ioctl.c
>> @@ -476,7 +476,6 @@ long ocfs2_ioctl(struct file *filp, unsigned int
>> cmd, unsigned long arg)
>>       unsigned int flags;
>>       int new_clusters;
>>       int status;
>> -    struct ocfs2_space_resv sr;
>>       struct ocfs2_new_group_input input;
>>       struct reflink_arguments args;
>>       const char *old_path, *new_path;
>> @@ -502,14 +501,6 @@ long ocfs2_ioctl(struct file *filp, unsigned int
>> cmd, unsigned long arg)
>>               OCFS2_FL_MODIFIABLE);
>>           mnt_drop_write(filp->f_path.mnt);
>>           return status;
>> -    case OCFS2_IOC_RESVSP:
>> -    case OCFS2_IOC_RESVSP64:
>> -    case OCFS2_IOC_UNRESVSP:
>> -    case OCFS2_IOC_UNRESVSP64:
>> -        if (copy_from_user(&sr, (int __user *) arg, sizeof(sr)))
>> -            return -EFAULT;
>> -
>> -        return ocfs2_change_file_space(filp, cmd,&sr);
>>       case OCFS2_IOC_GROUP_EXTEND:
>>           if (!capable(CAP_SYS_RESOURCE))
>>               return -EPERM;
>> @@ -562,10 +553,6 @@ long ocfs2_compat_ioctl(struct file *file,
>> unsigned cmd, unsigned long arg)
>>       case OCFS2_IOC32_SETFLAGS:
>>           cmd = OCFS2_IOC_SETFLAGS;
>>           break;
>> -    case OCFS2_IOC_RESVSP:
>> -    case OCFS2_IOC_RESVSP64:
>> -    case OCFS2_IOC_UNRESVSP:
>> -    case OCFS2_IOC_UNRESVSP64:
>>       case OCFS2_IOC_GROUP_EXTEND:
>>       case OCFS2_IOC_GROUP_ADD:
>>       case OCFS2_IOC_GROUP_ADD64:
>> diff --git a/fs/ocfs2/ocfs2_ioctl.h b/fs/ocfs2/ocfs2_ioctl.h
>> index b46f39b..4f9bf28 100644
>> --- a/fs/ocfs2/ocfs2_ioctl.h
>> +++ b/fs/ocfs2/ocfs2_ioctl.h
>> @@ -28,31 +28,10 @@
>>   #define OCFS2_IOC32_GETFLAGS    FS_IOC32_GETFLAGS
>>   #define OCFS2_IOC32_SETFLAGS    FS_IOC32_SETFLAGS
>>
>> -/*
>> - * Space reservation / allocation / free ioctls and argument structure
>> - * are designed to be compatible with XFS.
>> - *
>> - * ALLOCSP* and FREESP* are not and will never be supported, but are
>> - * included here for completeness.
>> - */
>> -struct ocfs2_space_resv {
>> -    __s16        l_type;
>> -    __s16        l_whence;
>> -    __s64        l_start;
>> -    __s64        l_len;        /* len == 0 means until end of file */
>> -    __s32        l_sysid;
>> -    __u32        l_pid;
>> -    __s32        l_pad[4];    /* reserve area                */
>> -};
>> -
>> -#define OCFS2_IOC_ALLOCSP        _IOW ('X', 10, struct ocfs2_space_resv)
>> -#define OCFS2_IOC_FREESP        _IOW ('X', 11, struct ocfs2_space_resv)
>> -#define OCFS2_IOC_RESVSP        _IOW ('X', 40, struct ocfs2_space_resv)
>> -#define OCFS2_IOC_UNRESVSP    _IOW ('X', 41, struct ocfs2_space_resv)
>> -#define OCFS2_IOC_ALLOCSP64    _IOW ('X', 36, struct ocfs2_space_resv)
>> -#define OCFS2_IOC_FREESP64    _IOW ('X', 37, struct ocfs2_space_resv)
>> -#define OCFS2_IOC_RESVSP64    _IOW ('X', 42, struct ocfs2_space_resv)
>> -#define OCFS2_IOC_UNRESVSP64    _IOW ('X', 43, struct ocfs2_space_resv)
>> +#define OCFS2_IOC_ALLOCSP        _IOW ('X', 10, struct space_resv)
>> +#define OCFS2_IOC_FREESP        _IOW ('X', 11, struct space_resv)
>> +#define OCFS2_IOC_ALLOCSP64    _IOW ('X', 36, struct space_resv)
>> +#define OCFS2_IOC_FREESP64    _IOW ('X', 37, struct space_resv)
>>
>>   /* Used to pass group descriptor data when online resize is done */
>>   struct ocfs2_new_group_input {
> 

[Ocfs2-devel] [PATCH 3/3] Ocfs2: Cleanup for OCFS2_IOC_RESVSP & OCFS2_IOC_UNRESVSP.

[Sunil Mushran]

On 3/31/2011 7:46 PM, Tristan Ye wrote:
>
> 	Those changes were a bit coherent to previous patch of adding punching-hole
> support to VFS ioctl(), which needs punching-hole to have both
> FALLOC_FL_KEEP_SIZE and FALLOC_FL_PUNCH_HOLE set, on the other hand, however
> you're right, I may have to make those changes in a more neat way.
>
> 	Given the fact that VFS already have two paths doing
> preallocation/punching-hole(maybe they're going to fire one), ways from ocfs2's
> specific ioctl() to accomplish this really can be deprecated.

Agreed.

[Ocfs2-devel] [PATCH 2/3] Ocfs2: Teach local-mounted ocfs2 to handle unwritten_extents correctly.

[Joel Becker]

On Thu, Mar 31, 2011 at 02:06:43PM -0700, Sunil Mushran wrote:
> Acked-by: Sunil Mushran <sunil.mushran@oracle.com>
> 
> BTW, this affects the o2cb stack too. Not just local mounts.

	I don't have this patch series in my inbox.  Damned Oracle
email.  Tristan, can you resend to me?

Joel

-- 

"The nearest approach to immortality on Earth is a government
 bureau."
	- James F. Byrnes

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

[Ocfs2-devel] [PATCH 3/3] Ocfs2: Cleanup for OCFS2_IOC_RESVSP & OCFS2_IOC_UNRESVSP.

[Joel Becker]

On Thu, Mar 31, 2011 at 02:13:41PM -0700, Sunil Mushran wrote:
> On 03/31/2011 12:34 AM, Tristan Ye wrote:
> >Given that VFS has alreay supported an entry to handle the unwritten_extents
> >and punching-hole(is to be supported) by filp->f_op->fallocate(), our path of
> >OCFS2_IOC_RESVSP becomes a bit redundant somehow, especially given the fact
> >that ocfs2_fallocate() is working well.

Josef is planning to genericize the RESVP code via fallocate().  Let's
wait until he is done to clean up our side.

Joel

-- 

"But then she looks me in the eye
 And says, 'We're going to last forever,'
 And man you know I can't begin to doubt it.
 Cause it just feels so good and so free and so right,
 I know we ain't never going to change our minds about it, Hey!
 Here comes my girl."

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