All of lore.kernel.org
 help / color / mirror / Atom feed
* Add stripes filter
@ 2015-09-28 17:57 Gabríel Arthúr Pétursson
  2015-09-28 21:11 ` Omar Sandoval
                   ` (4 more replies)
  0 siblings, 5 replies; 11+ messages in thread
From: Gabríel Arthúr Pétursson @ 2015-09-28 17:57 UTC (permalink / raw)
  To: linux-btrfs

[-- Attachment #1: Type: text/plain, Size: 398 bytes --]

Hello everyone!

The attached patches to linux and btrfs-progs add support for filtering
based on the number of strips in a block when balancing.

This is my first attempt at kernel development, I'd love if you could
please point out any mistakes that I've made.

Thanks,
Gabríel

P.S. I could not figure out how to subscribe to this mailing list, so
please remember to include me in your replies.

[-- Attachment #2: btrfs-progs.diff --]
[-- Type: text/x-patch, Size: 2903 bytes --]

diff --git a/Documentation/btrfs-balance.asciidoc b/Documentation/btrfs-balance.asciidoc
index 6d2fd0c..8663855 100644
--- a/Documentation/btrfs-balance.asciidoc
+++ b/Documentation/btrfs-balance.asciidoc
@@ -110,6 +110,10 @@ Process only given number of chunks, after all filters apply. This can be used
 to specifically target a chunk in connection with other filters (drange,
 vrange) or just simply limit the amount of work done by a single balance run.
 
+*stripes*::
+Balances only block groups which have the given number of stripes. The
+parameter is either a range specified as <start..end>, or a single integer.
+
 *soft*::
 Takes no parameters. Only has meaning when converting between profiles.
 When doing convert from one profile to another and soft mode is on,
diff --git a/cmds-balance.c b/cmds-balance.c
index 9af218b..c47f8b7 100644
--- a/cmds-balance.c
+++ b/cmds-balance.c
@@ -226,6 +226,21 @@ static int parse_filters(char *filters, struct btrfs_balance_args *args)
 				return 1;
 			}
 			args->flags |= BTRFS_BALANCE_ARGS_LIMIT;
+		} else if (!strcmp(this_char, "stripes")) {
+			if (!value || !*value) {
+				fprintf(stderr,
+					"the stripes filter requires an argument\n");
+				return 1;
+			}
+			if (parse_u64(value, &args->sstart)) {
+				if (parse_range(value, &args->sstart, &args->send)) {
+					fprintf(stderr, "Invalid stripes argument\n");
+					return 1;
+				}
+			} else {
+				args->send = args->sstart;
+			}
+			args->flags |= BTRFS_BALANCE_ARGS_STRIPES;
 		} else {
 			fprintf(stderr, "Unrecognized balance option '%s'\n",
 				this_char);
@@ -262,6 +277,15 @@ static void dump_balance_args(struct btrfs_balance_args *args)
 		       (unsigned long long)args->vend);
 	if (args->flags & BTRFS_BALANCE_ARGS_LIMIT)
 		printf(", limit=%llu", (unsigned long long)args->limit);
+	if (args->flags & BTRFS_BALANCE_ARGS_STRIPES) {
+		if (args->sstart == args->send) {
+			printf(", stripes=%llu", (unsigned long long)args->sstart);
+		} else {
+			printf(", stripes=%llu..%llu",
+			        (unsigned long long)args->sstart,
+			        (unsigned long long)args->send);
+		}
+	}
 
 	printf("\n");
 }
diff --git a/ioctl.h b/ioctl.h
index dff015a..c1f6aec 100644
--- a/ioctl.h
+++ b/ioctl.h
@@ -228,7 +228,11 @@ struct btrfs_balance_args {
 	__u64 flags;
 
 	__u64 limit;		/* limit number of processed chunks */
-	__u64 unused[7];
+
+	__u64 sstart;
+	__u64 send;
+
+	__u64 unused[5];
 } __attribute__ ((__packed__));
 
 /* report balance progress to userspace */
diff --git a/volumes.h b/volumes.h
index 4ecb993..a809a78 100644
--- a/volumes.h
+++ b/volumes.h
@@ -136,6 +136,7 @@ struct map_lookup {
 #define BTRFS_BALANCE_ARGS_DRANGE	(1ULL << 3)
 #define BTRFS_BALANCE_ARGS_VRANGE	(1ULL << 4)
 #define BTRFS_BALANCE_ARGS_LIMIT	(1ULL << 5)
+#define BTRFS_BALANCE_ARGS_STRIPES	(1ULL << 6)
 
 /*
  * Profile changing flags.  When SOFT is set we won't relocate chunk if

[-- Attachment #3: linux.diff --]
[-- Type: text/x-patch, Size: 2277 bytes --]

diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 938efe3..78573e5 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -849,7 +849,11 @@ struct btrfs_disk_balance_args {
 	/* BTRFS_BALANCE_ARGS_LIMIT value */
 	__le64 limit;
 
-	__le64 unused[7];
+	/* btrfs stripes filter */
+	__le64 sstart;
+	__le64 send;
+
+	__le64 unused[5];
 } __attribute__ ((__packed__));
 
 /*
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 6fc73586..dc65fbb 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -3170,6 +3170,18 @@ static int chunk_vrange_filter(struct extent_buffer *leaf,
 	return 1;
 }
 
+static int chunk_stripes_filter(struct extent_buffer *leaf,
+			       struct btrfs_chunk *chunk,
+			       struct btrfs_balance_args *bargs)
+{
+	int num_stripes = btrfs_chunk_num_stripes(leaf, chunk);
+
+	if (bargs->sstart <= num_stripes && num_stripes <= bargs->send)
+		return 0;
+
+	return 1;
+}
+
 static int chunk_soft_convert_filter(u64 chunk_type,
 				     struct btrfs_balance_args *bargs)
 {
@@ -3236,6 +3248,12 @@ static int should_balance_chunk(struct btrfs_root *root,
 		return 0;
 	}
 
+	/* stripes filter */
+	if ((bargs->flags & BTRFS_BALANCE_ARGS_STRIPES) &&
+	    chunk_stripes_filter(leaf, chunk, bargs)) {
+		return 0;
+	}
+
 	/* soft profile changing mode */
 	if ((bargs->flags & BTRFS_BALANCE_ARGS_SOFT) &&
 	    chunk_soft_convert_filter(chunk_type, bargs)) {
diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h
index 2ca784a..fb6b89a 100644
--- a/fs/btrfs/volumes.h
+++ b/fs/btrfs/volumes.h
@@ -375,6 +375,7 @@ struct map_lookup {
 #define BTRFS_BALANCE_ARGS_DRANGE	(1ULL << 3)
 #define BTRFS_BALANCE_ARGS_VRANGE	(1ULL << 4)
 #define BTRFS_BALANCE_ARGS_LIMIT	(1ULL << 5)
+#define BTRFS_BALANCE_ARGS_STRIPES	(1ULL << 6)
 
 /*
  * Profile changing flags.  When SOFT is set we won't relocate chunk if
diff --git a/include/uapi/linux/btrfs.h b/include/uapi/linux/btrfs.h
index b6dec05..a7819d0 100644
--- a/include/uapi/linux/btrfs.h
+++ b/include/uapi/linux/btrfs.h
@@ -218,7 +218,11 @@ struct btrfs_balance_args {
 	__u64 flags;
 
 	__u64 limit;		/* limit number of processed chunks */
-	__u64 unused[7];
+
+	__u64 sstart;
+	__u64 send;
+
+	__u64 unused[5];
 } __attribute__ ((__packed__));
 
 /* report balance progress to userspace */

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

* Re: Add stripes filter
  2015-09-28 17:57 Add stripes filter Gabríel Arthúr Pétursson
@ 2015-09-28 21:11 ` Omar Sandoval
  2015-09-28 22:32 ` [PATCH] btrfs: add " Gabríel Arthúr Pétursson
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 11+ messages in thread
From: Omar Sandoval @ 2015-09-28 21:11 UTC (permalink / raw)
  To: Gabríel Arthúr Pétursson; +Cc: linux-btrfs

On Mon, Sep 28, 2015 at 05:57:05PM +0000, Gabríel Arthúr Pétursson wrote:
> Hello everyone!
> 
> The attached patches to linux and btrfs-progs add support for filtering
> based on the number of strips in a block when balancing.
> 
> This is my first attempt at kernel development, I'd love if you could
> please point out any mistakes that I've made.
> 
> Thanks,
> Gabríel
> 
> P.S. I could not figure out how to subscribe to this mailing list, so
> please remember to include me in your replies.

Hi, Gabríel,

To subscribe to the mailing list, all you have to do is email
majordomo@vger.kernel.org with "subscribe linux-btrfs" in the body (see
http://vger.kernel.org/vger-lists.html#linux-btrfs). Also, be sure to
take a look at https://btrfs.wiki.kernel.org/index.php/Developer%27s_FAQ
and https://www.kernel.org/doc/Documentation/SubmittingPatches
for instructions on how to send in patches. The biggest takeaway is to
use `git format-patch` and `git send-email`.

Thanks!
-- 
Omar

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

* [PATCH] btrfs: add stripes filter
  2015-09-28 17:57 Add stripes filter Gabríel Arthúr Pétursson
  2015-09-28 21:11 ` Omar Sandoval
@ 2015-09-28 22:32 ` Gabríel Arthúr Pétursson
  2015-09-30 15:50   ` David Sterba
  2015-09-28 22:33 ` [PATCH] btrfs-progs: " Gabríel Arthúr Pétursson
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 11+ messages in thread
From: Gabríel Arthúr Pétursson @ 2015-09-28 22:32 UTC (permalink / raw)
  To: linux-btrfs; +Cc: Gabríel Arthúr Pétursson

---
 fs/btrfs/ctree.h           |  6 +++++-
 fs/btrfs/volumes.c         | 18 ++++++++++++++++++
 fs/btrfs/volumes.h         |  1 +
 include/uapi/linux/btrfs.h |  6 +++++-
 4 files changed, 29 insertions(+), 2 deletions(-)

diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 938efe3..78573e5 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -849,7 +849,11 @@ struct btrfs_disk_balance_args {
 	/* BTRFS_BALANCE_ARGS_LIMIT value */
 	__le64 limit;
 
-	__le64 unused[7];
+	/* btrfs stripes filter */
+	__le64 sstart;
+	__le64 send;
+
+	__le64 unused[5];
 } __attribute__ ((__packed__));
 
 /*
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 6fc73586..dc65fbb 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -3170,6 +3170,18 @@ static int chunk_vrange_filter(struct extent_buffer *leaf,
 	return 1;
 }
 
+static int chunk_stripes_filter(struct extent_buffer *leaf,
+			       struct btrfs_chunk *chunk,
+			       struct btrfs_balance_args *bargs)
+{
+	int num_stripes = btrfs_chunk_num_stripes(leaf, chunk);
+
+	if (bargs->sstart <= num_stripes && num_stripes <= bargs->send)
+		return 0;
+
+	return 1;
+}
+
 static int chunk_soft_convert_filter(u64 chunk_type,
 				     struct btrfs_balance_args *bargs)
 {
@@ -3236,6 +3248,12 @@ static int should_balance_chunk(struct btrfs_root *root,
 		return 0;
 	}
 
+	/* stripes filter */
+	if ((bargs->flags & BTRFS_BALANCE_ARGS_STRIPES) &&
+	    chunk_stripes_filter(leaf, chunk, bargs)) {
+		return 0;
+	}
+
 	/* soft profile changing mode */
 	if ((bargs->flags & BTRFS_BALANCE_ARGS_SOFT) &&
 	    chunk_soft_convert_filter(chunk_type, bargs)) {
diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h
index 2ca784a..fb6b89a 100644
--- a/fs/btrfs/volumes.h
+++ b/fs/btrfs/volumes.h
@@ -375,6 +375,7 @@ struct map_lookup {
 #define BTRFS_BALANCE_ARGS_DRANGE	(1ULL << 3)
 #define BTRFS_BALANCE_ARGS_VRANGE	(1ULL << 4)
 #define BTRFS_BALANCE_ARGS_LIMIT	(1ULL << 5)
+#define BTRFS_BALANCE_ARGS_STRIPES	(1ULL << 6)
 
 /*
  * Profile changing flags.  When SOFT is set we won't relocate chunk if
diff --git a/include/uapi/linux/btrfs.h b/include/uapi/linux/btrfs.h
index b6dec05..a7819d0 100644
--- a/include/uapi/linux/btrfs.h
+++ b/include/uapi/linux/btrfs.h
@@ -218,7 +218,11 @@ struct btrfs_balance_args {
 	__u64 flags;
 
 	__u64 limit;		/* limit number of processed chunks */
-	__u64 unused[7];
+
+	__u64 sstart;
+	__u64 send;
+
+	__u64 unused[5];
 } __attribute__ ((__packed__));
 
 /* report balance progress to userspace */
-- 
2.5.3


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

* [PATCH] btrfs-progs: add stripes filter
  2015-09-28 17:57 Add stripes filter Gabríel Arthúr Pétursson
  2015-09-28 21:11 ` Omar Sandoval
  2015-09-28 22:32 ` [PATCH] btrfs: add " Gabríel Arthúr Pétursson
@ 2015-09-28 22:33 ` Gabríel Arthúr Pétursson
  2015-09-29 12:00 ` Add " David Sterba
  2015-10-12 14:10 ` David Sterba
  4 siblings, 0 replies; 11+ messages in thread
From: Gabríel Arthúr Pétursson @ 2015-09-28 22:33 UTC (permalink / raw)
  To: linux-btrfs; +Cc: Gabríel Arthúr Pétursson

---
 Documentation/btrfs-balance.asciidoc |  4 ++++
 cmds-balance.c                       | 24 ++++++++++++++++++++++++
 ioctl.h                              |  6 +++++-
 volumes.h                            |  1 +
 4 files changed, 34 insertions(+), 1 deletion(-)

diff --git a/Documentation/btrfs-balance.asciidoc b/Documentation/btrfs-balance.asciidoc
index 6d2fd0c..8663855 100644
--- a/Documentation/btrfs-balance.asciidoc
+++ b/Documentation/btrfs-balance.asciidoc
@@ -110,6 +110,10 @@ Process only given number of chunks, after all filters apply. This can be used
 to specifically target a chunk in connection with other filters (drange,
 vrange) or just simply limit the amount of work done by a single balance run.
 
+*stripes*::
+Balances only block groups which have the given number of stripes. The
+parameter is either a range specified as <start..end>, or a single integer.
+
 *soft*::
 Takes no parameters. Only has meaning when converting between profiles.
 When doing convert from one profile to another and soft mode is on,
diff --git a/cmds-balance.c b/cmds-balance.c
index 9af218b..c47f8b7 100644
--- a/cmds-balance.c
+++ b/cmds-balance.c
@@ -226,6 +226,21 @@ static int parse_filters(char *filters, struct btrfs_balance_args *args)
 				return 1;
 			}
 			args->flags |= BTRFS_BALANCE_ARGS_LIMIT;
+		} else if (!strcmp(this_char, "stripes")) {
+			if (!value || !*value) {
+				fprintf(stderr,
+					"the stripes filter requires an argument\n");
+				return 1;
+			}
+			if (parse_u64(value, &args->sstart)) {
+				if (parse_range(value, &args->sstart, &args->send)) {
+					fprintf(stderr, "Invalid stripes argument\n");
+					return 1;
+				}
+			} else {
+				args->send = args->sstart;
+			}
+			args->flags |= BTRFS_BALANCE_ARGS_STRIPES;
 		} else {
 			fprintf(stderr, "Unrecognized balance option '%s'\n",
 				this_char);
@@ -262,6 +277,15 @@ static void dump_balance_args(struct btrfs_balance_args *args)
 		       (unsigned long long)args->vend);
 	if (args->flags & BTRFS_BALANCE_ARGS_LIMIT)
 		printf(", limit=%llu", (unsigned long long)args->limit);
+	if (args->flags & BTRFS_BALANCE_ARGS_STRIPES) {
+		if (args->sstart == args->send) {
+			printf(", stripes=%llu", (unsigned long long)args->sstart);
+		} else {
+			printf(", stripes=%llu..%llu",
+			        (unsigned long long)args->sstart,
+			        (unsigned long long)args->send);
+		}
+	}
 
 	printf("\n");
 }
diff --git a/ioctl.h b/ioctl.h
index dff015a..c1f6aec 100644
--- a/ioctl.h
+++ b/ioctl.h
@@ -228,7 +228,11 @@ struct btrfs_balance_args {
 	__u64 flags;
 
 	__u64 limit;		/* limit number of processed chunks */
-	__u64 unused[7];
+
+	__u64 sstart;
+	__u64 send;
+
+	__u64 unused[5];
 } __attribute__ ((__packed__));
 
 /* report balance progress to userspace */
diff --git a/volumes.h b/volumes.h
index 4ecb993..a809a78 100644
--- a/volumes.h
+++ b/volumes.h
@@ -136,6 +136,7 @@ struct map_lookup {
 #define BTRFS_BALANCE_ARGS_DRANGE	(1ULL << 3)
 #define BTRFS_BALANCE_ARGS_VRANGE	(1ULL << 4)
 #define BTRFS_BALANCE_ARGS_LIMIT	(1ULL << 5)
+#define BTRFS_BALANCE_ARGS_STRIPES	(1ULL << 6)
 
 /*
  * Profile changing flags.  When SOFT is set we won't relocate chunk if
-- 
2.5.3


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

* Re: Add stripes filter
  2015-09-28 17:57 Add stripes filter Gabríel Arthúr Pétursson
                   ` (2 preceding siblings ...)
  2015-09-28 22:33 ` [PATCH] btrfs-progs: " Gabríel Arthúr Pétursson
@ 2015-09-29 12:00 ` David Sterba
  2015-09-29 12:10   ` Austin S Hemmelgarn
  2015-10-12 14:10 ` David Sterba
  4 siblings, 1 reply; 11+ messages in thread
From: David Sterba @ 2015-09-29 12:00 UTC (permalink / raw)
  To: Gabríel Arthúr Pétursson; +Cc: linux-btrfs

On Mon, Sep 28, 2015 at 05:57:05PM +0000, Gabríel Arthúr Pétursson wrote:
> The attached patches to linux and btrfs-progs add support for filtering
> based on the number of strips in a block when balancing.

What usecase do you want to address? As I understand it, this would help
the raid56 rebalancing to process only blockgroups that are not spread
accross enough devices.

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

* Re: Add stripes filter
  2015-09-29 12:00 ` Add " David Sterba
@ 2015-09-29 12:10   ` Austin S Hemmelgarn
  2015-09-29 12:21     ` Hugo Mills
  2015-09-30 15:16     ` David Sterba
  0 siblings, 2 replies; 11+ messages in thread
From: Austin S Hemmelgarn @ 2015-09-29 12:10 UTC (permalink / raw)
  To: dsterba, Gabríel Arthúr Pétursson, linux-btrfs

[-- Attachment #1: Type: text/plain, Size: 506 bytes --]

On 2015-09-29 08:00, David Sterba wrote:
> On Mon, Sep 28, 2015 at 05:57:05PM +0000, Gabríel Arthúr Pétursson wrote:
>> The attached patches to linux and btrfs-progs add support for filtering
>> based on the number of strips in a block when balancing.
>
> What usecase do you want to address? As I understand it, this would help
> the raid56 rebalancing to process only blockgroups that are not spread
> accross enough devices.
This could also be helpful when reshaping a raid10 or raid0 setup.


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 3019 bytes --]

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

* Re: Add stripes filter
  2015-09-29 12:10   ` Austin S Hemmelgarn
@ 2015-09-29 12:21     ` Hugo Mills
  2015-09-30 15:28       ` David Sterba
  2015-09-30 15:16     ` David Sterba
  1 sibling, 1 reply; 11+ messages in thread
From: Hugo Mills @ 2015-09-29 12:21 UTC (permalink / raw)
  To: Austin S Hemmelgarn
  Cc: dsterba, Gabríel Arthúr Pétursson, linux-btrfs

[-- Attachment #1: Type: text/plain, Size: 1790 bytes --]

On Tue, Sep 29, 2015 at 08:10:19AM -0400, Austin S Hemmelgarn wrote:
> On 2015-09-29 08:00, David Sterba wrote:
> >On Mon, Sep 28, 2015 at 05:57:05PM +0000, Gabríel Arthúr Pétursson wrote:
> >>The attached patches to linux and btrfs-progs add support for filtering
> >>based on the number of strips in a block when balancing.
> >
> >What usecase do you want to address? As I understand it, this would help
> >the raid56 rebalancing to process only blockgroups that are not spread
> >accross enough devices.

   Exactly. Last week, I was trying to help Gabríel on IRC with a
close-to-full filesystem balance it to add some new devices in a
parity RAID configuration. He'd added the devices and balanced, but
the usage was unequal across the devices. The only way I could think
of dealing with it with the current tools was either to do a full
balance repeatedly until it worked itself out, or to delve into the
metadata with btrfs-debug-tree, and balance selected block groups
individually.

   I whinged that we needed a filter to pick just the block groups
that weren't "as full as possible", and Gabríel picked up the idea and
ran with it.

> This could also be helpful when reshaping a raid10 or raid0 setup.

   Yes, although ultimately less important in most cases, I think,
because you don't lose space by reducing the number of devices in a
block group for those. There are some corner cases where you could end
up losing space by having one (for RAID-0) or up to three (for
RAID-10) devices with more space left than the rest.

   Hugo.

-- 
Hugo Mills             | Dullest spy film ever: The Eastbourne Ultimatum
hugo@... carfax.org.uk |
http://carfax.org.uk/  |
PGP: E2AB1DE4          |                                       The Thick of It

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

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

* Re: Add stripes filter
  2015-09-29 12:10   ` Austin S Hemmelgarn
  2015-09-29 12:21     ` Hugo Mills
@ 2015-09-30 15:16     ` David Sterba
  1 sibling, 0 replies; 11+ messages in thread
From: David Sterba @ 2015-09-30 15:16 UTC (permalink / raw)
  To: Austin S Hemmelgarn; +Cc: Gabríel Arthúr Pétursson, linux-btrfs

On Tue, Sep 29, 2015 at 08:10:19AM -0400, Austin S Hemmelgarn wrote:
> On 2015-09-29 08:00, David Sterba wrote:
> > On Mon, Sep 28, 2015 at 05:57:05PM +0000, Gabríel Arthúr Pétursson wrote:
> >> The attached patches to linux and btrfs-progs add support for filtering
> >> based on the number of strips in a block when balancing.
> >
> > What usecase do you want to address? As I understand it, this would help
> > the raid56 rebalancing to process only blockgroups that are not spread
> > accross enough devices.
> This could also be helpful when reshaping a raid10 or raid0 setup.

Right, I forgot to mention it, I should have said "any raid profile that
uses stripes".

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

* Re: Add stripes filter
  2015-09-29 12:21     ` Hugo Mills
@ 2015-09-30 15:28       ` David Sterba
  0 siblings, 0 replies; 11+ messages in thread
From: David Sterba @ 2015-09-30 15:28 UTC (permalink / raw)
  To: Hugo Mills, Austin S Hemmelgarn,
	Gabríel Arthúr Pétursson, linux-btrfs

On Tue, Sep 29, 2015 at 12:21:39PM +0000, Hugo Mills wrote:
> On Tue, Sep 29, 2015 at 08:10:19AM -0400, Austin S Hemmelgarn wrote:
> > On 2015-09-29 08:00, David Sterba wrote:
> > >On Mon, Sep 28, 2015 at 05:57:05PM +0000, Gabríel Arthúr Pétursson wrote:
> > >>The attached patches to linux and btrfs-progs add support for filtering
> > >>based on the number of strips in a block when balancing.
> > >
> > >What usecase do you want to address? As I understand it, this would help
> > >the raid56 rebalancing to process only blockgroups that are not spread
> > >accross enough devices.
> 
>    Exactly. Last week, I was trying to help Gabríel on IRC with a
> close-to-full filesystem balance it to add some new devices in a
> parity RAID configuration. He'd added the devices and balanced, but
> the usage was unequal across the devices. The only way I could think
> of dealing with it with the current tools was either to do a full
> balance repeatedly until it worked itself out, or to delve into the
> metadata with btrfs-debug-tree, and balance selected block groups
> individually.
> 
>    I whinged that we needed a filter to pick just the block groups
> that weren't "as full as possible", and Gabríel picked up the idea and
> ran with it.

That's great, thanks. The stripe filters are really missing.

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

* Re: [PATCH] btrfs: add stripes filter
  2015-09-28 22:32 ` [PATCH] btrfs: add " Gabríel Arthúr Pétursson
@ 2015-09-30 15:50   ` David Sterba
  0 siblings, 0 replies; 11+ messages in thread
From: David Sterba @ 2015-09-30 15:50 UTC (permalink / raw)
  To: Gabríel Arthúr Pétursson; +Cc: linux-btrfs

Hi,

thanks for the patch. The stripe filter is really helpful. There are
some minor comments below but otherwise the patch looks good.

On Mon, Sep 28, 2015 at 10:32:41PM +0000, Gabríel Arthúr Pétursson wrote:
> --- a/fs/btrfs/ctree.h
> +++ b/fs/btrfs/ctree.h
> @@ -849,7 +849,11 @@ struct btrfs_disk_balance_args {
>  	/* BTRFS_BALANCE_ARGS_LIMIT value */
>  	__le64 limit;
>  
> -	__le64 unused[7];
> +	/* btrfs stripes filter */
> +	__le64 sstart;
> +	__le64 send;

Please be more descriptive, eg. min_stripes/max_stripes. The u64 type
seems too much, I think we can fit the stripe count into a 32bit number.
I made a mistake with u64 type for the 'limit' filter but I think that
we can somehow extend it to be two u32 with the min/max meaning as well.
Either way, this is independent of your patch.

> +
> +	__le64 unused[5];
>  } __attribute__ ((__packed__));
>  
>  /*
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -3236,6 +3248,12 @@ static int should_balance_chunk(struct btrfs_root *root,
>  		return 0;
>  	}
>  
> +	/* stripes filter */
> +	if ((bargs->flags & BTRFS_BALANCE_ARGS_STRIPES) &&
> +	    chunk_stripes_filter(leaf, chunk, bargs)) {
> +		return 0;
> +	}

Ok, I think that this ordering of the filters is right.

> +
>  	/* soft profile changing mode */
>  	if ((bargs->flags & BTRFS_BALANCE_ARGS_SOFT) &&
>  	    chunk_soft_convert_filter(chunk_type, bargs)) {
> diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h
> index 2ca784a..fb6b89a 100644
> --- a/fs/btrfs/volumes.h
> +++ b/fs/btrfs/volumes.h
> @@ -375,6 +375,7 @@ struct map_lookup {
>  #define BTRFS_BALANCE_ARGS_DRANGE	(1ULL << 3)
>  #define BTRFS_BALANCE_ARGS_VRANGE	(1ULL << 4)
>  #define BTRFS_BALANCE_ARGS_LIMIT	(1ULL << 5)
> +#define BTRFS_BALANCE_ARGS_STRIPES	(1ULL << 6)
>  
>  /*
>   * Profile changing flags.  When SOFT is set we won't relocate chunk if
> diff --git a/include/uapi/linux/btrfs.h b/include/uapi/linux/btrfs.h
> index b6dec05..a7819d0 100644
> --- a/include/uapi/linux/btrfs.h
> +++ b/include/uapi/linux/btrfs.h
> @@ -218,7 +218,11 @@ struct btrfs_balance_args {
>  	__u64 flags;
>  
>  	__u64 limit;		/* limit number of processed chunks */
> -	__u64 unused[7];

same comment from the ctree.h applies here

> +
> +	__u64 sstart;
> +	__u64 send;
> +
> +	__u64 unused[5];
>  } __attribute__ ((__packed__));
>  
>  /* report balance progress to userspace */

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

* Re: Add stripes filter
  2015-09-28 17:57 Add stripes filter Gabríel Arthúr Pétursson
                   ` (3 preceding siblings ...)
  2015-09-29 12:00 ` Add " David Sterba
@ 2015-10-12 14:10 ` David Sterba
  4 siblings, 0 replies; 11+ messages in thread
From: David Sterba @ 2015-10-12 14:10 UTC (permalink / raw)
  To: Gabríel Arthúr Pétursson; +Cc: linux-btrfs

On Mon, Sep 28, 2015 at 05:57:05PM +0000, Gabríel Arthúr Pétursson wrote:
> The attached patches to linux and btrfs-progs add support for filtering
> based on the number of strips in a block when balancing.

FYI, I'm going to make the fixups myself as they're mostly cosmetic and
prepare this patch for 4.4 merge window, together with the extension to
the 'limit' filter I mentioned.

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

end of thread, other threads:[~2015-10-12 14:11 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-09-28 17:57 Add stripes filter Gabríel Arthúr Pétursson
2015-09-28 21:11 ` Omar Sandoval
2015-09-28 22:32 ` [PATCH] btrfs: add " Gabríel Arthúr Pétursson
2015-09-30 15:50   ` David Sterba
2015-09-28 22:33 ` [PATCH] btrfs-progs: " Gabríel Arthúr Pétursson
2015-09-29 12:00 ` Add " David Sterba
2015-09-29 12:10   ` Austin S Hemmelgarn
2015-09-29 12:21     ` Hugo Mills
2015-09-30 15:28       ` David Sterba
2015-09-30 15:16     ` David Sterba
2015-10-12 14:10 ` David Sterba

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.