* [RESEND PATCH 1/2] btrfs: Read stripe len directly in btrfs_rmap_block
@ 2020-04-03 13:40 Nikolay Borisov
2020-04-03 13:40 ` [RESEND PATCH 2/2] btrfs: Remove dead code exclude_super_stripes Nikolay Borisov
` (3 more replies)
0 siblings, 4 replies; 6+ messages in thread
From: Nikolay Borisov @ 2020-04-03 13:40 UTC (permalink / raw)
To: linux-btrfs; +Cc: Nikolay Borisov
extent_map::orig_block_len contains the size of a physical stripe when
it's used to describe block groups (calculated in read_one_chunk via
calc_stripe_length or calculated in decide_stripe_size and then assigned to
extent_map::orig_block_len in create_chunk). Exploit this fact to get the
size directly rather than opencoding the calculations. No functional changes.
Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
Hello David,
You had some reservations for this patch but now I've expanded the changelog to
explain why it's safe to do so.
fs/btrfs/block-group.c | 13 +++----------
1 file changed, 3 insertions(+), 10 deletions(-)
diff --git a/fs/btrfs/block-group.c b/fs/btrfs/block-group.c
index 786849fcc319..d0dbaa470b88 100644
--- a/fs/btrfs/block-group.c
+++ b/fs/btrfs/block-group.c
@@ -1628,19 +1628,12 @@ int btrfs_rmap_block(struct btrfs_fs_info *fs_info, u64 chunk_start,
return -EIO;
map = em->map_lookup;
- data_stripe_length = em->len;
+ data_stripe_length = em->orig_block_len;
io_stripe_size = map->stripe_len;
- if (map->type & BTRFS_BLOCK_GROUP_RAID10)
- data_stripe_length = div_u64(data_stripe_length,
- map->num_stripes / map->sub_stripes);
- else if (map->type & BTRFS_BLOCK_GROUP_RAID0)
- data_stripe_length = div_u64(data_stripe_length, map->num_stripes);
- else if (map->type & BTRFS_BLOCK_GROUP_RAID56_MASK) {
- data_stripe_length = div_u64(data_stripe_length,
- nr_data_stripes(map));
+ /* For raid5/6 adjust to a full IO stripe length */
+ if (map->type & BTRFS_BLOCK_GROUP_RAID56_MASK)
io_stripe_size = map->stripe_len * nr_data_stripes(map);
- }
buf = kcalloc(map->num_stripes, sizeof(u64), GFP_NOFS);
if (!buf) {
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [RESEND PATCH 2/2] btrfs: Remove dead code exclude_super_stripes
2020-04-03 13:40 [RESEND PATCH 1/2] btrfs: Read stripe len directly in btrfs_rmap_block Nikolay Borisov
@ 2020-04-03 13:40 ` Nikolay Borisov
2020-04-10 19:29 ` [RESEND PATCH 1/2] btrfs: Read stripe len directly in btrfs_rmap_block Josef Bacik
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Nikolay Borisov @ 2020-04-03 13:40 UTC (permalink / raw)
To: linux-btrfs; +Cc: Nikolay Borisov
Adresses held in 'logical' array are always guaranteed to fall within
the boundaries of the block group. That is, 'start' can never be
smaller than cache->start. This invariant follows from the way the
address are calculated in btrfs_rmap_block:
stripe_nr = physical - map->stripes[i].physical;
stripe_nr = div64_u64(stripe_nr, map->stripe_len);
bytenr = chunk_start + stripe_nr * io_stripe_size;
I.e it's always some IO stripe within the given chunk.
Exploit this invariant to simplify the body of the loop by removing the
unnecessary 'if' since its 'else' part is the one always executed.
Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
fs/btrfs/block-group.c | 21 ++++-----------------
1 file changed, 4 insertions(+), 17 deletions(-)
diff --git a/fs/btrfs/block-group.c b/fs/btrfs/block-group.c
index d0dbaa470b88..7b96ded820f9 100644
--- a/fs/btrfs/block-group.c
+++ b/fs/btrfs/block-group.c
@@ -1712,25 +1712,12 @@ static int exclude_super_stripes(struct btrfs_block_group *cache)
return ret;
while (nr--) {
- u64 start, len;
-
- if (logical[nr] > cache->start + cache->length)
- continue;
-
- if (logical[nr] + stripe_len <= cache->start)
- continue;
-
- start = logical[nr];
- if (start < cache->start) {
- start = cache->start;
- len = (logical[nr] + stripe_len) - start;
- } else {
- len = min_t(u64, stripe_len,
- cache->start + cache->length - start);
- }
+ u64 len = min_t(u64, stripe_len,
+ cache->start + cache->length - logical[nr]);
cache->bytes_super += len;
- ret = btrfs_add_excluded_extent(fs_info, start, len);
+ ret = btrfs_add_excluded_extent(fs_info, logical[nr],
+ len);
if (ret) {
kfree(logical);
return ret;
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [RESEND PATCH 1/2] btrfs: Read stripe len directly in btrfs_rmap_block
2020-04-03 13:40 [RESEND PATCH 1/2] btrfs: Read stripe len directly in btrfs_rmap_block Nikolay Borisov
2020-04-03 13:40 ` [RESEND PATCH 2/2] btrfs: Remove dead code exclude_super_stripes Nikolay Borisov
@ 2020-04-10 19:29 ` Josef Bacik
2020-04-10 20:16 ` Nikolay Borisov
2020-05-28 8:12 ` Nikolay Borisov
2020-05-29 12:37 ` David Sterba
3 siblings, 1 reply; 6+ messages in thread
From: Josef Bacik @ 2020-04-10 19:29 UTC (permalink / raw)
To: Nikolay Borisov, linux-btrfs
On 4/3/20 9:40 AM, Nikolay Borisov wrote:
> extent_map::orig_block_len contains the size of a physical stripe when
> it's used to describe block groups (calculated in read_one_chunk via
> calc_stripe_length or calculated in decide_stripe_size and then assigned to
> extent_map::orig_block_len in create_chunk). Exploit this fact to get the
> size directly rather than opencoding the calculations. No functional changes.
>
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
> ---
>
> Hello David,
>
> You had some reservations for this patch but now I've expanded the changelog to
> explain why it's safe to do so.
>
>
> fs/btrfs/block-group.c | 13 +++----------
> 1 file changed, 3 insertions(+), 10 deletions(-)
>
> diff --git a/fs/btrfs/block-group.c b/fs/btrfs/block-group.c
> index 786849fcc319..d0dbaa470b88 100644
> --- a/fs/btrfs/block-group.c
> +++ b/fs/btrfs/block-group.c
> @@ -1628,19 +1628,12 @@ int btrfs_rmap_block(struct btrfs_fs_info *fs_info, u64 chunk_start,
> return -EIO;
>
> map = em->map_lookup;
> - data_stripe_length = em->len;
> + data_stripe_length = em->orig_block_len;
> io_stripe_size = map->stripe_len;
>
> - if (map->type & BTRFS_BLOCK_GROUP_RAID10)
> - data_stripe_length = div_u64(data_stripe_length,
> - map->num_stripes / map->sub_stripes);
> - else if (map->type & BTRFS_BLOCK_GROUP_RAID0)
> - data_stripe_length = div_u64(data_stripe_length, map->num_stripes);
> - else if (map->type & BTRFS_BLOCK_GROUP_RAID56_MASK) {
> - data_stripe_length = div_u64(data_stripe_length,
> - nr_data_stripes(map));
> + /* For raid5/6 adjust to a full IO stripe length */
> + if (map->type & BTRFS_BLOCK_GROUP_RAID56_MASK)
> io_stripe_size = map->stripe_len * nr_data_stripes(map);
> - }
>
So now data_stripe_length is different in the RAID1 case and the RAID1C* case,
right? Is that ok? I *think* it is, but I'm a little drunk and can't really
reason it out well. Thanks,
Josef
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RESEND PATCH 1/2] btrfs: Read stripe len directly in btrfs_rmap_block
2020-04-10 19:29 ` [RESEND PATCH 1/2] btrfs: Read stripe len directly in btrfs_rmap_block Josef Bacik
@ 2020-04-10 20:16 ` Nikolay Borisov
0 siblings, 0 replies; 6+ messages in thread
From: Nikolay Borisov @ 2020-04-10 20:16 UTC (permalink / raw)
To: Josef Bacik, linux-btrfs
On 10.04.20 г. 22:29 ч., Josef Bacik wrote:
> On 4/3/20 9:40 AM, Nikolay Borisov wrote:
>> extent_map::orig_block_len contains the size of a physical stripe when
>> it's used to describe block groups (calculated in read_one_chunk via
>> calc_stripe_length or calculated in decide_stripe_size and then
>> assigned to
>> extent_map::orig_block_len in create_chunk). Exploit this fact to get the
>> size directly rather than opencoding the calculations. No functional
>> changes.
>>
>> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
>> ---
>>
>> Hello David,
>>
>> You had some reservations for this patch but now I've expanded the
>> changelog to
>> explain why it's safe to do so.
>>
>>
>> fs/btrfs/block-group.c | 13 +++----------
>> 1 file changed, 3 insertions(+), 10 deletions(-)
>>
>> diff --git a/fs/btrfs/block-group.c b/fs/btrfs/block-group.c
>> index 786849fcc319..d0dbaa470b88 100644
>> --- a/fs/btrfs/block-group.c
>> +++ b/fs/btrfs/block-group.c
>> @@ -1628,19 +1628,12 @@ int btrfs_rmap_block(struct btrfs_fs_info
>> *fs_info, u64 chunk_start,
>> return -EIO;
>>
>> map = em->map_lookup;
>> - data_stripe_length = em->len;
>> + data_stripe_length = em->orig_block_len;
>> io_stripe_size = map->stripe_len;
>>
>> - if (map->type & BTRFS_BLOCK_GROUP_RAID10)
>> - data_stripe_length = div_u64(data_stripe_length,
>> - map->num_stripes / map->sub_stripes);
>> - else if (map->type & BTRFS_BLOCK_GROUP_RAID0)
>> - data_stripe_length = div_u64(data_stripe_length,
>> map->num_stripes);
>> - else if (map->type & BTRFS_BLOCK_GROUP_RAID56_MASK) {
>> - data_stripe_length = div_u64(data_stripe_length,
>> - nr_data_stripes(map));
>> + /* For raid5/6 adjust to a full IO stripe length */
>> + if (map->type & BTRFS_BLOCK_GROUP_RAID56_MASK)
>> io_stripe_size = map->stripe_len * nr_data_stripes(map);
>> - }
>>
>
> So now data_stripe_length is different in the RAID1 case and the RAID1C*
> case, right? Is that ok? I *think* it is, but I'm a little drunk and
> can't really reason it out well. Thanks,
The stripe for RAID1C* is calculated in decide_stripe_size_regular based
on the data in btrfs_raid_array. This patch only gets whatever was
calculated at chunk allocation time.
>
> Josef
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RESEND PATCH 1/2] btrfs: Read stripe len directly in btrfs_rmap_block
2020-04-03 13:40 [RESEND PATCH 1/2] btrfs: Read stripe len directly in btrfs_rmap_block Nikolay Borisov
2020-04-03 13:40 ` [RESEND PATCH 2/2] btrfs: Remove dead code exclude_super_stripes Nikolay Borisov
2020-04-10 19:29 ` [RESEND PATCH 1/2] btrfs: Read stripe len directly in btrfs_rmap_block Josef Bacik
@ 2020-05-28 8:12 ` Nikolay Borisov
2020-05-29 12:37 ` David Sterba
3 siblings, 0 replies; 6+ messages in thread
From: Nikolay Borisov @ 2020-05-28 8:12 UTC (permalink / raw)
To: David Sterba; +Cc: linux-btrfs
On 3.04.20 г. 16:40 ч., Nikolay Borisov wrote:
> extent_map::orig_block_len contains the size of a physical stripe when
> it's used to describe block groups (calculated in read_one_chunk via
> calc_stripe_length or calculated in decide_stripe_size and then assigned to
> extent_map::orig_block_len in create_chunk). Exploit this fact to get the
> size directly rather than opencoding the calculations. No functional changes.
>
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
> ---
>
> Hello David,
>
> You had some reservations for this patch but now I've expanded the changelog to
> explain why it's safe to do so.
>
Ping
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RESEND PATCH 1/2] btrfs: Read stripe len directly in btrfs_rmap_block
2020-04-03 13:40 [RESEND PATCH 1/2] btrfs: Read stripe len directly in btrfs_rmap_block Nikolay Borisov
` (2 preceding siblings ...)
2020-05-28 8:12 ` Nikolay Borisov
@ 2020-05-29 12:37 ` David Sterba
3 siblings, 0 replies; 6+ messages in thread
From: David Sterba @ 2020-05-29 12:37 UTC (permalink / raw)
To: Nikolay Borisov; +Cc: linux-btrfs
On Fri, Apr 03, 2020 at 04:40:34PM +0300, Nikolay Borisov wrote:
> extent_map::orig_block_len contains the size of a physical stripe when
> it's used to describe block groups (calculated in read_one_chunk via
> calc_stripe_length or calculated in decide_stripe_size and then assigned to
> extent_map::orig_block_len in create_chunk). Exploit this fact to get the
> size directly rather than opencoding the calculations. No functional changes.
>
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
> ---
>
> Hello David,
>
> You had some reservations for this patch but now I've expanded the changelog to
> explain why it's safe to do so.
I've applied the patches to misc-next, if something goes wrong we'll
now. The patches look correct as long as I followed the changelog, but
it's not code I know by heart so if there's something missing I'm
relying on tests to catch it.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-05-29 12:37 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-03 13:40 [RESEND PATCH 1/2] btrfs: Read stripe len directly in btrfs_rmap_block Nikolay Borisov
2020-04-03 13:40 ` [RESEND PATCH 2/2] btrfs: Remove dead code exclude_super_stripes Nikolay Borisov
2020-04-10 19:29 ` [RESEND PATCH 1/2] btrfs: Read stripe len directly in btrfs_rmap_block Josef Bacik
2020-04-10 20:16 ` Nikolay Borisov
2020-05-28 8:12 ` Nikolay Borisov
2020-05-29 12:37 ` 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).