All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] btrfs: Factor out logical address mapping
@ 2019-06-17 13:37 Nikolay Borisov
  2019-06-17 13:37 ` [PATCH 2/2] btrfs: Use btrfs_get_io_geometry/btrfs_get_stripe_rem appropriately Nikolay Borisov
  2019-06-17 15:42 ` [PATCH 1/2] btrfs: Factor out logical address mapping David Sterba
  0 siblings, 2 replies; 3+ messages in thread
From: Nikolay Borisov @ 2019-06-17 13:37 UTC (permalink / raw)
  To: linux-btrfs; +Cc: Nikolay Borisov

Prepare for refactoring and simplifying the code in __btrfs_map_block
by factoring out common code into a separete function -
btrfs_get_io_geometry. It perform necessary actions to map a
(logical addr, len) pair to the underlying physical disk. This code is
also necessary to figure if particular IO req will span a btrfs stripe,
to that effect __btrfs_map_block was called with bio_ret parameter set
to NULL as a way to indicate we only need the "mapping" logic and not
submit.

Further it introduces a simple wrapper over its numerous return
parameters. Having multiple return params rather than a single struct
is a deliberate choice so as not to bloat the stack.

Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
 fs/btrfs/volumes.c | 119 +++++++++++++++++++++++++++++++++++++++++++++
 fs/btrfs/volumes.h |   2 +
 2 files changed, 121 insertions(+)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 41813813f840..d2627d3f9043 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -5915,6 +5915,125 @@ static bool need_full_stripe(enum btrfs_map_op op)
 	return (op == BTRFS_MAP_WRITE || op == BTRFS_MAP_GET_READ_MIRRORS);
 }
 
+/*
+ * btrfs_get_io_geometry - calculates the geomery of a particular (address, len)
+ *		       tuple. This information is used to calculate how big a
+ *		       particular bio can get before it straddles a stripe.
+ *
+ * @fs_info - The omnipresent btrfs structure
+ * @logical - Address that we want to figure out the geometry of
+ * @len	    - Size of IO we are going to perform, starting at @logical
+ * @op      - Type of operation - Write or Read
+ * @out_len - length before we cross btrfs' internal stripe boundary
+ * @out_offset - offset of @logical in chunk
+ * @out_stripe_len - length of a single stripe
+ * @out_stripe_nr - the number of stripe @logical falls within
+ * @out_stripe_offset - offset of @logical in stripe
+ * @out_raid56_stripe_offset - offset of raid56 stripe in chunk
+ *
+ * Returns < 0 in case a chunk for the given logical address cannot be found,
+ * usually shouldn't happen unless @logical is corrupted, 0 otherwise.
+ */
+static int btrfs_get_io_geometry(struct btrfs_fs_info *fs_info,
+			     enum btrfs_map_op op, u64 logical, u64 len,
+			     u64 *out_len, u64 *out_offset, u64 *out_stripe_len,
+			     u64 *out_stripe_nr, u64 *out_stripe_offset,
+			     u64 *out_raid56_stripe_offset)
+
+{
+	struct extent_map *em;
+	struct map_lookup *map;
+	u64 offset;
+	u64 stripe_offset;
+	u64 stripe_nr;
+	u64 stripe_len;
+	u64 raid56_full_stripe_start = (u64)-1;
+	int data_stripes;
+
+	ASSERT(op != BTRFS_MAP_DISCARD);
+
+	em = btrfs_get_chunk_map(fs_info, logical, len);
+	if (IS_ERR(em))
+		return PTR_ERR(em);
+
+	map = em->map_lookup;
+	/* Offset of this logical address in the chunk */
+	offset = logical - em->start;
+	/* Len of a stripe in a chunk */
+	stripe_len = map->stripe_len;
+	/* Stripe wher this block falls in */
+	stripe_nr = div64_u64(offset, stripe_len);
+	/* Offset of stripe in the chunk */
+	stripe_offset = stripe_nr * stripe_len;
+	if (offset < stripe_offset) {
+		btrfs_crit(fs_info,
+			   "stripe math has gone wrong, stripe_offset=%llu, offset=%llu, start=%llu, logical=%llu, stripe_len=%llu",
+			   stripe_offset, offset, em->start, logical,
+			   stripe_len);
+		free_extent_map(em);
+		return -EINVAL;
+	}
+
+	/* stripe_offset is the offset of this block in its stripe*/
+	stripe_offset = offset - stripe_offset;
+	data_stripes = nr_data_stripes(map);
+
+	if (map->type & BTRFS_BLOCK_GROUP_PROFILE_MASK) {
+		u64 max_len = stripe_len - stripe_offset;
+
+		/*
+		 * In case of raid56, we need to know the stripe aligned start
+		 */
+		if (map->type & BTRFS_BLOCK_GROUP_RAID56_MASK) {
+			unsigned long full_stripe_len = stripe_len * data_stripes;
+			raid56_full_stripe_start = offset;
+
+			/*
+			 * Allow a write of a full stripe, but make sure we
+			 * don't allow straddling of stripes
+			 */
+			raid56_full_stripe_start = div64_u64(raid56_full_stripe_start,
+					full_stripe_len);
+			raid56_full_stripe_start *= full_stripe_len;
+
+			/*
+			 * For writes to RAID[56], allow a full stripeset across
+			 * all disks. For other RAID types and for RAID[56]
+			 * reads, just allow a single stripe (on a single disk).
+			 */
+			if (op == BTRFS_MAP_WRITE) {
+				max_len = stripe_len * data_stripes -
+					(offset - raid56_full_stripe_start);
+			}
+		}
+		len = min_t(u64, em->len - offset, max_len);
+	} else {
+		len = em->len - offset;
+	}
+
+	if (out_len)
+		*out_len = len;
+	if (out_offset)
+		*out_offset = offset;
+	if (out_stripe_len)
+		*out_stripe_len = stripe_len;
+	if (out_stripe_nr)
+		*out_stripe_nr = stripe_nr;
+	if (out_stripe_offset)
+		*out_stripe_offset = stripe_offset;
+	if (out_raid56_stripe_offset)
+		*out_raid56_stripe_offset = raid56_full_stripe_start;
+
+	return 0;
+}
+
+int btrfs_get_stripe_rem(struct btrfs_fs_info *fs_info, enum btrfs_map_op op,
+			 u64 logical, u64 len, u64 *out_len)
+{
+	return btrfs_get_io_geometry(fs_info, op, logical, len, out_len, NULL,
+				 NULL, NULL, NULL, NULL);
+}
+
 static int __btrfs_map_block(struct btrfs_fs_info *fs_info,
 			     enum btrfs_map_op op,
 			     u64 logical, u64 *length,
diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h
index fea7b65a712e..10b18c38591b 100644
--- a/fs/btrfs/volumes.h
+++ b/fs/btrfs/volumes.h
@@ -413,6 +413,8 @@ int btrfs_map_block(struct btrfs_fs_info *fs_info, enum btrfs_map_op op,
 int btrfs_map_sblock(struct btrfs_fs_info *fs_info, enum btrfs_map_op op,
 		     u64 logical, u64 *length,
 		     struct btrfs_bio **bbio_ret);
+int btrfs_get_stripe_rem(struct btrfs_fs_info *fs_info, enum btrfs_map_op op,
+			 u64 logical, u64 len, u64 *out_len);
 int btrfs_rmap_block(struct btrfs_fs_info *fs_info, u64 chunk_start,
 		     u64 physical, u64 **logical, int *naddrs, int *stripe_len);
 int btrfs_read_sys_array(struct btrfs_fs_info *fs_info);
-- 
2.17.1


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

* [PATCH 2/2] btrfs: Use btrfs_get_io_geometry/btrfs_get_stripe_rem appropriately
  2019-06-17 13:37 [PATCH 1/2] btrfs: Factor out logical address mapping Nikolay Borisov
@ 2019-06-17 13:37 ` Nikolay Borisov
  2019-06-17 15:42 ` [PATCH 1/2] btrfs: Factor out logical address mapping David Sterba
  1 sibling, 0 replies; 3+ messages in thread
From: Nikolay Borisov @ 2019-06-17 13:37 UTC (permalink / raw)
  To: linux-btrfs; +Cc: Nikolay Borisov

Presently btrfs_map_block is used not only to do everything necessary
to map a bio to the underlying allocation profile but it's also used to
identify how much data could be written based on btrfs' stripe logic
without actually submitting anything. This is achieved by passing NULL
for 'bbio_ret' parameter.

This patch refactors all callers that require just the mapping length
by switching them to btrfs_get_stripe_rem instead of calling
btrfs_map_block with a special NULL value for 'bbio_ret'. It also
switches __btrfs_map_block to calling btrfs_get_io_geometry.

No functional changes.

Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
 fs/btrfs/inode.c   | 22 +++++++-------
 fs/btrfs/volumes.c | 71 ++++++----------------------------------------
 2 files changed, 21 insertions(+), 72 deletions(-)

diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 7126493d8d8c..253ee6e6b695 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -1931,6 +1931,7 @@ int btrfs_bio_fits_in_stripe(struct page *page, size_t size, struct bio *bio,
 	u64 logical = (u64)bio->bi_iter.bi_sector << 9;
 	u64 length = 0;
 	u64 map_length;
+	u64 stripe_rem_len;
 	int ret;
 
 	if (bio_flags & EXTENT_BIO_COMPRESSED)
@@ -1938,11 +1939,12 @@ int btrfs_bio_fits_in_stripe(struct page *page, size_t size, struct bio *bio,
 
 	length = bio->bi_iter.bi_size;
 	map_length = length;
-	ret = btrfs_map_block(fs_info, btrfs_op(bio), logical, &map_length,
-			      NULL, 0);
+	ret = btrfs_get_stripe_rem(fs_info, btrfs_op(bio), logical, map_length,
+				   &stripe_rem_len);
 	if (ret < 0)
 		return ret;
-	if (map_length < length + size)
+
+	if (stripe_rem_len < length + size)
 		return 1;
 	return 0;
 }
@@ -8316,10 +8318,10 @@ static int btrfs_submit_direct_hook(struct btrfs_dio_private *dip)
 	int ret;
 	blk_status_t status;
 
-	map_length = orig_bio->bi_iter.bi_size;
-	submit_len = map_length;
-	ret = btrfs_map_block(fs_info, btrfs_op(orig_bio), start_sector << 9,
-			      &map_length, NULL, 0);
+	submit_len = orig_bio->bi_iter.bi_size;
+	ret = btrfs_get_stripe_rem(fs_info, btrfs_op(orig_bio),
+				   start_sector << 9, submit_len,
+				   &map_length);
 	if (ret)
 		return -EIO;
 
@@ -8376,9 +8378,9 @@ static int btrfs_submit_direct_hook(struct btrfs_dio_private *dip)
 		start_sector += clone_len >> 9;
 		file_offset += clone_len;
 
-		map_length = submit_len;
-		ret = btrfs_map_block(fs_info, btrfs_op(orig_bio),
-				      start_sector << 9, &map_length, NULL, 0);
+		ret = btrfs_get_stripe_rem(fs_info, btrfs_op(orig_bio),
+					   start_sector << 9, submit_len,
+					   &map_length);
 		if (ret)
 			goto out_err;
 	} while (submit_len > 0);
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index d2627d3f9043..05197620a6e2 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -6061,77 +6061,24 @@ static int __btrfs_map_block(struct btrfs_fs_info *fs_info,
 	u64 physical_to_patch_in_first_stripe = 0;
 	u64 raid56_full_stripe_start = (u64)-1;
 
+	ASSERT(bbio_ret);
+
 	if (op == BTRFS_MAP_DISCARD)
 		return __btrfs_map_block_for_discard(fs_info, logical,
 						     *length, bbio_ret);
 
-	em = btrfs_get_chunk_map(fs_info, logical, *length);
-	if (IS_ERR(em))
-		return PTR_ERR(em);
+	ret = btrfs_get_io_geometry(fs_info, op, logical, *length, length, &offset,
+				&stripe_len, &stripe_nr, &stripe_offset,
+				&raid56_full_stripe_start);
+	if (ret < 0)
+		return ret;
 
+	em = btrfs_get_chunk_map(fs_info, logical, *length);
+	ASSERT(em);
 	map = em->map_lookup;
-	offset = logical - em->start;
 
-	stripe_len = map->stripe_len;
-	stripe_nr = offset;
-	/*
-	 * stripe_nr counts the total number of stripes we have to stride
-	 * to get to this block
-	 */
-	stripe_nr = div64_u64(stripe_nr, stripe_len);
 	data_stripes = nr_data_stripes(map);
 
-	stripe_offset = stripe_nr * stripe_len;
-	if (offset < stripe_offset) {
-		btrfs_crit(fs_info,
-			   "stripe math has gone wrong, stripe_offset=%llu, offset=%llu, start=%llu, logical=%llu, stripe_len=%llu",
-			   stripe_offset, offset, em->start, logical,
-			   stripe_len);
-		free_extent_map(em);
-		return -EINVAL;
-	}
-
-	/* stripe_offset is the offset of this block in its stripe*/
-	stripe_offset = offset - stripe_offset;
-
-	/* if we're here for raid56, we need to know the stripe aligned start */
-	if (map->type & BTRFS_BLOCK_GROUP_RAID56_MASK) {
-		unsigned long full_stripe_len = stripe_len * data_stripes;
-		raid56_full_stripe_start = offset;
-
-		/* allow a write of a full stripe, but make sure we don't
-		 * allow straddling of stripes
-		 */
-		raid56_full_stripe_start = div64_u64(raid56_full_stripe_start,
-				full_stripe_len);
-		raid56_full_stripe_start *= full_stripe_len;
-	}
-
-	if (map->type & BTRFS_BLOCK_GROUP_PROFILE_MASK) {
-		u64 max_len;
-		/* For writes to RAID[56], allow a full stripeset across all disks.
-		   For other RAID types and for RAID[56] reads, just allow a single
-		   stripe (on a single disk). */
-		if ((map->type & BTRFS_BLOCK_GROUP_RAID56_MASK) &&
-		    (op == BTRFS_MAP_WRITE)) {
-			max_len = stripe_len * data_stripes -
-				(offset - raid56_full_stripe_start);
-		} else {
-			/* we limit the length of each bio to what fits in a stripe */
-			max_len = stripe_len - stripe_offset;
-		}
-		*length = min_t(u64, em->len - offset, max_len);
-	} else {
-		*length = em->len - offset;
-	}
-
-	/*
-	 * This is for when we're called from btrfs_bio_fits_in_stripe and all
-	 * it cares about is the length
-	 */
-	if (!bbio_ret)
-		goto out;
-
 	down_read(&dev_replace->rwsem);
 	dev_replace_is_ongoing = btrfs_dev_replace_is_ongoing(dev_replace);
 	/*
-- 
2.17.1


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

* Re: [PATCH 1/2] btrfs: Factor out logical address mapping
  2019-06-17 13:37 [PATCH 1/2] btrfs: Factor out logical address mapping Nikolay Borisov
  2019-06-17 13:37 ` [PATCH 2/2] btrfs: Use btrfs_get_io_geometry/btrfs_get_stripe_rem appropriately Nikolay Borisov
@ 2019-06-17 15:42 ` David Sterba
  1 sibling, 0 replies; 3+ messages in thread
From: David Sterba @ 2019-06-17 15:42 UTC (permalink / raw)
  To: Nikolay Borisov; +Cc: linux-btrfs

On Mon, Jun 17, 2019 at 04:37:16PM +0300, Nikolay Borisov wrote:
> Prepare for refactoring and simplifying the code in __btrfs_map_block
> by factoring out common code into a separete function -
> btrfs_get_io_geometry. It perform necessary actions to map a
> (logical addr, len) pair to the underlying physical disk. This code is
> also necessary to figure if particular IO req will span a btrfs stripe,
> to that effect __btrfs_map_block was called with bio_ret parameter set
> to NULL as a way to indicate we only need the "mapping" logic and not
> submit.
> 
> Further it introduces a simple wrapper over its numerous return
> parameters. Having multiple return params rather than a single struct
> is a deliberate choice so as not to bloat the stack.
> 
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>

This turns out a bit worse compared to the struct approach in v1. The
stack stats for v2:

__btrfs_map_block                                  +64 (200 -> 264)

NEW (152):

btrfs_get_io_geometry
btrfs_get_stripe_rem

LOST/NEW DELTA:     +152
PRE/POST DELTA:     +216

From the code prespective the struct is more readable than a series of NULL
parameters. That would be justifiable in case the stack consumption would be
dramatically better, but it's not.

Thanks for the v2, I think this exercise was useful. I'm going to merge v1.

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

end of thread, other threads:[~2019-06-17 15:41 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-17 13:37 [PATCH 1/2] btrfs: Factor out logical address mapping Nikolay Borisov
2019-06-17 13:37 ` [PATCH 2/2] btrfs: Use btrfs_get_io_geometry/btrfs_get_stripe_rem appropriately Nikolay Borisov
2019-06-17 15:42 ` [PATCH 1/2] btrfs: Factor out logical address mapping 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.