Skip to contentSkip to conversation

VFS/ioctl: Add punching-hole support to ioctl().

PATCH New Tristan Ye 3 participants 9 messages Mar 31, 2011 – Apr 4, 2011

Subsystems (3)
VFS OCFS2 Landlock security module
lore
Share & tools
lore

Tristan Ye

author
#0 lore raw

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

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 <[email protected]>
---
 fs/ioctl.c             |   10 +++++++---
 include/linux/falloc.h |    2 ++
 2 files changed, 9 insertions(+), 3 deletions(-)
fs/ioctl.c +7 -3
index 1d9b9fc..234e26f 100644
@@ -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);
include/linux/falloc.h +2 -0
index 73e0b62..fd1e871 100644
@@ -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__ */
Signature · 1 line
1.5.5
author
#1 lore raw

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

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 <[email protected]>
---
 fs/ocfs2/file.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
fs/ocfs2/file.c +1 -0
index 41565ae..cce8c2b 100644
@@ -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 = {
Signature · 1 line
1.5.5
#2 lore raw

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

Acked-by Sunil Mushran <[email protected]>

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.

Quoted 21 lines · 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<[email protected]>
---
  fs/ocfs2/file.c |    1 +
  1 files changed, 1 insertions(+), 0 deletions(-)
fs/ocfs2/file.c +1 -0
index 41565ae..cce8c2b 100644
@@ -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 = {
maintainer
#3 lore raw

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

Quoted 3 lines · Sunil Mushran
Acked-by Sunil Mushran <[email protected]>

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

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

			http://www.jlbec.org/
			jlbec at evilplan.org
author
#4 lore raw

PATCH 3/3 Ocfs2: Cleanup for OCFS2_IOC_RESVSP & OCFS2_IOC_UNRESVSP.

In reply to 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 <[email protected]>
---
 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(-)
fs/ocfs2/file.c +27 -36
index cce8c2b..d016322 100644
@@ -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;
fs/ocfs2/file.h +0 -3
index f5afbbe..2c9b7a8 100644
@@ -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 */
fs/ocfs2/ioctl.c +0 -13
index 8f13c59..d703210 100644
@@ -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:
fs/ocfs2/ocfs2_ioctl.h +4 -25
index b46f39b..4f9bf28 100644
@@ -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 {
Signature · 1 line
1.5.5
#5 lore raw

PATCH 3/3 Ocfs2: Cleanup for OCFS2_IOC_RESVSP & OCFS2_IOC_UNRESVSP.

Quoted 108 lines · 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<[email protected]>
---
  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(-)
fs/ocfs2/file.c +9 -33
index cce8c2b..d016322 100644
@@ -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)

fs/ocfs2/file.c out: · line 1961

Quoted 10 lines
fs/ocfs2/file.c +9 -1
-		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.

Quoted 107 lines
fs/ocfs2/file.c +9 -2
-	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;
fs/ocfs2/file.h +0 -3
index f5afbbe..2c9b7a8 100644
@@ -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 */
fs/ocfs2/ioctl.c +0 -13
index 8f13c59..d703210 100644
@@ -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:
fs/ocfs2/ocfs2_ioctl.h +4 -25
index b46f39b..4f9bf28 100644
@@ -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 {
author
#6 lore raw

PATCH 3/3 Ocfs2: Cleanup for OCFS2_IOC_RESVSP & OCFS2_IOC_UNRESVSP.

Quoted 118 lines · Sunil Mushran
Quoted 117 lines · 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<[email protected]>
---
  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(-)
fs/ocfs2/file.c +2 -1
index cce8c2b..d016322 100644
@@ -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;
fs/ocfs2/file.c +1 -1
@@ -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;
fs/ocfs2/file.c +2 -2
@@ -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.

fs/ocfs2/file.c +2 -2
@@ -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;
fs/ocfs2/file.c +0 -10
@@ -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)&&

fs/ocfs2/file.c +0 -10
-         !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);

fs/ocfs2/file.c +2 -6
-}
-
  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;
Quoted 14 lines

fs/ocfs2/file.c out: · line 1961

Quoted 12 lines
fs/ocfs2/file.c +4 -2
-    if (mode&  FALLOC_FL_KEEP_SIZE)
-        change_size = 0;
+    /*
+     * unwritten extents
+     */
+    if ((mode&  FALLOC_FL_KEEP_SIZE)&&  !(mode&

FALLOC_FL_PUNCH_HOLE)) {

fs/ocfs2/file.c +5 -0
+        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.

Quoted 113 lines
Quoted 112 lines
fs/ocfs2/file.c +9 -2
-    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;
fs/ocfs2/file.h +0 -3
index f5afbbe..2c9b7a8 100644
@@ -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 */
fs/ocfs2/ioctl.c +0 -1
index 8f13c59..d703210 100644
@@ -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;
fs/ocfs2/ioctl.c +0 -8
@@ -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;
fs/ocfs2/ioctl.c +0 -4
@@ -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:
fs/ocfs2/ocfs2_ioctl.h +4 -25
index b46f39b..4f9bf28 100644
@@ -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 {
#7 lore raw

PATCH 3/3 Ocfs2: Cleanup for OCFS2_IOC_RESVSP & OCFS2_IOC_UNRESVSP.

Quoted 9 lines · Tristan Ye

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.

maintainer
#8 lore raw

PATCH 3/3 Ocfs2: Cleanup for OCFS2_IOC_RESVSP & OCFS2_IOC_UNRESVSP.

In reply to Sunil Mushran
Quoted 5 lines · Sunil Mushran
Quoted 4 lines · 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.

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

Joel

Signature · 9 lines
"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