From: "Darrick J. Wong" <djwong@kernel.org>
To: liuhuan01@kylinos.cn
Cc: linux-xfs@vger.kernel.org, dchinner@redhat.com
Subject: Re: [PATCH] xfs_io: use FICLONE/FICLONERANGE/FIDEDUPERANGE for reflink/dudupe IO commands
Date: Fri, 9 Aug 2024 08:04:55 -0700 [thread overview]
Message-ID: <20240809150455.GV6051@frogsfrogsfrogs> (raw)
In-Reply-To: <20240809090226.196381-1-liuhuan01@kylinos.cn>
On Fri, Aug 09, 2024 at 05:02:26PM +0800, liuhuan01@kylinos.cn wrote:
> From: liuh <liuhuan01@kylinos.cn>
>
> Use FICLONE/FICLONERANGE/FIDEDUPERANGE instead of XFS_IOC_CLONE/XFS_IOC_CLONE_RANGE/XFS_IOC_FILE_EXTENT_SAME.
> And remove dead code.
Where was the dead code?
> Signed-off-by: liuh <liuhuan01@kylinos.cn>
> ---
> include/xfs_fs_compat.h | 54 -----------------------------------------
> io/reflink.c | 52 +++++++++++++++++++--------------------
> 2 files changed, 26 insertions(+), 80 deletions(-)
>
> diff --git a/include/xfs_fs_compat.h b/include/xfs_fs_compat.h
> index 0077f00c..e53dcc6e 100644
> --- a/include/xfs_fs_compat.h
> +++ b/include/xfs_fs_compat.h
> @@ -31,60 +31,6 @@
> #define XFS_XFLAG_FILESTREAM FS_XFLAG_FILESTREAM
> #define XFS_XFLAG_HASATTR FS_XFLAG_HASATTR
>
> -/*
> - * Don't use this.
> - * Use struct file_clone_range
> - */
> -struct xfs_clone_args {
> - __s64 src_fd;
> - __u64 src_offset;
> - __u64 src_length;
> - __u64 dest_offset;
> -};
> -
> -/*
> - * Don't use these.
> - * Use FILE_DEDUPE_RANGE_SAME / FILE_DEDUPE_RANGE_DIFFERS
> - */
> -#define XFS_EXTENT_DATA_SAME 0
> -#define XFS_EXTENT_DATA_DIFFERS 1
> -
> -/* Don't use this. Use file_dedupe_range_info */
> -struct xfs_extent_data_info {
> - __s64 fd; /* in - destination file */
> - __u64 logical_offset; /* in - start of extent in destination */
> - __u64 bytes_deduped; /* out - total # of bytes we were able
> - * to dedupe from this file */
> - /* status of this dedupe operation:
> - * < 0 for error
> - * == XFS_EXTENT_DATA_SAME if dedupe succeeds
> - * == XFS_EXTENT_DATA_DIFFERS if data differs
> - */
> - __s32 status; /* out - see above description */
> - __u32 reserved;
> -};
> -
> -/*
> - * Don't use this.
> - * Use struct file_dedupe_range
> - */
> -struct xfs_extent_data {
> - __u64 logical_offset; /* in - start of extent in source */
> - __u64 length; /* in - length of extent */
> - __u16 dest_count; /* in - total elements in info array */
> - __u16 reserved1;
> - __u32 reserved2;
> - struct xfs_extent_data_info info[0];
> -};
> -
> -/*
> - * Don't use these.
> - * Use FICLONE/FICLONERANGE/FIDEDUPERANGE
> - */
> -#define XFS_IOC_CLONE _IOW (0x94, 9, int)
> -#define XFS_IOC_CLONE_RANGE _IOW (0x94, 13, struct xfs_clone_args)
> -#define XFS_IOC_FILE_EXTENT_SAME _IOWR(0x94, 54, struct xfs_extent_data)
> -
> /* 64-bit seconds counter that works independently of the C library time_t. */
> typedef long long int time64_t;
>
> diff --git a/io/reflink.c b/io/reflink.c
> index b6a3c05a..154ca65b 100644
> --- a/io/reflink.c
> +++ b/io/reflink.c
> @@ -43,49 +43,49 @@ dedupe_ioctl(
> uint64_t len,
> int *ops)
> {
> - struct xfs_extent_data *args;
> - struct xfs_extent_data_info *info;
> + struct file_dedupe_range *args;
> + struct file_dedupe_range_info *info;
> int error;
> uint64_t deduped = 0;
>
> - args = calloc(1, sizeof(struct xfs_extent_data) +
> - sizeof(struct xfs_extent_data_info));
> + args = calloc(1, sizeof(struct file_dedupe_range) +
> + sizeof(struct file_dedupe_range_info));
> if (!args)
> goto done;
> - info = (struct xfs_extent_data_info *)(args + 1);
> - args->logical_offset = soffset;
> - args->length = len;
> + info = (struct file_dedupe_range_info *)(args + 1);
> + args->src_offset = soffset;
> + args->src_length = len;
> args->dest_count = 1;
> - info->fd = file->fd;
> - info->logical_offset = doffset;
> + info->dest_fd = file->fd;
> + info->dest_offset = doffset;
>
> - while (args->length > 0 || !*ops) {
> - error = ioctl(fd, XFS_IOC_FILE_EXTENT_SAME, args);
> + while (args->src_length > 0 || !*ops) {
> + error = ioctl(fd, FIDEDUPERANGE, args);
> if (error) {
> - perror("XFS_IOC_FILE_EXTENT_SAME");
> + perror("FIDEDUPERANGE");
If you update these error message prefixes, you'll likely need to update
fstests too:
tests/generic/122.out:7:XFS_IOC_FILE_EXTENT_SAME: Extents did not match.
tests/generic/136.out:10:XFS_IOC_FILE_EXTENT_SAME: Extents did not match.
tests/generic/157.out:5:XFS_IOC_CLONE_RANGE: Invalid cross-device link
tests/generic/157.out:7:XFS_IOC_CLONE_RANGE: Invalid argument
tests/generic/157.out:9:XFS_IOC_CLONE_RANGE: Invalid argument
tests/generic/157.out:11:XFS_IOC_CLONE_RANGE: Invalid argument
tests/generic/157.out:13:XFS_IOC_CLONE_RANGE: Is a directory
tests/generic/157.out:15:XFS_IOC_CLONE_RANGE: Invalid argument
tests/generic/157.out:19:XFS_IOC_CLONE_RANGE: Invalid argument
tests/generic/157.out:21:XFS_IOC_CLONE_RANGE: Invalid argument
tests/generic/157.out:23:XFS_IOC_CLONE_RANGE: Bad file descriptor
tests/generic/158.out:5:XFS_IOC_FILE_EXTENT_SAME: Invalid cross-device link
tests/generic/158.out:7:XFS_IOC_FILE_EXTENT_SAME: Invalid argument
tests/generic/158.out:9:XFS_IOC_FILE_EXTENT_SAME: Invalid argument
tests/generic/158.out:11:XFS_IOC_FILE_EXTENT_SAME: Invalid argument
tests/generic/158.out:13:XFS_IOC_FILE_EXTENT_SAME: Invalid argument
tests/generic/158.out:15:XFS_IOC_FILE_EXTENT_SAME: Invalid argument
tests/generic/158.out:17:XFS_IOC_FILE_EXTENT_SAME: Is a directory
tests/generic/158.out:19:XFS_IOC_FILE_EXTENT_SAME: Invalid argument
tests/generic/158.out:23:XFS_IOC_FILE_EXTENT_SAME: Invalid argument
tests/generic/158.out:25:XFS_IOC_FILE_EXTENT_SAME: Invalid argument
tests/generic/303.out:7:XFS_IOC_CLONE_RANGE: Invalid argument
tests/generic/303.out:10:XFS_IOC_CLONE_RANGE: Invalid argument
tests/generic/303.out:12:XFS_IOC_CLONE_RANGE: Invalid argument
tests/generic/303.out:14:XFS_IOC_CLONE_RANGE: Invalid argument
tests/generic/304.out:5:XFS_IOC_FILE_EXTENT_SAME: Invalid argument
tests/generic/304.out:7:XFS_IOC_FILE_EXTENT_SAME: Invalid argument
tests/generic/304.out:9:XFS_IOC_FILE_EXTENT_SAME: Invalid argument
tests/generic/304.out:11:XFS_IOC_FILE_EXTENT_SAME: Invalid argument
tests/generic/304.out:13:XFS_IOC_FILE_EXTENT_SAME: Invalid argument
tests/generic/304.out:15:XFS_IOC_FILE_EXTENT_SAME: Invalid argument
tests/generic/304.out:17:XFS_IOC_FILE_EXTENT_SAME: Invalid argument
tests/generic/493.out:5:XFS_IOC_FILE_EXTENT_SAME: Text file busy
tests/generic/493.out:6:XFS_IOC_FILE_EXTENT_SAME: Text file busy
tests/generic/516.out:7:XFS_IOC_FILE_EXTENT_SAME: Extents did not match.
tests/generic/518.out:6:XFS_IOC_CLONE_RANGE: Invalid argument
tests/xfs/319.out:10:XFS_IOC_CLONE_RANGE: Input/output error
tests/xfs/321.out:9:XFS_IOC_CLONE_RANGE: Input/output error
tests/xfs/322.out:9:XFS_IOC_CLONE_RANGE: Input/output error
tests/xfs/323.out:9:XFS_IOC_CLONE_RANGE: Input/output error
(or leave the prefixes alone)
--D
> exitcode = 1;
> goto done;
> }
> if (info->status < 0) {
> - fprintf(stderr, "XFS_IOC_FILE_EXTENT_SAME: %s\n",
> + fprintf(stderr, "FIDEDUPERANGE: %s\n",
> _(strerror(-info->status)));
> goto done;
> }
> - if (info->status == XFS_EXTENT_DATA_DIFFERS) {
> - fprintf(stderr, "XFS_IOC_FILE_EXTENT_SAME: %s\n",
> + if (info->status == FILE_DEDUPE_RANGE_DIFFERS) {
> + fprintf(stderr, "FIDEDUPERANGE: %s\n",
> _("Extents did not match."));
> goto done;
> }
> - if (args->length != 0 &&
> + if (args->src_length != 0 &&
> (info->bytes_deduped == 0 ||
> - info->bytes_deduped > args->length))
> + info->bytes_deduped > args->src_length))
> break;
>
> (*ops)++;
> - args->logical_offset += info->bytes_deduped;
> - info->logical_offset += info->bytes_deduped;
> - if (args->length >= info->bytes_deduped)
> - args->length -= info->bytes_deduped;
> + args->src_offset += info->bytes_deduped;
> + info->dest_offset += info->bytes_deduped;
> + if (args->src_length >= info->bytes_deduped)
> + args->src_length -= info->bytes_deduped;
> deduped += info->bytes_deduped;
> }
> done:
> @@ -200,21 +200,21 @@ reflink_ioctl(
> uint64_t len,
> int *ops)
> {
> - struct xfs_clone_args args;
> + struct file_clone_range args;
> int error;
>
> if (soffset == 0 && doffset == 0 && len == 0) {
> - error = ioctl(file->fd, XFS_IOC_CLONE, fd);
> + error = ioctl(file->fd, FICLONE, fd);
> if (error)
> - perror("XFS_IOC_CLONE");
> + perror("FICLONE");
> } else {
> args.src_fd = fd;
> args.src_offset = soffset;
> args.src_length = len;
> args.dest_offset = doffset;
> - error = ioctl(file->fd, XFS_IOC_CLONE_RANGE, &args);
> + error = ioctl(file->fd, FICLONERANGE, &args);
> if (error)
> - perror("XFS_IOC_CLONE_RANGE");
> + perror("FICLONERANGE");
> }
> if (!error)
> (*ops)++;
> --
> 2.43.0
>
>
next prev parent reply other threads:[~2024-08-09 15:04 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-08-09 9:02 [PATCH] xfs_io: use FICLONE/FICLONERANGE/FIDEDUPERANGE for reflink/dudupe IO commands liuhuan01
2024-08-09 15:04 ` Darrick J. Wong [this message]
2024-08-17 6:48 ` [PATCH] xfs_io: use FICLONE/FICLONERANGE/FIDEDUPERANGE for reflink/dedupe " liuhuan01
2024-08-17 6:50 ` [PATCH] xfs: use FICLONE/FICLONERANGE/FIDEDUPERANGE for test cases liuhuan01
2024-08-21 17:14 ` Darrick J. Wong
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240809150455.GV6051@frogsfrogsfrogs \
--to=djwong@kernel.org \
--cc=dchinner@redhat.com \
--cc=linux-xfs@vger.kernel.org \
--cc=liuhuan01@kylinos.cn \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox