Re: [PATCH v2 1/2] ocfs2: replace ocfs2-specific falloc ioctls with generic ones

[Joseph Qi <joseph.qi@linux.alibaba.com>]

On 10/22/26 9:04 PM, Anthony Iliopoulos wrote:
> Replace custom ocfs2 space reservation structures and ioctl definitions
> with the generic file space allocation interfaces.
> 
> Those were originally copied from xfs in commit b25801038da5 ("ocfs2:
> Support xfs style space reservation ioctls") for compatibility reasons,
> which were eventually lifted to vfs and made generic. As such there is
> no reason to carry around the redundant definitions any longer, they can
> be dropped at this point.
> 
> Signed-off-by: Anthony Iliopoulos <ailiop@suse.com>

Looks fine.
Reviewed-by: Joseph Qi <joseph.qi@linux.alibaba.com>

> ---
>  fs/ocfs2/file.c        | 26 +++++++++++++-------------
>  fs/ocfs2/file.h        |  4 +++-
>  fs/ocfs2/ioctl.c       | 19 ++++++++++---------
>  fs/ocfs2/ocfs2_ioctl.h | 26 --------------------------
>  4 files changed, 26 insertions(+), 49 deletions(-)
> 
> diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c
> index 62f45a1b5ca1..5ccc2a613e2d 100644
> --- a/fs/ocfs2/file.c
> +++ b/fs/ocfs2/file.c
> @@ -1944,7 +1944,7 @@ int ocfs2_remove_inode_range(struct inode *inode,
>   */
>  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;
> @@ -2008,8 +2008,8 @@ 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 ||
> -	    cmd == OCFS2_IOC_UNRESVSP || cmd == OCFS2_IOC_UNRESVSP64) {
> +	if (cmd == FS_IOC_RESVSP || cmd == FS_IOC_RESVSP64 ||
> +	    cmd == FS_IOC_UNRESVSP || cmd == FS_IOC_UNRESVSP64) {
>  		if (sr->l_len <= 0) {
>  			ret = -EINVAL;
>  			goto out_inode_unlock;
> @@ -2026,8 +2026,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.
> @@ -2035,8 +2035,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;
> @@ -2090,16 +2090,16 @@ static int __ocfs2_change_file_space(struct file *file, struct inode *inode,
>  }
>  
>  int ocfs2_change_file_space(struct file *file, unsigned int cmd,
> -			    struct ocfs2_space_resv *sr)
> +			    struct space_resv *sr)
>  {
>  	struct inode *inode = file_inode(file);
>  	struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
>  	int ret;
>  
> -	if ((cmd == OCFS2_IOC_RESVSP || cmd == OCFS2_IOC_RESVSP64) &&
> +	if ((cmd == FS_IOC_RESVSP || cmd == FS_IOC_RESVSP64) &&
>  	    !ocfs2_writes_unwritten_extents(osb))
>  		return -ENOTTY;
> -	else if ((cmd == OCFS2_IOC_UNRESVSP || cmd == OCFS2_IOC_UNRESVSP64) &&
> +	else if ((cmd == FS_IOC_UNRESVSP || cmd == FS_IOC_UNRESVSP64) &&
>  		 !ocfs2_sparse_alloc(osb))
>  		return -ENOTTY;
>  
> @@ -2122,9 +2122,9 @@ static long ocfs2_fallocate(struct file *file, int mode, loff_t offset,
>  {
>  	struct inode *inode = file_inode(file);
>  	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;
>  	int ret = 0;
>  
>  	if (mode & ~(FALLOC_FL_KEEP_SIZE | FALLOC_FL_PUNCH_HOLE))
> @@ -2141,7 +2141,7 @@ static long ocfs2_fallocate(struct file *file, int mode, loff_t offset,
>  	}
>  
>  	if (mode & FALLOC_FL_PUNCH_HOLE)
> -		cmd = OCFS2_IOC_UNRESVSP64;
> +		cmd = FS_IOC_UNRESVSP64;
>  
>  	sr.l_whence = 0;
>  	sr.l_start = (s64)offset;
> diff --git a/fs/ocfs2/file.h b/fs/ocfs2/file.h
> index 97492ee5789e..7f587dc06dab 100644
> --- a/fs/ocfs2/file.h
> +++ b/fs/ocfs2/file.h
> @@ -10,6 +10,8 @@
>  #ifndef OCFS2_FILE_H
>  #define OCFS2_FILE_H
>  
> +#include <linux/falloc.h>
> +
>  extern const struct file_operations ocfs2_fops;
>  extern const struct file_operations ocfs2_dops;
>  extern const struct file_operations ocfs2_fops_no_plocks;
> @@ -64,7 +66,7 @@ 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);
> +			    struct space_resv *sr);
>  
>  int ocfs2_check_range_for_refcount(struct inode *inode, loff_t pos,
>  				   size_t count);
> diff --git a/fs/ocfs2/ioctl.c b/fs/ocfs2/ioctl.c
> index 36c7c9ac8b5d..2d95c1e5ad8b 100644
> --- a/fs/ocfs2/ioctl.c
> +++ b/fs/ocfs2/ioctl.c
> @@ -11,6 +11,7 @@
>  #include <linux/blkdev.h>
>  #include <linux/compat.h>
>  #include <linux/fileattr.h>
> +#include <linux/falloc.h>
>  
>  #include <cluster/masklog.h>
>  
> @@ -857,12 +858,12 @@ long ocfs2_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
>  	int status;
>  
>  	switch (cmd) {
> -	case OCFS2_IOC_RESVSP:
> -	case OCFS2_IOC_RESVSP64:
> -	case OCFS2_IOC_UNRESVSP:
> -	case OCFS2_IOC_UNRESVSP64:
> +	case FS_IOC_RESVSP:
> +	case FS_IOC_RESVSP64:
> +	case FS_IOC_UNRESVSP:
> +	case FS_IOC_UNRESVSP64:
>  	{
> -		struct ocfs2_space_resv sr;
> +		struct space_resv sr;
>  
>  		if (copy_from_user(&sr, (int __user *) arg, sizeof(sr)))
>  			return -EFAULT;
> @@ -971,10 +972,10 @@ long ocfs2_compat_ioctl(struct file *file, unsigned cmd, unsigned long arg)
>  	void __user *argp = (void __user *)arg;
>  
>  	switch (cmd) {
> -	case OCFS2_IOC_RESVSP:
> -	case OCFS2_IOC_RESVSP64:
> -	case OCFS2_IOC_UNRESVSP:
> -	case OCFS2_IOC_UNRESVSP64:
> +	case FS_IOC_RESVSP:
> +	case FS_IOC_RESVSP64:
> +	case FS_IOC_UNRESVSP:
> +	case FS_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 2de2f8733283..30244c94239b 100644
> --- a/fs/ocfs2/ocfs2_ioctl.h
> +++ b/fs/ocfs2/ocfs2_ioctl.h
> @@ -10,32 +10,6 @@
>  #ifndef OCFS2_IOCTL_H
>  #define OCFS2_IOCTL_H
>  
> -/*
> - * 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)
> -
>  /* Used to pass group descriptor data when online resize is done */
>  struct ocfs2_new_group_input {
>  	__u64 group;		/* Group descriptor's blkno. */