ocfs2-devel.oss.oracle.com archive mirror
 help / color / mirror / Atom feed
* [Ocfs2-devel] [PATCH 1/3] VFS/ioctl: Add punching-hole support to ioctl().
@ 2011-03-31  7:33 Tristan Ye
  2011-03-31 21:14 ` Sunil Mushran
  0 siblings, 1 reply; 15+ messages in thread
From: Tristan Ye @ 2011-03-31  7:33 UTC (permalink / raw)
  To: ocfs2-devel, josef, linux-kernel, linux-fsdevel, linux-btrfs; +Cc: chris.mason

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

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

* [Ocfs2-devel] [PATCH 1/3] VFS/ioctl: Add punching-hole support to ioctl().
@ 2011-03-31  7:34 Tristan Ye
  2011-03-31  7:34 ` [Ocfs2-devel] [PATCH 2/3] Ocfs2: Teach local-mounted ocfs2 to handle unwritten_extents correctly Tristan Ye
  2011-03-31  7:34 ` [Ocfs2-devel] [PATCH 3/3] Ocfs2: Cleanup for OCFS2_IOC_RESVSP & OCFS2_IOC_UNRESVSP Tristan Ye
  0 siblings, 2 replies; 15+ messages in thread
From: Tristan Ye @ 2011-03-31  7:34 UTC (permalink / raw)
  To: ocfs2-devel

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

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

* [Ocfs2-devel] [PATCH 2/3] Ocfs2: Teach local-mounted ocfs2 to handle unwritten_extents correctly.
  2011-03-31  7:34 [Ocfs2-devel] [PATCH 1/3] VFS/ioctl: Add punching-hole support to ioctl() Tristan Ye
@ 2011-03-31  7:34 ` Tristan Ye
  2011-03-31 21:06   ` Sunil Mushran
  2011-03-31  7:34 ` [Ocfs2-devel] [PATCH 3/3] Ocfs2: Cleanup for OCFS2_IOC_RESVSP & OCFS2_IOC_UNRESVSP Tristan Ye
  1 sibling, 1 reply; 15+ messages in thread
From: Tristan Ye @ 2011-03-31  7:34 UTC (permalink / raw)
  To: ocfs2-devel

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

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

* [Ocfs2-devel] [PATCH 3/3] Ocfs2: Cleanup for OCFS2_IOC_RESVSP & OCFS2_IOC_UNRESVSP.
  2011-03-31  7:34 [Ocfs2-devel] [PATCH 1/3] VFS/ioctl: Add punching-hole support to ioctl() Tristan Ye
  2011-03-31  7:34 ` [Ocfs2-devel] [PATCH 2/3] Ocfs2: Teach local-mounted ocfs2 to handle unwritten_extents correctly Tristan Ye
@ 2011-03-31  7:34 ` Tristan Ye
  2011-03-31 21:13   ` Sunil Mushran
  1 sibling, 1 reply; 15+ messages in thread
From: Tristan Ye @ 2011-03-31  7:34 UTC (permalink / raw)
  To: ocfs2-devel

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

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

* [Ocfs2-devel] [PATCH 2/3] Ocfs2: Teach local-mounted ocfs2 to handle unwritten_extents correctly.
  2011-03-31  7:34 ` [Ocfs2-devel] [PATCH 2/3] Ocfs2: Teach local-mounted ocfs2 to handle unwritten_extents correctly Tristan Ye
@ 2011-03-31 21:06   ` Sunil Mushran
  2011-04-04 17:48     ` Joel Becker
  0 siblings, 1 reply; 15+ messages in thread
From: Sunil Mushran @ 2011-03-31 21:06 UTC (permalink / raw)
  To: ocfs2-devel

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 = {

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

* [Ocfs2-devel] [PATCH 3/3] Ocfs2: Cleanup for OCFS2_IOC_RESVSP & OCFS2_IOC_UNRESVSP.
  2011-03-31  7:34 ` [Ocfs2-devel] [PATCH 3/3] Ocfs2: Cleanup for OCFS2_IOC_RESVSP & OCFS2_IOC_UNRESVSP Tristan Ye
@ 2011-03-31 21:13   ` Sunil Mushran
  2011-04-01  2:46     ` Tristan Ye
  2011-04-04 17:51     ` Joel Becker
  0 siblings, 2 replies; 15+ messages in thread
From: Sunil Mushran @ 2011-03-31 21:13 UTC (permalink / raw)
  To: ocfs2-devel

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 {

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

* [Ocfs2-devel] [PATCH 1/3] VFS/ioctl: Add punching-hole support to ioctl().
  2011-03-31  7:33 [Ocfs2-devel] [PATCH 1/3] VFS/ioctl: Add punching-hole support to ioctl() Tristan Ye
@ 2011-03-31 21:14 ` Sunil Mushran
  2011-03-31 22:56   ` Josef Bacik
  0 siblings, 1 reply; 15+ messages in thread
From: Sunil Mushran @ 2011-03-31 21:14 UTC (permalink / raw)
  To: Tristan Ye
  Cc: linux-kernel, josef, ocfs2-devel, chris.mason, linux-fsdevel,
	linux-btrfs

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

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

* [Ocfs2-devel] [PATCH 1/3] VFS/ioctl: Add punching-hole support to ioctl().
  2011-03-31 21:14 ` Sunil Mushran
@ 2011-03-31 22:56   ` Josef Bacik
  2011-03-31 23:44     ` Joel Becker
  0 siblings, 1 reply; 15+ messages in thread
From: Josef Bacik @ 2011-03-31 22:56 UTC (permalink / raw)
  To: Sunil Mushran
  Cc: Tristan Ye, ocfs2-devel, josef, linux-kernel, linux-fsdevel,
	linux-btrfs, chris.mason, jlbec, tm

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

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

* [Ocfs2-devel] [PATCH 1/3] VFS/ioctl: Add punching-hole support to ioctl().
  2011-03-31 22:56   ` Josef Bacik
@ 2011-03-31 23:44     ` Joel Becker
  2011-04-01  0:34       ` Josef Bacik
  0 siblings, 1 reply; 15+ messages in thread
From: Joel Becker @ 2011-03-31 23:44 UTC (permalink / raw)
  To: Josef Bacik
  Cc: Sunil Mushran, Tristan Ye, ocfs2-devel, linux-kernel,
	linux-fsdevel, linux-btrfs, chris.mason, tm

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 at evilplan.org

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

* [Ocfs2-devel] [PATCH 1/3] VFS/ioctl: Add punching-hole support to ioctl().
  2011-03-31 23:44     ` Joel Becker
@ 2011-04-01  0:34       ` Josef Bacik
  2011-04-01 16:54         ` Joel Becker
  0 siblings, 1 reply; 15+ messages in thread
From: Josef Bacik @ 2011-04-01  0:34 UTC (permalink / raw)
  To: Josef Bacik, Sunil Mushran, Tristan Ye, ocfs2-devel, linux-kernel,
	linux-fsdevel, linux-btrfs, chris.mason, tm

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

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

* [Ocfs2-devel] [PATCH 3/3] Ocfs2: Cleanup for OCFS2_IOC_RESVSP & OCFS2_IOC_UNRESVSP.
  2011-03-31 21:13   ` Sunil Mushran
@ 2011-04-01  2:46     ` Tristan Ye
  2011-04-01  5:29       ` Sunil Mushran
  2011-04-04 17:51     ` Joel Becker
  1 sibling, 1 reply; 15+ messages in thread
From: Tristan Ye @ 2011-04-01  2:46 UTC (permalink / raw)
  To: ocfs2-devel

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 {
> 

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

* [Ocfs2-devel] [PATCH 3/3] Ocfs2: Cleanup for OCFS2_IOC_RESVSP & OCFS2_IOC_UNRESVSP.
  2011-04-01  2:46     ` Tristan Ye
@ 2011-04-01  5:29       ` Sunil Mushran
  0 siblings, 0 replies; 15+ messages in thread
From: Sunil Mushran @ 2011-04-01  5:29 UTC (permalink / raw)
  To: ocfs2-devel

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.

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

* [Ocfs2-devel] [PATCH 1/3] VFS/ioctl: Add punching-hole support to ioctl().
  2011-04-01  0:34       ` Josef Bacik
@ 2011-04-01 16:54         ` Joel Becker
  0 siblings, 0 replies; 15+ messages in thread
From: Joel Becker @ 2011-04-01 16:54 UTC (permalink / raw)
  To: Josef Bacik
  Cc: Sunil Mushran, Tristan Ye, ocfs2-devel, linux-kernel,
	linux-fsdevel, linux-btrfs, chris.mason, tm

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 at evilplan.org

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

* [Ocfs2-devel] [PATCH 2/3] Ocfs2: Teach local-mounted ocfs2 to handle unwritten_extents correctly.
  2011-03-31 21:06   ` Sunil Mushran
@ 2011-04-04 17:48     ` Joel Becker
  0 siblings, 0 replies; 15+ messages in thread
From: Joel Becker @ 2011-04-04 17:48 UTC (permalink / raw)
  To: ocfs2-devel

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

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

* [Ocfs2-devel] [PATCH 3/3] Ocfs2: Cleanup for OCFS2_IOC_RESVSP & OCFS2_IOC_UNRESVSP.
  2011-03-31 21:13   ` Sunil Mushran
  2011-04-01  2:46     ` Tristan Ye
@ 2011-04-04 17:51     ` Joel Becker
  1 sibling, 0 replies; 15+ messages in thread
From: Joel Becker @ 2011-04-04 17:51 UTC (permalink / raw)
  To: ocfs2-devel

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

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

end of thread, other threads:[~2011-04-04 17:51 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-03-31  7:34 [Ocfs2-devel] [PATCH 1/3] VFS/ioctl: Add punching-hole support to ioctl() Tristan Ye
2011-03-31  7:34 ` [Ocfs2-devel] [PATCH 2/3] Ocfs2: Teach local-mounted ocfs2 to handle unwritten_extents correctly Tristan Ye
2011-03-31 21:06   ` Sunil Mushran
2011-04-04 17:48     ` Joel Becker
2011-03-31  7:34 ` [Ocfs2-devel] [PATCH 3/3] Ocfs2: Cleanup for OCFS2_IOC_RESVSP & OCFS2_IOC_UNRESVSP Tristan Ye
2011-03-31 21:13   ` Sunil Mushran
2011-04-01  2:46     ` Tristan Ye
2011-04-01  5:29       ` Sunil Mushran
2011-04-04 17:51     ` Joel Becker
  -- strict thread matches above, loose matches on Subject: below --
2011-03-31  7:33 [Ocfs2-devel] [PATCH 1/3] VFS/ioctl: Add punching-hole support to ioctl() Tristan Ye
2011-03-31 21:14 ` Sunil Mushran
2011-03-31 22:56   ` Josef Bacik
2011-03-31 23:44     ` Joel Becker
2011-04-01  0:34       ` Josef Bacik
2011-04-01 16:54         ` Joel Becker

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).