linux-btrfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] refactoring __btrfs_map_block
@ 2019-06-03  9:05 Nikolay Borisov
  2019-06-03  9:05 ` [PATCH 1/3] btrfs: Introduce struct btrfs_io_geometry Nikolay Borisov
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Nikolay Borisov @ 2019-06-03  9:05 UTC (permalink / raw)
  To: linux-btrfs; +Cc: Nikolay Borisov

__btrfs_map_block is probably one of the longest functions in btrfs and is responsible 
for mapping high-level RW requests to a logical address to lower-level bios
that are sent to multiple devices (depending on the allocation profile the block
group this address belongs to). Additionally, it's also used to calculate the
various characteristic of the given (address,len) tuple such as the internal 
stripe len that remains if the given request is satisfied. 

This conflation of 2 actions make it a bit hard to follow the function, this 
patchset aims to rectify this by factoring out the "address calculation
mechanics" into a separate function. To reduce the number of variables having 
to pass also introduce a struct with the same name that holds all the output 
values. 

Nikolay Borisov (3):
  btrfs: Introduce struct btrfs_io_geometry
  btrfs: Introduce btrfs_io_geometry
  btrfs: Use btrfs_io_geometry appropriately

 fs/btrfs/inode.c   |  25 +++----
 fs/btrfs/volumes.c | 169 +++++++++++++++++++++++++++++----------------
 fs/btrfs/volumes.h |  11 +++
 3 files changed, 133 insertions(+), 72 deletions(-)

-- 
2.17.1


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

* [PATCH 1/3] btrfs: Introduce struct btrfs_io_geometry
  2019-06-03  9:05 [PATCH 0/3] refactoring __btrfs_map_block Nikolay Borisov
@ 2019-06-03  9:05 ` Nikolay Borisov
  2019-06-05  7:35   ` Johannes Thumshirn
  2019-06-03  9:05 ` [PATCH 2/3] btrfs: Introduce btrfs_io_geometry Nikolay Borisov
  2019-06-03  9:05 ` [PATCH 3/3] btrfs: Use btrfs_io_geometry appropriately Nikolay Borisov
  2 siblings, 1 reply; 9+ messages in thread
From: Nikolay Borisov @ 2019-06-03  9:05 UTC (permalink / raw)
  To: linux-btrfs; +Cc: Nikolay Borisov

Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
 fs/btrfs/volumes.h | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h
index a7121d72388f..7c1ddf35b7d4 100644
--- a/fs/btrfs/volumes.h
+++ b/fs/btrfs/volumes.h
@@ -23,6 +23,15 @@ struct btrfs_pending_bios {
 	struct bio *tail;
 };
 
+struct btrfs_io_geometry {
+	u64 len; /* remaining bytes in chunk */
+	u64 offset; /* offset of logical address in chunk */
+	u64 stripe_len; /* len of single io stripe */
+	u64 stripe_nr; /* number of stripe where address falls */
+	u64 stripe_offset; /* offset of stripe in chunk */
+	u64 raid56_stripe_offset; /* offset of raid56 stripe into the chunk */
+};
+
 /*
  * Use sequence counter to get consistent device stat data on
  * 32-bit processors.
-- 
2.17.1


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

* [PATCH 2/3] btrfs: Introduce btrfs_io_geometry
  2019-06-03  9:05 [PATCH 0/3] refactoring __btrfs_map_block Nikolay Borisov
  2019-06-03  9:05 ` [PATCH 1/3] btrfs: Introduce struct btrfs_io_geometry Nikolay Borisov
@ 2019-06-03  9:05 ` Nikolay Borisov
  2019-06-05  8:01   ` Johannes Thumshirn
  2019-06-12 16:31   ` David Sterba
  2019-06-03  9:05 ` [PATCH 3/3] btrfs: Use btrfs_io_geometry appropriately Nikolay Borisov
  2 siblings, 2 replies; 9+ messages in thread
From: Nikolay Borisov @ 2019-06-03  9:05 UTC (permalink / raw)
  To: linux-btrfs; +Cc: Nikolay Borisov

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

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 776f5c7ca7c5..b130f465ca6d 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -5907,6 +5907,104 @@ static bool need_full_stripe(enum btrfs_map_op op)
 	return (op == BTRFS_MAP_WRITE || op == BTRFS_MAP_GET_READ_MIRRORS);
 }
 
+/*
+ * btrfs_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	    - The length of IO we are going to perform, starting at @logical
+ * @op      - Type of operation - Write or Read
+ * @io_geom - Pointer used to return values
+ *
+ * Returns < 0 in case a chunk for the given logical address cannot be found,
+ * usually shouldn't happen unless @logical is corrupted, 0 otherwise.
+ */
+int btrfs_io_geometry(struct btrfs_fs_info *fs_info, enum btrfs_map_op op,
+		      u64 logical, u64 len, struct btrfs_io_geometry *io_geom)
+{
+	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;
+	}
+
+	io_geom->len = len;
+	io_geom->offset = offset;
+	io_geom->stripe_len = stripe_len;
+	io_geom->stripe_nr = stripe_nr;
+	io_geom->stripe_offset = stripe_offset;
+	io_geom->raid56_stripe_offset = raid56_full_stripe_start;
+
+	return 0;
+}
+
 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 7c1ddf35b7d4..f3bdf768bbab 100644
--- a/fs/btrfs/volumes.h
+++ b/fs/btrfs/volumes.h
@@ -421,6 +421,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_io_geometry(struct btrfs_fs_info *fs_info, enum btrfs_map_op op,
+		      u64 logical, u64 len, struct btrfs_io_geometry *io_geom);
 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] 9+ messages in thread

* [PATCH 3/3] btrfs: Use btrfs_io_geometry appropriately
  2019-06-03  9:05 [PATCH 0/3] refactoring __btrfs_map_block Nikolay Borisov
  2019-06-03  9:05 ` [PATCH 1/3] btrfs: Introduce struct btrfs_io_geometry Nikolay Borisov
  2019-06-03  9:05 ` [PATCH 2/3] btrfs: Introduce btrfs_io_geometry Nikolay Borisov
@ 2019-06-03  9:05 ` Nikolay Borisov
  2019-06-12 16:29   ` David Sterba
  2 siblings, 1 reply; 9+ messages in thread
From: Nikolay Borisov @ 2019-06-03  9:05 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 using btrfs_io_geometry instead of calling
btrfs_map_block with a special NULL value for 'bbio_ret'. No functional
change.

Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
 fs/btrfs/inode.c   | 25 +++++++--------
 fs/btrfs/volumes.c | 77 +++++++++-------------------------------------
 2 files changed, 27 insertions(+), 75 deletions(-)

diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 80fdf6f21f74..a3abba4c2e2c 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -1932,17 +1932,19 @@ int btrfs_bio_fits_in_stripe(struct page *page, size_t size, struct bio *bio,
 	u64 length = 0;
 	u64 map_length;
 	int ret;
+	struct btrfs_io_geometry geom;
 
 	if (bio_flags & EXTENT_BIO_COMPRESSED)
 		return 0;
 
 	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_io_geometry(fs_info, btrfs_op(bio), logical, map_length,
+				&geom);
 	if (ret < 0)
 		return ret;
-	if (map_length < length + size)
+
+	if (geom.len < length + size)
 		return 1;
 	return 0;
 }
@@ -8331,15 +8333,15 @@ static int btrfs_submit_direct_hook(struct btrfs_dio_private *dip)
 	int clone_len;
 	int ret;
 	blk_status_t status;
+	struct btrfs_io_geometry geom;
 
-	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_io_geometry(fs_info, btrfs_op(orig_bio), start_sector << 9,
+			      submit_len, &geom);
 	if (ret)
 		return -EIO;
 
-	map_length = min_t(u64, map_length, SZ_1M);
+	map_length = min_t(u64, geom.len, SZ_1M);
 	if (map_length >= submit_len) {
 		bio = orig_bio;
 		dip->flags |= BTRFS_DIO_ORIG_BIO_SUBMITTED;
@@ -8393,13 +8395,12 @@ 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_io_geometry(fs_info, btrfs_op(orig_bio),
+				      start_sector << 9, submit_len, &geom);
 		if (ret)
 			goto out_err;
 
-		map_length = min_t(u64, map_length, SZ_1M);
+		map_length = min_t(u64, geom.len, SZ_1M);
 	} while (submit_len > 0);
 
 submit:
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index b130f465ca6d..9d9d1d3329bb 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -5961,7 +5961,6 @@ int btrfs_io_geometry(struct btrfs_fs_info *fs_info, enum btrfs_map_op op,
 	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;
 
@@ -6031,78 +6030,30 @@ static int __btrfs_map_block(struct btrfs_fs_info *fs_info,
 	int patch_the_first_stripe_for_dev_replace = 0;
 	u64 physical_to_patch_in_first_stripe = 0;
 	u64 raid56_full_stripe_start = (u64)-1;
+	struct btrfs_io_geometry geom;
+
+	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_io_geometry(fs_info, op, logical, *length, &geom);
+	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);
+	*length = geom.len;
+	offset = geom.offset;
+	stripe_len = geom.stripe_len;
+	stripe_nr = geom.stripe_nr;
+	stripe_offset = geom.stripe_offset;
+	raid56_full_stripe_start = geom.raid56_stripe_offset;
 	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] 9+ messages in thread

* Re: [PATCH 1/3] btrfs: Introduce struct btrfs_io_geometry
  2019-06-03  9:05 ` [PATCH 1/3] btrfs: Introduce struct btrfs_io_geometry Nikolay Borisov
@ 2019-06-05  7:35   ` Johannes Thumshirn
  2019-06-05 12:37     ` David Sterba
  0 siblings, 1 reply; 9+ messages in thread
From: Johannes Thumshirn @ 2019-06-05  7:35 UTC (permalink / raw)
  To: Nikolay Borisov; +Cc: linux-btrfs

I'd merge this patch into the next one, so you only introduce
btrfs_io_geometry when there are actual users for it.

But that's personal preference I guess.

Byte,
	Johannes
-- 
Johannes Thumshirn                            SUSE Labs Filesystems
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

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

* Re: [PATCH 2/3] btrfs: Introduce btrfs_io_geometry
  2019-06-03  9:05 ` [PATCH 2/3] btrfs: Introduce btrfs_io_geometry Nikolay Borisov
@ 2019-06-05  8:01   ` Johannes Thumshirn
  2019-06-12 16:31   ` David Sterba
  1 sibling, 0 replies; 9+ messages in thread
From: Johannes Thumshirn @ 2019-06-05  8:01 UTC (permalink / raw)
  To: Nikolay Borisov; +Cc: linux-btrfs

And maybe this patch should be merged into the next one as well, so one can
see the actual code movement between __btrfs_map_block() and
btrfs_io_geometry()

-- 
Johannes Thumshirn                            SUSE Labs Filesystems
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

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

* Re: [PATCH 1/3] btrfs: Introduce struct btrfs_io_geometry
  2019-06-05  7:35   ` Johannes Thumshirn
@ 2019-06-05 12:37     ` David Sterba
  0 siblings, 0 replies; 9+ messages in thread
From: David Sterba @ 2019-06-05 12:37 UTC (permalink / raw)
  To: Johannes Thumshirn; +Cc: Nikolay Borisov, linux-btrfs

On Wed, Jun 05, 2019 at 09:35:33AM +0200, Johannes Thumshirn wrote:
> I'd merge this patch into the next one, so you only introduce
> btrfs_io_geometry when there are actual users for it.
> 
> But that's personal preference I guess.

Yeah 1+2 is a good idea, I'll to that.

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

* Re: [PATCH 3/3] btrfs: Use btrfs_io_geometry appropriately
  2019-06-03  9:05 ` [PATCH 3/3] btrfs: Use btrfs_io_geometry appropriately Nikolay Borisov
@ 2019-06-12 16:29   ` David Sterba
  0 siblings, 0 replies; 9+ messages in thread
From: David Sterba @ 2019-06-12 16:29 UTC (permalink / raw)
  To: Nikolay Borisov; +Cc: linux-btrfs

On Mon, Jun 03, 2019 at 12:05:05PM +0300, Nikolay Borisov wrote:
> 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 using btrfs_io_geometry instead of calling
> btrfs_map_block with a special NULL value for 'bbio_ret'. No functional
> change.
> 
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
> ---
>  fs/btrfs/inode.c   | 25 +++++++--------
>  fs/btrfs/volumes.c | 77 +++++++++-------------------------------------
>  2 files changed, 27 insertions(+), 75 deletions(-)
> 
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index 80fdf6f21f74..a3abba4c2e2c 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -1932,17 +1932,19 @@ int btrfs_bio_fits_in_stripe(struct page *page, size_t size, struct bio *bio,
>  	u64 length = 0;
>  	u64 map_length;
>  	int ret;
> +	struct btrfs_io_geometry geom;

Stack bloatometer does not like it:

__btrfs_map_block                                                 +56 (200 -> 256)
btrfs_submit_direct                                               +40 (176 -> 216)
btrfs_bio_fits_in_stripe                                          +40 (40 -> 80)

OLD/NEW DELTA:     +104
PRE/POST DELTA:     +240

>  
>  	if (bio_flags & EXTENT_BIO_COMPRESSED)
>  		return 0;
>  
>  	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_io_geometry(fs_info, btrfs_op(bio), logical, map_length,
> +				&geom);
>  	if (ret < 0)
>  		return ret;
> -	if (map_length < length + size)
> +
> +	if (geom.len < length + size)

All the function needs from the geometry is one member 'len'.

>  		return 1;
>  	return 0;
>  }
> @@ -8331,15 +8333,15 @@ static int btrfs_submit_direct_hook(struct btrfs_dio_private *dip)
>  	int clone_len;
>  	int ret;
>  	blk_status_t status;
> +	struct btrfs_io_geometry geom;

And btrfs_submit_direct_hook does not seem to use the geom members at
all, which looks like the parameters are only validated in some way.

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

* Re: [PATCH 2/3] btrfs: Introduce btrfs_io_geometry
  2019-06-03  9:05 ` [PATCH 2/3] btrfs: Introduce btrfs_io_geometry Nikolay Borisov
  2019-06-05  8:01   ` Johannes Thumshirn
@ 2019-06-12 16:31   ` David Sterba
  1 sibling, 0 replies; 9+ messages in thread
From: David Sterba @ 2019-06-12 16:31 UTC (permalink / raw)
  To: Nikolay Borisov; +Cc: linux-btrfs

On Mon, Jun 03, 2019 at 12:05:04PM +0300, Nikolay Borisov wrote:
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
> ---
>  fs/btrfs/volumes.c | 98 ++++++++++++++++++++++++++++++++++++++++++++++
>  fs/btrfs/volumes.h |  2 +
>  2 files changed, 100 insertions(+)
> 
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index 776f5c7ca7c5..b130f465ca6d 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -5907,6 +5907,104 @@ static bool need_full_stripe(enum btrfs_map_op op)
>  	return (op == BTRFS_MAP_WRITE || op == BTRFS_MAP_GET_READ_MIRRORS);
>  }
>  
> +/*
> + * btrfs_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	    - The length of IO we are going to perform, starting at @logical
> + * @op      - Type of operation - Write or Read
> + * @io_geom - Pointer used to return values
> + *
> + * Returns < 0 in case a chunk for the given logical address cannot be found,
> + * usually shouldn't happen unless @logical is corrupted, 0 otherwise.
> + */
> +int btrfs_io_geometry(struct btrfs_fs_info *fs_info, enum btrfs_map_op op,
> +		      u64 logical, u64 len, struct btrfs_io_geometry *io_geom)
> +{

I think the function name lacks the descriptivity what it does, besides
that it also collides with the structure name.

> +	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",

un-indent long strings

> +			   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);
> +
> +

double newline

> +	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);

indentation

> +			}
> +		}
> +		len = min_t(u64, em->len - offset, max_len);
> +	} else {
> +		len = em->len - offset;
> +	}
> +
> +	io_geom->len = len;
> +	io_geom->offset = offset;
> +	io_geom->stripe_len = stripe_len;
> +	io_geom->stripe_nr = stripe_nr;
> +	io_geom->stripe_offset = stripe_offset;
> +	io_geom->raid56_stripe_offset = raid56_full_stripe_start;
> +
> +	return 0;
> +}
> +
>  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 7c1ddf35b7d4..f3bdf768bbab 100644
> --- a/fs/btrfs/volumes.h
> +++ b/fs/btrfs/volumes.h
> @@ -421,6 +421,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_io_geometry(struct btrfs_fs_info *fs_info, enum btrfs_map_op op,
> +		      u64 logical, u64 len, struct btrfs_io_geometry *io_geom);
>  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	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2019-06-12 16:30 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-03  9:05 [PATCH 0/3] refactoring __btrfs_map_block Nikolay Borisov
2019-06-03  9:05 ` [PATCH 1/3] btrfs: Introduce struct btrfs_io_geometry Nikolay Borisov
2019-06-05  7:35   ` Johannes Thumshirn
2019-06-05 12:37     ` David Sterba
2019-06-03  9:05 ` [PATCH 2/3] btrfs: Introduce btrfs_io_geometry Nikolay Borisov
2019-06-05  8:01   ` Johannes Thumshirn
2019-06-12 16:31   ` David Sterba
2019-06-03  9:05 ` [PATCH 3/3] btrfs: Use btrfs_io_geometry appropriately Nikolay Borisov
2019-06-12 16:29   ` David Sterba

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).