diff mbox

[2/3] vfs: dedupe: rationalize args

Message ID 20180507082108.28186-3-mszeredi@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Miklos Szeredi May 7, 2018, 8:21 a.m. UTC
Clean up f_op->dedupe_file_range() interface.

1) Use loff_t for offsets instead of u64
2) Order the arguments the same way as {copy|clone}_file_range().

Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
---
 fs/btrfs/ctree.h   | 4 ++--
 fs/btrfs/ioctl.c   | 4 ++--
 fs/ocfs2/file.c    | 6 +++---
 fs/read_write.c    | 4 ++--
 fs/xfs/xfs_file.c  | 6 +++---
 include/linux/fs.h | 2 +-
 6 files changed, 13 insertions(+), 13 deletions(-)

Comments

Matthew Wilcox (Oracle) May 7, 2018, 11:16 a.m. UTC | #1
On Mon, May 07, 2018 at 10:21:07AM +0200, Miklos Szeredi wrote:
> @@ -1738,7 +1738,7 @@ struct file_operations {
>  			loff_t, size_t, unsigned int);
>  	int (*clone_file_range)(struct file *, loff_t, struct file *, loff_t,
>  			u64);
> -	s64 (*dedupe_file_range)(struct file *, u64, u64, struct file *,
> +	s64 (*dedupe_file_range)(struct file *, loff_t, struct file *, loff_t,
>  			u64);

Please name the parameters here ...

+	loff_t (*dedupe_file_range)(struct file *src, loff_t src_off,
+			struct file *dst, loff_t dst_off, loff_t len);
Miklos Szeredi May 7, 2018, 11:36 a.m. UTC | #2
On Mon, May 7, 2018 at 1:16 PM, Matthew Wilcox <willy@infradead.org> wrote:
> On Mon, May 07, 2018 at 10:21:07AM +0200, Miklos Szeredi wrote:
>> @@ -1738,7 +1738,7 @@ struct file_operations {
>>                       loff_t, size_t, unsigned int);
>>       int (*clone_file_range)(struct file *, loff_t, struct file *, loff_t,
>>                       u64);
>> -     s64 (*dedupe_file_range)(struct file *, u64, u64, struct file *,
>> +     s64 (*dedupe_file_range)(struct file *, loff_t, struct file *, loff_t,
>>                       u64);
>
> Please name the parameters here ...
>
> +       loff_t (*dedupe_file_range)(struct file *src, loff_t src_off,
> +                       struct file *dst, loff_t dst_off, loff_t len);

It's the convention here.  Going against the convention looks odd and
has dubious value.

Fixing the convention is okay by me, but I'd leave that to some
kernelnewbie to worry about.

Thanks,
Miklos
diff mbox

Patch

diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 019a25962ac8..a8f1ab69158b 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -3269,8 +3269,8 @@  void btrfs_get_block_group_info(struct list_head *groups_list,
 				struct btrfs_ioctl_space_info *space);
 void update_ioctl_balance_args(struct btrfs_fs_info *fs_info, int lock,
 			       struct btrfs_ioctl_balance_args *bargs);
-s64 btrfs_dedupe_file_range(struct file *src_file, u64 loff, u64 olen,
-			    struct file *dst_file, u64 dst_loff);
+s64 btrfs_dedupe_file_range(struct file *src_file, loff_t loff,
+			    struct file *dst_file, loff_t dst_loff, u64 olen);
 
 /* file.c */
 int __init btrfs_auto_defrag_init(void);
diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index b6a1df6bb895..10658c0a5ac9 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -3194,8 +3194,8 @@  static int btrfs_extent_same(struct inode *src, u64 loff, u64 olen,
 
 #define BTRFS_MAX_DEDUPE_LEN	SZ_16M
 
-s64 btrfs_dedupe_file_range(struct file *src_file, u64 loff, u64 olen,
-			    struct file *dst_file, u64 dst_loff)
+s64 btrfs_dedupe_file_range(struct file *src_file, loff_t loff,
+			    struct file *dst_file, loff_t dst_loff, u64 olen)
 {
 	struct inode *src = file_inode(src_file);
 	struct inode *dst = file_inode(dst_file);
diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c
index cf3732df4c2e..00656f4b6059 100644
--- a/fs/ocfs2/file.c
+++ b/fs/ocfs2/file.c
@@ -2538,10 +2538,10 @@  static int ocfs2_file_clone_range(struct file *file_in,
 }
 
 static s64 ocfs2_file_dedupe_range(struct file *src_file,
-				   u64 loff,
-				   u64 len,
+				   loff_t loff,
 				   struct file *dst_file,
-				   u64 dst_loff)
+				   loff_t dst_loff,
+				   u64 len)
 {
 	int error;
 
diff --git a/fs/read_write.c b/fs/read_write.c
index 6a79c7ec2bb2..0cdef381d9d9 100644
--- a/fs/read_write.c
+++ b/fs/read_write.c
@@ -2046,8 +2046,8 @@  int vfs_dedupe_file_range(struct file *file, struct file_dedupe_range *same)
 			info->status = -EINVAL;
 		} else {
 			deduped = dst_file->f_op->dedupe_file_range(file, off,
-							len, dst_file,
-							info->dest_offset);
+							dst_file,
+							info->dest_offset, len);
 			if (deduped == -EBADE)
 				info->status = FILE_DEDUPE_RANGE_DIFFERS;
 			else if (deduped < 0)
diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
index d06dd1557d0e..e2620a00d132 100644
--- a/fs/xfs/xfs_file.c
+++ b/fs/xfs/xfs_file.c
@@ -871,10 +871,10 @@  xfs_file_clone_range(
 STATIC s64
 xfs_file_dedupe_range(
 	struct file	*src_file,
-	u64		loff,
-	u64		len,
+	loff_t		loff,
 	struct file	*dst_file,
-	u64		dst_loff)
+	loff_t		dst_loff,
+	u64		len)
 {
 	int		error;
 
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 6feb121ae48c..8007a31c4d3c 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1738,7 +1738,7 @@  struct file_operations {
 			loff_t, size_t, unsigned int);
 	int (*clone_file_range)(struct file *, loff_t, struct file *, loff_t,
 			u64);
-	s64 (*dedupe_file_range)(struct file *, u64, u64, struct file *,
+	s64 (*dedupe_file_range)(struct file *, loff_t, struct file *, loff_t,
 			u64);
 } __randomize_layout;