Balance filters: stripes, enhanced limit

The 'limit' filter is underdesigned, it should have been a range for [min,max], with some relaxed semantics when one of the bounds is missing. Besides that, using a full u64 for a single value is a waste of bytes.

Let's fix both by extending the use of the u64 bytes for the [min,max] range. This can be done in a backward compatible way, the range will be interpreted only if the appropriate flag is set (BTRFS_BALANCE_ARGS_LIMITS).

Signed-off-by David Sterba <[email protected]>

---
 fs/btrfs/ctree.h           | 14 ++++++++++++--
 fs/btrfs/volumes.c         | 42 ++++++++++++++++++++++++++++++++++++++++++
 fs/btrfs/volumes.h         |  1 +
 include/uapi/linux/btrfs.h | 13 ++++++++++++-
 4 files changed, 67 insertions(+), 3 deletions(-)

fs/btrfs/ctree.h +12 -2

index 938efe33be80..7d2e1b6d0ac1 100644
@@ -846,8 +846,18 @@ struct btrfs_disk_balance_args {
 	/* BTRFS_BALANCE_ARGS_* */
 	__le64 flags;
 
-	/* BTRFS_BALANCE_ARGS_LIMIT value */
-	__le64 limit;
+	/*
+	 * BTRFS_BALANCE_ARGS_LIMIT with value 'limit'
+	 * BTRFS_BALANCE_ARGS_LIMITS - the extend version can use minimum and
+	 * maximum
+	 */
+	union {
+		__le64 limit;
+		struct {
+			__le32 limit_min;
+			__le32 limit_max;
+		};
+	};
 
 	__le64 unused[7];
 } __attribute__ ((__packed__));

fs/btrfs/volumes.c +42 -0

index 6fc735869c18..0fbe6855ea05 100644
@@ -3250,6 +3250,16 @@ static int should_balance_chunk(struct btrfs_root *root,
 			return 0;
 		else
 			bargs->limit--;
+	} else if ((bargs->flags & BTRFS_BALANCE_ARGS_LIMITS)) {
+		/*
+		 * Same logic as the 'limit' filter; the minimum cannot be
+		 * determined here because we do not have the global informatoin
+		 * about the count of all chunks that satisfy the filters.
+		 */
+		if (bargs->limit_max == 0)
+			return 0;
+		else
+			bargs->limit_max--;
 	}
 
 	return 1;

@@ -3264,6 +3274,7 @@ static int __btrfs_balance(struct btrfs_fs_info *fs_info)
 	struct btrfs_device *device;
 	u64 old_size;
 	u64 size_to_free;
+	u64 chunk_type;
 	struct btrfs_chunk *chunk;
 	struct btrfs_path *path;
 	struct btrfs_key key;

@@ -3274,9 +3285,13 @@ static int __btrfs_balance(struct btrfs_fs_info *fs_info)
 	int ret;
 	int enospc_errors = 0;
 	bool counting = true;
+	/* The single value limit and min/max limits use the same bytes in the */
 	u64 limit_data = bctl->data.limit;
 	u64 limit_meta = bctl->meta.limit;
 	u64 limit_sys = bctl->sys.limit;
+	u32 count_data = 0;
+	u32 count_meta = 0;
+	u32 count_sys = 0;
 
 	/* step one make some room on all the devices */
 	devices = &fs_info->fs_devices->devices;

@@ -3317,6 +3332,10 @@ static int __btrfs_balance(struct btrfs_fs_info *fs_info)
 	spin_unlock(&fs_info->balance_lock);
 again:
 	if (!counting) {
+		/*
+		 * The single value limit and min/max limits use the same bytes
+		 * in the
+		 */
 		bctl->data.limit = limit_data;
 		bctl->meta.limit = limit_meta;
 		bctl->sys.limit = limit_sys;

@@ -3364,6 +3383,7 @@ static int __btrfs_balance(struct btrfs_fs_info *fs_info)
 		}
 
 		chunk = btrfs_item_ptr(leaf, slot, struct btrfs_chunk);
+		chunk_type = btrfs_chunk_type(leaf, chunk);
 
 		if (!counting) {
 			spin_lock(&fs_info->balance_lock);

@@ -3384,6 +3404,28 @@ static int __btrfs_balance(struct btrfs_fs_info *fs_info)
 			spin_lock(&fs_info->balance_lock);
 			bctl->stat.expected++;
 			spin_unlock(&fs_info->balance_lock);
+
+			if (chunk_type & BTRFS_BLOCK_GROUP_DATA)
+				count_data++;
+			else if (chunk_type & BTRFS_BLOCK_GROUP_SYSTEM)
+				count_sys++;
+			else if (chunk_type & BTRFS_BLOCK_GROUP_METADATA)
+				count_meta++;
+
+			goto loop;
+		}
+
+		/*
+		 * Apply limit_min filter, no need to check if the LIMITS
+		 * filter is used, limit_min is 0 by default
+		 */
+		if (((chunk_type & BTRFS_BLOCK_GROUP_DATA) &&
+					count_data < bctl->data.limit_min)
+				|| ((chunk_type & BTRFS_BLOCK_GROUP_METADATA) &&
+					count_meta < bctl->meta.limit_min)
+				|| ((chunk_type & BTRFS_BLOCK_GROUP_SYSTEM) &&
+					count_sys < bctl->sys.limit_min)) {
+			mutex_unlock(&fs_info->delete_unused_bgs_mutex);
 			goto loop;
 		}
 

fs/btrfs/volumes.h +1 -0

index 2ca784a14e84..1c9d8edd7d57 100644
@@ -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_LIMITS	(1ULL << 6)
 
 /*
  * Profile changing flags.  When SOFT is set we won't relocate chunk if

include/uapi/linux/btrfs.h +12 -1

index b6dec05c7196..264ecea5ecfc 100644
@@ -217,7 +217,18 @@ struct btrfs_balance_args {
 
 	__u64 flags;
 
-	__u64 limit;		/* limit number of processed chunks */
+	/*
+	 * BTRFS_BALANCE_ARGS_LIMIT with value 'limit'
+	 * BTRFS_BALANCE_ARGS_LIMITS - the extend version can use minimum and
+	 * maximum
+	 */
+	union {
+		__u64 limit;		/* limit number of processed chunks */
+		struct {
+			__u32 limit_min;
+			__u32 limit_max;
+		};
+	};
 	__u64 unused[7];
 } __attribute__ ((__packed__));
 

Signature · 6 lines

2.6.1

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Quoted 4 lines · David Sterba

The 'limit' filter is underdesigned, it should have been a range for [min,max], with some relaxed semantics when one of the bounds is missing. Besides that, using a full u64 for a single value is a waste of bytes.

What is min for?

If we have more than 'max' matching chunks, we'd process 'max' and stop.

If we have fewer than 'min' chunks, we'd process what we have and run out.

If we have more than 'min' chunks but fewer than 'max' chunks, why would we stop before we reach 'max' chunks?

Quoted 8 lines · Zygo Blaxell

Quoted 4 lines · David Sterba

The 'limit' filter is underdesigned, it should have been a range for [min,max], with some relaxed semantics when one of the bounds is missing. Besides that, using a full u64 for a single value is a waste of bytes.

What is min for?

Quoted 1 line

If we have more than 'max' matching chunks, we'd process 'max' and stop.

Right.

Quoted 1 line

If we have fewer than 'min' chunks, we'd process what we have and run out.

If we have fewer than min, we'll do nothing.

Quoted 2 lines

If we have more than 'min' chunks but fewer than 'max' chunks, why would we stop before we reach 'max' chunks?

I must be missing something here. If there are less than 'max' chunks (with all filters applied), how are we supposed to reach 'max'?

The 'limit' filter is applied after all the other filters. It can be used standalone, or with eg. usage. The usecase where I find it useful:

* we want to make metadata chunks more compact
* let's set usage=30
* avoid unnecessary work (which is a bigger performance hit in case of metadata chunks), ie. if there's a single chunk with usage < 30%, let's skip it

The command:

 $ btrfs balance start -musage=30,limit=2.. /path

So in case there's only one such chunk, balance would just move it without any gain.

Avoiding the unnecessary work here is IMO a win,
though it might seem limited. But I'm going to utilize this in the btrfsmaintenance package that among others does periodic balancing on the background, I believe the effects will be noticeable.

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html