[PROGS PATCH] Import btrfs-extent-same

[PROGS PATCH] Import btrfs-extent-same

[Gabriel de Perthuis]

Originally from
https://github.com/markfasheh/duperemove/blob/master/btrfs-extent-same.c

Signed-off-by: Gabriel de Perthuis <g2p.code+btrfs@gmail.com>
---
 .gitignore          |   1 +
 Makefile            |   2 +-
 btrfs-extent-same.c | 145 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 147 insertions(+), 1 deletion(-)
 create mode 100644 btrfs-extent-same.c

diff --git a/.gitignore b/.gitignore
index a7e1b19..8f0ec54 100644
--- a/.gitignore
+++ b/.gitignore
@@ -5,10 +5,11 @@ version.h
 version
 man/*.gz
 btrfs
 btrfs.static
 btrfs-debug-tree
+btrfs-extent-same
 btrfs-map-logical
 btrfs-fragments
 btrfsck
 calc-size
 ioctl-test
diff --git a/Makefile b/Makefile
index da7438e..35cc502 100644
--- a/Makefile
+++ b/Makefile
@@ -43,11 +43,11 @@ endif
 
 MAKEOPTS = --no-print-directory Q=$(Q)
 
 progs = mkfs.btrfs btrfs-debug-tree btrfsck \
 	btrfs btrfs-map-logical btrfs-image btrfs-zero-log btrfs-convert \
-	btrfs-find-root btrfstune btrfs-show-super
+	btrfs-find-root btrfstune btrfs-show-super btrfs-extent-same
 
 # external libs required by various binaries; for btrfs-foo,
 # specify btrfs_foo_libs = <list of libs>; see $($(subst...)) rules below
 btrfs_convert_libs = -lext2fs -lcom_err
 btrfs_image_libs = -lpthread
diff --git a/btrfs-extent-same.c b/btrfs-extent-same.c
new file mode 100644
index 0000000..acf46b7
--- /dev/null
+++ b/btrfs-extent-same.c
@@ -0,0 +1,145 @@
+/*
+ * btrfs-extent-same.c
+ *
+ * Copyright (C) 2013 SUSE.  All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public
+ * License version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ */
+
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+#include <unistd.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <stdint.h>
+#include <stddef.h>
+#include <errno.h>
+#include <string.h>
+#include <sys/ioctl.h>
+
+#define BTRFS_IOCTL_MAGIC 0x94
+
+#define BTRFS_IOC_FILE_EXTENT_SAME _IOWR(BTRFS_IOCTL_MAGIC, 54, \
+					 struct btrfs_ioctl_same_args)
+
+#define BTRFS_SAME_DATA_DIFFERS	1
+/* For extent-same ioctl */
+struct btrfs_ioctl_same_extent_info {
+	int64_t fd;			/* in - destination file */
+	uint64_t logical_offset;	/* in - start of extent in destination */
+	uint64_t bytes_deduped;		/* out - total # of bytes we
+					 * were able to dedupe from
+					 * this file */
+	/* status of this dedupe operation:
+	 * 0 if dedup succeeds
+	 * < 0 for error
+	 * == BTRFS_SAME_DATA_DIFFERS if data differs
+	 */
+	int32_t status;			/* out - see above description */
+	uint32_t reserved;
+};
+
+struct btrfs_ioctl_same_args {
+	uint64_t logical_offset;	/* in - start of extent in source */
+	uint64_t length;		/* in - length of extent */
+	uint16_t dest_count;		/* in - total elements in info array */
+	uint16_t reserved1;		/* out - number of files that got deduped */
+	uint32_t reserved2;
+	struct btrfs_ioctl_same_extent_info info[0];
+};
+
+static void usage(const char *prog)
+{
+	printf("Usage: %s len file1 loff1 file2 loff2\n", prog);
+}
+
+int main(int argc, char **argv)
+{
+	int ret, src_fd, i, numfiles;
+	char *srcf, *destf;
+	struct btrfs_ioctl_same_args *same;
+	struct btrfs_ioctl_same_extent_info *info;
+	unsigned long long bytes = 0ULL;
+
+	if (argc < 6 || (argc % 2)) {
+		usage(argv[0]);
+		return 1;
+	}
+
+	numfiles = (argc / 2) - 2;
+
+	printf("Deduping %d total files\n", numfiles + 1);
+
+	same = calloc(1,
+		      sizeof(struct btrfs_ioctl_same_args) +
+		      sizeof(struct btrfs_ioctl_same_extent_info) * numfiles);
+	if (!same)
+		return -ENOMEM;
+
+	srcf = argv[2];
+	same->length = atoll(argv[1]);
+	same->logical_offset = atoll(argv[3]);
+	same->dest_count = numfiles;
+
+	ret = open(srcf, O_RDONLY);
+	if (ret < 0) {
+		ret = errno;
+		fprintf(stderr, "Could not open file %s: (%d) %s\n", srcf, ret,
+			strerror(ret));
+		return -ret;
+	}
+	src_fd = ret;
+
+	printf("(%llu, %llu): %s\n", (unsigned long long)same->logical_offset,
+	       (unsigned long long)same->length, srcf);
+
+	for (i = 0; i < same->dest_count; i++) {
+		destf = argv[4 + (i * 2)];
+
+		ret = open(destf, O_RDONLY);
+		if (ret < 0) {
+			ret = errno;
+			fprintf(stderr, "Could not open file %s: (%d) %s\n",
+				destf, ret, strerror(ret));
+			return -ret;
+		}
+
+		same->info[i].fd = ret;
+		same->info[i].logical_offset = atoll(argv[5 + (i * 2)]);
+		printf("(%llu, %llu): %s\n",
+		       (unsigned long long)same->info[i].logical_offset,
+		       (unsigned long long)same->length, destf);
+
+	}
+
+	ret = ioctl(src_fd, BTRFS_IOC_FILE_EXTENT_SAME, same);
+	if (ret < 0) {
+		ret = errno;
+		fprintf(stderr, "btrfs_same returned error: (%d) %s\n", ret,
+			strerror(ret));
+		return -ret;
+	}
+
+	printf("%u files asked to be deduped\n", same->dest_count);
+
+	for (i = 0; i < same->dest_count; i++) {
+		info = &same->info[i];
+
+		printf("i: %d, status: %d, bytes_deduped: %llu\n", i,
+		       info->status, (unsigned long long)info->bytes_deduped);
+
+		bytes += info->bytes_deduped;
+	}
+
+	printf("%llu total bytes deduped in this operation\n", bytes);
+
+	return ret;
+}
-- 
1.8.3.1.588.gb04834f

Re: [PROGS PATCH] Import btrfs-extent-same

[David Sterba]

On Thu, Jun 27, 2013 at 12:38:19AM +0200, Gabriel de Perthuis wrote:
> Originally from
> https://github.com/markfasheh/duperemove/blob/master/btrfs-extent-same.c

Can you (or Mark) please turn it into a subcommand of dedup? The idea is
to merge both in-bound and out-bound dedup into one command, eg.

 btrfs dedup files dir/*

Liu Bo's dedup patch is in today's integration snapshot
http://repo.or.cz/w/btrfs-progs-unstable/devel.git/shortlog/refs/heads/integration-20130806-2

you can use it as a base.

thanks,
david

Re: [PROGS PATCH] Import btrfs-extent-same

[Mark Fasheh]

Hey Dave,

On Tue, Aug 06, 2013 at 05:31:12PM +0200, David Sterba wrote:
> On Thu, Jun 27, 2013 at 12:38:19AM +0200, Gabriel de Perthuis wrote:
> > Originally from
> > https://github.com/markfasheh/duperemove/blob/master/btrfs-extent-same.c
> 
> Can you (or Mark) please turn it into a subcommand of dedup? The idea is
> to merge both in-bound and out-bound dedup into one command, eg.
> 
>  btrfs dedup files dir/*

I can handle this - it's pretty easy but we have to talk about what we're
expecting here.

Specifically, the btrfs-extent-same.c software is just a wrapper around the
ioctl. It never does actual reads of the files or data comparisons, etc. The
syntax you describe above seems like it wants an actual "scan these files
and dedupe them" which is a whole other ball game.

So I would suggest maybe something like the syhntax of btrfs-extent-same.c:

btrfs dedupe files len file1 loff1 file2 loff2 ...

Sound reasonable?
	--Mark

--
Mark Fasheh

Re: [PROGS PATCH] Import btrfs-extent-same

[David Sterba]

On Tue, Aug 13, 2013 at 12:35:15PM -0700, Mark Fasheh wrote:
> > Can you (or Mark) please turn it into a subcommand of dedup? The idea is
> > to merge both in-bound and out-bound dedup into one command, eg.
> > 
> >  btrfs dedup files dir/*
> 
> I can handle this - it's pretty easy but we have to talk about what we're
> expecting here.

> Specifically, the btrfs-extent-same.c software is just a wrapper around the
> ioctl. It never does actual reads of the files or data comparisons, etc. The
> syntax you describe above seems like it wants an actual "scan these files
> and dedupe them" which is a whole other ball game.

Yes that's what I meant and that's what dupremove in your git tree does,
right?

Using the EXTENT_SAME ioctl without any checks is dangerous, and that's
what btrfs-extent-same.c does, so it's suitable for testing but not
about to be given to users as-is.

> So I would suggest maybe something like the syhntax of btrfs-extent-same.c:
> 
> btrfs dedupe files len file1 loff1 file2 loff2 ...

I'm not sure I see what 'len' means here, length of the dedup block?

david

Re: [PROGS PATCH] Import btrfs-extent-same

[David Sterba]

On Mon, Sep 02, 2013 at 05:53:42PM +0100, Simon Farnsworth wrote:
> On Monday 2 September 2013 18:43:58 David Sterba wrote:
> > Yes that's what I meant and that's what dupremove in your git tree does,
> > right?
> > 
> > Using the EXTENT_SAME ioctl without any checks is dangerous, and that's
> > what btrfs-extent-same.c does, so it's suitable for testing but not
> > about to be given to users as-is.
> > 
> Why is using EXTENT_SAME without any checks dangerous?
>
> If userspace has to do checks to guarantee safety, what stops an attacker
> deliberately triggering a TOCTTOU race against a checked user of EXTENT_SAME?
> I would expect that unchecked use of EXTENT_SAME simply causes the kernel to
> return BTRFS_SAME_DATA_DIFFERS most of the time, thus slowing you down.

I was mistaken, the ioctl does checks before merging the extents. Sorry
for confusion.

david

Re: [PROGS PATCH] Import btrfs-extent-same

[David Sterba]

On Mon, Sep 02, 2013 at 06:43:58PM +0200, David Sterba wrote:
> > So I would suggest maybe something like the syhntax of btrfs-extent-same.c:
> > 
> > btrfs dedupe files len file1 loff1 file2 loff2 ...
> 
> I'm not sure I see what 'len' means here, length of the dedup block?

Now I'm reading more carefully, the arguments are the same as for
btrfs-extent-same that does only the simple task of deduping just one
extent, but that's not the point behind 'btrfs dedup files *'.

So there are 2 usecases:

1 - give it a bunch of files and try to dedup as much as possible among
    their data
2 - what btrfs-extent-same does, dedup just a specified range in 2 files

I'm not sure if #2 is going to be used widely though it would bring some
flexibility and fine tuning, besides testing purposes.

I think it would be good to keep both modes under one command, so it's a
matter of a sane UI.

#2 would look like:

$ btrfs dedup files --length 4096 --src-offset 0 --dest-offset 4096 file1 file2

and fail if != 2 files are given

#1 :

$ btrfs dedup files --min-length 65536 file1 file2 file3 ...

I think we could come up with more hints like 'min-length' based on user
requirements, but I'd like to get some agreement if this is the way to
go.

thanks,
david

Re: [PROGS PATCH] Import btrfs-extent-same

[Mark Fasheh]

On Tue, Sep 03, 2013 at 03:41:29PM +0200, David Sterba wrote:
> On Mon, Sep 02, 2013 at 06:43:58PM +0200, David Sterba wrote:
> > > So I would suggest maybe something like the syhntax of btrfs-extent-same.c:
> > > 
> > > btrfs dedupe files len file1 loff1 file2 loff2 ...
> > 
> > I'm not sure I see what 'len' means here, length of the dedup block?
> 
> Now I'm reading more carefully, the arguments are the same as for
> btrfs-extent-same that does only the simple task of deduping just one
> extent, but that's not the point behind 'btrfs dedup files *'.
> 
> So there are 2 usecases:
> 
> 1 - give it a bunch of files and try to dedup as much as possible among
>     their data
> 2 - what btrfs-extent-same does, dedup just a specified range in 2 files
> 
> I'm not sure if #2 is going to be used widely though it would bring some
> flexibility and fine tuning, besides testing purposes.
> 
> I think it would be good to keep both modes under one command, so it's a
> matter of a sane UI.
> 
> #2 would look like:
> 
> $ btrfs dedup files --length 4096 --src-offset 0 --dest-offset 4096 file1 file2
> 
> and fail if != 2 files are given

Each file can have it's own start offset and of course we could include
multiple files, etc.

> 
> #1 :
> 
> $ btrfs dedup files --min-length 65536 file1 file2 file3 ...
> 
> I think we could come up with more hints like 'min-length' based on user
> requirements, but I'd like to get some agreement if this is the way to
> go.


I'm also wondering whether #1 is something that's within the scope of
btrfs-progs.


Btw, attached is a patch to merge what btrfs-extent-same does in the manner
which I was talking about before.


Frankly though I don't like where much of this is going. Aside from
the lazy naming ("btrfs dedupe" would make more sense) I have two major
problems:

1 - We usually change the file system in tunefs. I don't see why turning
online dedupe on / off should be any different. IMHO, this will confuse the end
user who is already used to turning on fs features via tunefs.

2 - That the dedupe command bunches online and offline modes together
is also problematic from an end-user perspective. For example, I had to
include a note in the "files" subcommand that it does not require online
dedupe to be turned on. Again I think most new end users are going to look
at this and be confused.
	--Mark

--
Mark Fasheh

>From 8285eacdffa9fad783efca0ce0b7979f607e9e24 Mon Sep 17 00:00:00 2001
From: Mark Fasheh <mfasheh@suse.de>
Date: Fri, 23 Aug 2013 00:48:17 -0700
Subject: btrfs-progs: add dedupe ioctl support

This patch adds a "files" subcommand to "btrfs dedup".

The files command wraps BTRFS_IOC_FILE_EXTENT_SAME, allowing the user to
do dedupe on a list of predefinied extents.

Signed-off-by: Mark Fasheh <mfasheh@suse.de>
---
 cmds-dedup.c | 94 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
 ioctl.h      | 27 +++++++++++++++++
 2 files changed, 119 insertions(+), 2 deletions(-)

diff --git a/cmds-dedup.c b/cmds-dedup.c
index 215fddc..f4ebb8c 100644
--- a/cmds-dedup.c
+++ b/cmds-dedup.c
@@ -17,7 +17,10 @@
  */
 
 #include <sys/ioctl.h>
+#include <sys/types.h>
+#include <sys/stat.h>
 #include <unistd.h>
+#include <fcntl.h>
 
 #include "ctree.h"
 #include "ioctl.h"
@@ -25,6 +28,91 @@
 #include "commands.h"
 #include "utils.h"
 
+static const char * const cmd_dedup_files_usage[] = {
+	"btrfs dedup files <len> <path1> <off1> <path2> <off2> ...",
+	"Try to deduplicate the file extents given in arguments.",
+	"Automatic deduplication support does not have to be turned on for this to work.",
+	"Extents whose data differs will be skipped for deduplication.",
+	NULL
+};
+
+static int cmd_dedup_files(int argc, char **argv)
+{
+	int ret, src_fd, i, idx, numfiles;
+	char *srcf, *destf;
+	struct btrfs_ioctl_same_args *same;
+	struct btrfs_ioctl_same_extent_info *info;
+
+	if (check_argc_min(argc, 6) || check_argc_exact(argc % 2, 0)) {
+		usage(cmd_dedup_files_usage);
+		return -1;
+	}
+
+	numfiles = (argc - 2) / 2; /* - 2 for cmd, and len args */
+	same = calloc(1,
+		      sizeof(struct btrfs_ioctl_same_args) +
+		      sizeof(struct btrfs_ioctl_same_extent_info) * numfiles);
+	if (!same)
+		return -ENOMEM;
+
+	srcf = argv[2];
+
+	src_fd = open(srcf, O_RDWR);
+	if (src_fd < 0) {
+		ret = -errno;
+		fprintf(stderr, "Could not open file %s: (%d) %s\n",
+			srcf, ret, strerror(-ret));
+		return ret;
+	}
+
+	same->length = parse_size(argv[1]);
+	same->logical_offset = parse_size(argv[3]);
+
+	for (i = 0; i < (numfiles - 1); i++) {
+		idx = 4 + (i * 2);
+		destf = argv[idx];
+
+		ret = open(destf, O_RDONLY);
+		if (ret < 0) {
+			ret = -errno;
+			fprintf(stderr, "Could not open file %s: (%d) %s\n",
+				destf, ret, strerror(-ret));
+			goto close_files;
+		}
+
+		same->info[i].fd = ret;
+		same->info[i].logical_offset = parse_size(argv[idx + 1]);
+		same->dest_count++;
+	}
+
+	printf("Deduplicate extents identical to (%llu, %llu) from %s\n",
+	       (unsigned long long)same->logical_offset,
+	       (unsigned long long)same->length, srcf);
+
+	ret = ioctl(src_fd, BTRFS_IOC_FILE_EXTENT_SAME, same);
+	if (ret) {
+		ret = -errno;
+		fprintf(stderr, "ioctl failed (%d): %s\n", ret, strerror(-ret));
+		goto close_files;
+	}
+
+	printf("%-40s  %-7s  %-s\n", "File", "Status", "Bytes Deduped");
+	for (i = 0; i < same->dest_count; i++) {
+		info = &same->info[i];
+		idx = 4 + (i * 2);
+
+		printf("%-40s   %-6d   %-llu\n", argv[idx],
+		       info->status, (unsigned long long)info->bytes_deduped);
+	}
+
+close_files:
+	close(src_fd);
+	for (i = 0; i < same->dest_count; i++)
+		close(same->info[i].fd);
+
+	return ret;
+}
+
 static const char * const dedup_cmd_group_usage[] = {
 	"btrfs dedup <command> [options] <path>",
 	NULL
@@ -62,7 +150,8 @@ int dedup_ctl(int cmd, int argc, char **argv)
 
 static const char * const cmd_dedup_reg_usage[] = {
 	"btrfs dedup register <path>",
-	"Enable data deduplication support for a filesystem.",
+	"Enable automatic data deduplication support for a filesystem.",
+	"If this is turned on, btrfs will try to deduplicate every file write.",
 	NULL
 };
 
@@ -76,7 +165,7 @@ static int cmd_dedup_reg(int argc, char **argv)
 
 static const char * const cmd_dedup_unreg_usage[] = {
 	"btrfs dedup unregister <path>",
-	"Disable data deduplication support for a filesystem.",
+	"Disable automatic data deduplication support for a filesystem.",
 	NULL
 };
 
@@ -90,6 +179,7 @@ static int cmd_dedup_unreg(int argc, char **argv)
 
 const struct cmd_group dedup_cmd_group = {
 	dedup_cmd_group_usage, NULL, {
+		{ "files", cmd_dedup_files, cmd_dedup_files_usage, 0, 0},
 		{ "register", cmd_dedup_reg, cmd_dedup_reg_usage, NULL, 0 },
 		{ "unregister", cmd_dedup_unreg, cmd_dedup_unreg_usage, 0, 0 },
 		{ 0, 0, 0, 0, 0 }
diff --git a/ioctl.h b/ioctl.h
index e959720..449d912 100644
--- a/ioctl.h
+++ b/ioctl.h
@@ -317,6 +317,31 @@ struct btrfs_ioctl_defrag_range_args {
 	__u32 unused[4];
 };
 
+#define BTRFS_SAME_DATA_DIFFERS	1
+/* For extent-same ioctl */
+struct btrfs_ioctl_same_extent_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 if dedup succeeds
+	 * < 0 for error
+	 * == BTRFS_SAME_DATA_DIFFERS if data differs
+	 */
+	__s32 status;		/* out - see above description */
+	__u32 reserved;
+};
+
+struct btrfs_ioctl_same_args {
+	__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 btrfs_ioctl_same_extent_info info[0];
+};
+
 struct btrfs_ioctl_space_info {
 	__u64 flags;
 	__u64 total_bytes;
@@ -597,6 +622,8 @@ struct btrfs_ioctl_clone_range_args {
 				      struct btrfs_ioctl_get_dev_stats)
 #define BTRFS_IOC_DEV_REPLACE _IOWR(BTRFS_IOCTL_MAGIC, 53, \
 				    struct btrfs_ioctl_dev_replace_args)
+#define BTRFS_IOC_FILE_EXTENT_SAME _IOWR(BTRFS_IOCTL_MAGIC, 54, \
+					 struct btrfs_ioctl_same_args)
 #define BTRFS_IOC_DEDUP_CTL _IOW(BTRFS_IOCTL_MAGIC, 55, int)
 
 #ifdef __cplusplus
-- 
1.8.1.4