diff mbox series

[03/20] btrfs: refactor find_free_dev_extent_start()

Message ID 20200206104214.400857-4-naohiro.aota@wdc.com (mailing list archive)
State New, archived
Headers show
Series btrfs: refactor and generalize chunk/dev_extent/extent allocation | expand

Commit Message

Naohiro Aota Feb. 6, 2020, 10:41 a.m. UTC 
Factor out two functions from find_free_dev_extent_start().
dev_extent_search_start() decides the starting position of the search.
dev_extent_hole_check() checks if a hole found is suitable for device
extent allocation.

These functions also have the switch-cases to change the allocation
behavior depending on the policy.

Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com>
---
 fs/btrfs/volumes.c | 80 ++++++++++++++++++++++++++++++++++------------
 1 file changed, 60 insertions(+), 20 deletions(-)

Comments

Johannes Thumshirn Feb. 6, 2020, 12:02 p.m. UTC | #1 
On 06/02/2020 11:44, Naohiro Aota wrote:
> +/*

Nit: /**

> + * dev_extent_hole_check - check if specified hole is suitable for allocation
> + * @device:	the device which we have the hole
> + * @hole_start: starting position of the hole
> + * @hole_size:	the size of the hole
> + * @num_bytes:	the size of the free space that we need
> + *
> + * This function may modify @hole_start and @hole_end to reflect the
> + * suitable position for allocation. Returns 1 if hole position is
> + * updated, 0 otherwise.
> + */
> +static int dev_extent_hole_check(struct btrfs_device *device, u64 *hole_start,
> +				 u64 *hole_size, u64 num_bytes)
> +{
> +	int ret = 0;
> +	u64 hole_end = *hole_start + *hole_size;
> +

Couldn't this be bool?

Thanks,
	Johannes
Josef Bacik Feb. 6, 2020, 4:34 p.m. UTC | #2 
On 2/6/20 5:41 AM, Naohiro Aota wrote:
> Factor out two functions from find_free_dev_extent_start().
> dev_extent_search_start() decides the starting position of the search.
> dev_extent_hole_check() checks if a hole found is suitable for device
> extent allocation.
> 
> These functions also have the switch-cases to change the allocation
> behavior depending on the policy.
> 
> Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com>

Reviewed-by: Josef Bacik <josef@toxicpanda.com>

Thanks,

Josef
Naohiro Aota Feb. 7, 2020, 6:22 a.m. UTC | #3 
On Thu, Feb 06, 2020 at 12:02:12PM +0000, Johannes Thumshirn wrote:
>On 06/02/2020 11:44, Naohiro Aota wrote:
>> +/*
>
>Nit: /**

Fixed.

>> + * dev_extent_hole_check - check if specified hole is suitable for allocation
>> + * @device:	the device which we have the hole
>> + * @hole_start: starting position of the hole
>> + * @hole_size:	the size of the hole
>> + * @num_bytes:	the size of the free space that we need
>> + *
>> + * This function may modify @hole_start and @hole_end to reflect the
>> + * suitable position for allocation. Returns 1 if hole position is
>> + * updated, 0 otherwise.
>> + */
>> +static int dev_extent_hole_check(struct btrfs_device *device, u64 *hole_start,
>> +				 u64 *hole_size, u64 num_bytes)
>> +{
>> +	int ret = 0;
>> +	u64 hole_end = *hole_start + *hole_size;
>> +
>
>Couldn't this be bool?

Good point. I changed it to bool and also renamed "ret" to "changed"
to make it clear.

Thanks,

>Thanks,
>	Johannes
>
diff mbox series

Patch

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index beee9a94142f..9bb673df777a 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -1384,6 +1384,61 @@  static bool contains_pending_extent(struct btrfs_device *device, u64 *start,
 	return false;
 }
 
+static u64 dev_extent_search_start(struct btrfs_device *device, u64 start)
+{
+	switch (device->fs_devices->chunk_alloc_policy) {
+	case BTRFS_CHUNK_ALLOC_REGULAR:
+		/*
+		 * We don't want to overwrite the superblock on the
+		 * drive nor any area used by the boot loader (grub
+		 * for example), so we make sure to start at an offset
+		 * of at least 1MB.
+		 */
+		return max_t(u64, start, SZ_1M);
+	default:
+		BUG();
+	}
+}
+
+/*
+ * dev_extent_hole_check - check if specified hole is suitable for allocation
+ * @device:	the device which we have the hole
+ * @hole_start: starting position of the hole
+ * @hole_size:	the size of the hole
+ * @num_bytes:	the size of the free space that we need
+ *
+ * This function may modify @hole_start and @hole_end to reflect the
+ * suitable position for allocation. Returns 1 if hole position is
+ * updated, 0 otherwise.
+ */
+static int dev_extent_hole_check(struct btrfs_device *device, u64 *hole_start,
+				 u64 *hole_size, u64 num_bytes)
+{
+	int ret = 0;
+	u64 hole_end = *hole_start + *hole_size;
+
+	/*
+	 * Have to check before we set max_hole_start, otherwise we
+	 * could end up sending back this offset anyway.
+	 */
+	if (contains_pending_extent(device, hole_start, *hole_size)) {
+		if (hole_end >= *hole_start)
+			*hole_size = hole_end - *hole_start;
+		else
+			*hole_size = 0;
+		ret = 1;
+	}
+
+	switch (device->fs_devices->chunk_alloc_policy) {
+	case BTRFS_CHUNK_ALLOC_REGULAR:
+		/* No extra check */
+		break;
+	default:
+		BUG();
+	}
+
+	return ret;
+}
 
 /*
  * find_free_dev_extent_start - find free space in the specified device
@@ -1430,12 +1485,7 @@  static int find_free_dev_extent_start(struct btrfs_device *device,
 	int slot;
 	struct extent_buffer *l;
 
-	/*
-	 * We don't want to overwrite the superblock on the drive nor any area
-	 * used by the boot loader (grub for example), so we make sure to start
-	 * at an offset of at least 1MB.
-	 */
-	search_start = max_t(u64, search_start, SZ_1M);
+	search_start = dev_extent_search_start(device, search_start);
 
 	path = btrfs_alloc_path();
 	if (!path)
@@ -1493,18 +1543,8 @@  static int find_free_dev_extent_start(struct btrfs_device *device,
 
 		if (key.offset > search_start) {
 			hole_size = key.offset - search_start;
-
-			/*
-			 * Have to check before we set max_hole_start, otherwise
-			 * we could end up sending back this offset anyway.
-			 */
-			if (contains_pending_extent(device, &search_start,
-						    hole_size)) {
-				if (key.offset >= search_start)
-					hole_size = key.offset - search_start;
-				else
-					hole_size = 0;
-			}
+			dev_extent_hole_check(device, &search_start, &hole_size,
+					      num_bytes);
 
 			if (hole_size > max_hole_size) {
 				max_hole_start = search_start;
@@ -1543,8 +1583,8 @@  static int find_free_dev_extent_start(struct btrfs_device *device,
 	 */
 	if (search_end > search_start) {
 		hole_size = search_end - search_start;
-
-		if (contains_pending_extent(device, &search_start, hole_size)) {
+		if (dev_extent_hole_check(device, &search_start, &hole_size,
+					  num_bytes)) {
 			btrfs_release_path(path);
 			goto again;
 		}