All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] duperemove: test presence of dedupe ioctl
@ 2016-12-09 17:56 Darrick J. Wong
  2016-12-14 10:44 ` Christoph Hellwig
  0 siblings, 1 reply; 6+ messages in thread
From: Darrick J. Wong @ 2016-12-09 17:56 UTC (permalink / raw)
  To: mfasheh; +Cc: xfs, linux-btrfs

Since a zero-length dedupe operation is guaranteed to succeed, use that
to test whether or not this filesystem supports dedupe.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 file_scan.c |   47 +++++++++++++++++++++++++++++++++++++----------
 1 file changed, 37 insertions(+), 10 deletions(-)

diff --git a/file_scan.c b/file_scan.c
index 617f166..a34453e 100644
--- a/file_scan.c
+++ b/file_scan.c
@@ -45,11 +45,7 @@
 #include "file_scan.h"
 #include "dbfile.h"
 #include "util.h"
-
-/* This is not in linux/magic.h */
-#ifndef	XFS_SB_MAGIC
-#define	XFS_SB_MAGIC		0x58465342	/* 'XFSB' */
-#endif
+#include "btrfs-ioctl.h"
 
 static char path[PATH_MAX] = { 0, };
 static char *pathp = path;
@@ -189,6 +185,39 @@ static int walk_dir(const char *name)
 	return ret;
 }
 
+struct fake_btrfs_ioctl_same_args {
+	struct btrfs_ioctl_same_args args;
+	struct btrfs_ioctl_same_extent_info info;
+};
+
+/*
+ * A zero-length dedupe between two files should always succeed,
+ * so we can use this to test the presence of dedupe functionality.
+ */
+static bool check_ioctl_works(int fd)
+{
+	struct fake_btrfs_ioctl_same_args sa = {0};
+	struct stat sb;
+	static int cached = -1;
+	int ret;
+
+	if (cached >= 0)
+		return cached != 0;
+
+	ret = fstat(fd, &sb);
+	if (ret)
+		return false;
+
+	sa.args.dest_count = 1;
+	sa.args.length = 0;
+	sa.info.fd = fd;
+	sa.info.logical_offset = 0;
+	errno = 0;
+	ret = btrfs_extent_same(fd, &sa.args);
+	cached = !ret && !errno && !sa.info.status;
+	return cached != 0;
+}
+
 static int __add_file(const char *name, struct stat *st,
 		      struct filerec **ret_file)
 {
@@ -235,12 +264,10 @@ static int __add_file(const char *name, struct stat *st,
 		goto out;
 	}
 
-	if (run_dedupe &&
-	    ((fs.f_type != BTRFS_SUPER_MAGIC &&
-	      fs.f_type != XFS_SB_MAGIC))) {
+	if (run_dedupe && !check_ioctl_works(fd)) {
 		close(fd);
-		fprintf(stderr,	"\"%s\": Can only dedupe files on btrfs or xfs "
-			"(experimental)\n", name);
+		fprintf(stderr,	"\"%s\": dedupe ioctl not supported on this "
+			"filesystem.\n", name);
 		return ENOSYS;
 	}
 

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] duperemove: test presence of dedupe ioctl
  2016-12-09 17:56 [PATCH] duperemove: test presence of dedupe ioctl Darrick J. Wong
@ 2016-12-14 10:44 ` Christoph Hellwig
  2016-12-14 18:38   ` Darrick J. Wong
  0 siblings, 1 reply; 6+ messages in thread
From: Christoph Hellwig @ 2016-12-14 10:44 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: mfasheh, xfs, linux-btrfs

On Fri, Dec 09, 2016 at 09:56:45AM -0800, Darrick J. Wong wrote:
> Since a zero-length dedupe operation is guaranteed to succeed, use that
> to test whether or not this filesystem supports dedupe.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  file_scan.c |   47 +++++++++++++++++++++++++++++++++++++----------
>  1 file changed, 37 insertions(+), 10 deletions(-)
> 
> diff --git a/file_scan.c b/file_scan.c
> index 617f166..a34453e 100644
> --- a/file_scan.c
> +++ b/file_scan.c
> @@ -45,11 +45,7 @@
>  #include "file_scan.h"
>  #include "dbfile.h"
>  #include "util.h"
> -
> -/* This is not in linux/magic.h */
> -#ifndef	XFS_SB_MAGIC
> -#define	XFS_SB_MAGIC		0x58465342	/* 'XFSB' */
> -#endif
> +#include "btrfs-ioctl.h"
>  
>  static char path[PATH_MAX] = { 0, };
>  static char *pathp = path;
> @@ -189,6 +185,39 @@ static int walk_dir(const char *name)
>  	return ret;
>  }
>  
> +struct fake_btrfs_ioctl_same_args {
> +	struct btrfs_ioctl_same_args args;
> +	struct btrfs_ioctl_same_extent_info info;
> +};

Why does this need a fake structure here?

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] duperemove: test presence of dedupe ioctl
  2016-12-14 10:44 ` Christoph Hellwig
@ 2016-12-14 18:38   ` Darrick J. Wong
  2016-12-14 19:26     ` Christoph Hellwig
  0 siblings, 1 reply; 6+ messages in thread
From: Darrick J. Wong @ 2016-12-14 18:38 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: mfasheh, xfs, linux-btrfs

On Wed, Dec 14, 2016 at 02:44:36AM -0800, Christoph Hellwig wrote:
> On Fri, Dec 09, 2016 at 09:56:45AM -0800, Darrick J. Wong wrote:
> > Since a zero-length dedupe operation is guaranteed to succeed, use that
> > to test whether or not this filesystem supports dedupe.
> > 
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > ---
> >  file_scan.c |   47 +++++++++++++++++++++++++++++++++++++----------
> >  1 file changed, 37 insertions(+), 10 deletions(-)
> > 
> > diff --git a/file_scan.c b/file_scan.c
> > index 617f166..a34453e 100644
> > --- a/file_scan.c
> > +++ b/file_scan.c
> > @@ -45,11 +45,7 @@
> >  #include "file_scan.h"
> >  #include "dbfile.h"
> >  #include "util.h"
> > -
> > -/* This is not in linux/magic.h */
> > -#ifndef	XFS_SB_MAGIC
> > -#define	XFS_SB_MAGIC		0x58465342	/* 'XFSB' */
> > -#endif
> > +#include "btrfs-ioctl.h"
> >  
> >  static char path[PATH_MAX] = { 0, };
> >  static char *pathp = path;
> > @@ -189,6 +185,39 @@ static int walk_dir(const char *name)
> >  	return ret;
> >  }
> >  
> > +struct fake_btrfs_ioctl_same_args {
> > +	struct btrfs_ioctl_same_args args;
> > +	struct btrfs_ioctl_same_extent_info info;
> > +};
> 
> Why does this need a fake structure here?

In order to test the ioctl we have to fill out at least one
btrfs_ioctl_same_extent_info so that we get far enough into the fs-specific
dedupe_range handler that we've verified that the fs is capable of dedupe and
that the fs is willing to try to satisfy the request.

We could just malloc sizeof(_same_args) + sizeof(_same_extent_info)...

--D

> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] duperemove: test presence of dedupe ioctl
  2016-12-14 18:38   ` Darrick J. Wong
@ 2016-12-14 19:26     ` Christoph Hellwig
  2016-12-16  1:20       ` Darrick J. Wong
  0 siblings, 1 reply; 6+ messages in thread
From: Christoph Hellwig @ 2016-12-14 19:26 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: Christoph Hellwig, mfasheh, xfs, linux-btrfs

On Wed, Dec 14, 2016 at 10:38:45AM -0800, Darrick J. Wong wrote:
> > > +struct fake_btrfs_ioctl_same_args {
> > > +	struct btrfs_ioctl_same_args args;
> > > +	struct btrfs_ioctl_same_extent_info info;
> > > +};
> > 
> > Why does this need a fake structure here?
> 
> In order to test the ioctl we have to fill out at least one
> btrfs_ioctl_same_extent_info so that we get far enough into the fs-specific
> dedupe_range handler that we've verified that the fs is capable of dedupe and
> that the fs is willing to try to satisfy the request.

Oh, got it, it's just the fake that tripped me up.

> We could just malloc sizeof(_same_args) + sizeof(_same_extent_info)...

Either that, or more simply just don't give the structure a name
by just declaring it locally on the stack:

	struct {
		struct btrfs_ioctl_same_args args;
		struct btrfs_ioctl_same_extent_info info;
	} sa = { 0 };


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] duperemove: test presence of dedupe ioctl
  2016-12-14 19:26     ` Christoph Hellwig
@ 2016-12-16  1:20       ` Darrick J. Wong
  2016-12-16  7:53         ` Christoph Hellwig
  0 siblings, 1 reply; 6+ messages in thread
From: Darrick J. Wong @ 2016-12-16  1:20 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: mfasheh, xfs, linux-btrfs

On Wed, Dec 14, 2016 at 11:26:07AM -0800, Christoph Hellwig wrote:
> On Wed, Dec 14, 2016 at 10:38:45AM -0800, Darrick J. Wong wrote:
> > > > +struct fake_btrfs_ioctl_same_args {
> > > > +	struct btrfs_ioctl_same_args args;
> > > > +	struct btrfs_ioctl_same_extent_info info;
> > > > +};
> > > 
> > > Why does this need a fake structure here?
> > 
> > In order to test the ioctl we have to fill out at least one
> > btrfs_ioctl_same_extent_info so that we get far enough into the fs-specific
> > dedupe_range handler that we've verified that the fs is capable of dedupe and
> > that the fs is willing to try to satisfy the request.
> 
> Oh, got it, it's just the fake that tripped me up.
> 
> > We could just malloc sizeof(_same_args) + sizeof(_same_extent_info)...
> 
> Either that, or more simply just don't give the structure a name
> by just declaring it locally on the stack:
> 
> 	struct {
> 		struct btrfs_ioctl_same_args args;
> 		struct btrfs_ioctl_same_extent_info info;
> 	} sa = { 0 };

Fair enough, no need to pollute the namespace.

--D

> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] duperemove: test presence of dedupe ioctl
  2016-12-16  1:20       ` Darrick J. Wong
@ 2016-12-16  7:53         ` Christoph Hellwig
  0 siblings, 0 replies; 6+ messages in thread
From: Christoph Hellwig @ 2016-12-16  7:53 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: Christoph Hellwig, mfasheh, xfs, linux-btrfs

On Thu, Dec 15, 2016 at 05:20:14PM -0800, Darrick J. Wong wrote:
> Fair enough, no need to pollute the namespace.

Or confuse poor readers with the 'fake' name :)

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2016-12-16  7:53 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-12-09 17:56 [PATCH] duperemove: test presence of dedupe ioctl Darrick J. Wong
2016-12-14 10:44 ` Christoph Hellwig
2016-12-14 18:38   ` Darrick J. Wong
2016-12-14 19:26     ` Christoph Hellwig
2016-12-16  1:20       ` Darrick J. Wong
2016-12-16  7:53         ` Christoph Hellwig

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.