| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Wed, 13 Dec 2023 15:36:23 +0000
From: Johannes Thumshirn <Johannes.Thumshirn@....com>
To: "hch@...radead.org" <hch@...radead.org>
CC: Chris Mason <clm@...com>, Josef Bacik <josef@...icpanda.com>,
David Sterba <dsterba@...e.com>,
"linux-btrfs@...r.kernel.org" <linux-btrfs@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 11/13] btrfs: open code set_io_stripe for RAID56
On 13.12.23 09:58, Christoph Hellwig wrote:
>
> I think raid stripe tree handling also really should move out of
> set_io_stripe. Below is the latest I have, although it probably won't
> apply to your tree:
I've decided to add that one afterwards giving the attribution to you.
There are some other patches in your tree as well, which I want to have
a look at too.
> ---
> From ac208da48d7f9d11eef8a01ac0c6fbf9681665b5 Mon Sep 17 00:00:00 2001
> From: Christoph Hellwig <hch@....de>
> Date: Thu, 22 Jun 2023 05:53:13 +0200
> Subject: btrfs: move raid-stripe-tree handling out of set_io_stripe
>
> set_io_stripe gets a little too complicated with the raid-stripe-tree
> handling. Move it out into the only callers that actually needs it.
>
> The only reads with more than a single stripe is the parity raid recovery
> case thast will need very special handling anyway once implemented.
>
> Signed-off-by: Christoph Hellwig <hch@....de>
> ---
> fs/btrfs/volumes.c | 61 ++++++++++++++++++++--------------------------
> 1 file changed, 27 insertions(+), 34 deletions(-)
>
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index 30ee5d1670d034..e32eefa242b0a4 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -6233,22 +6233,12 @@ static u64 btrfs_max_io_len(struct map_lookup *map, enum btrfs_map_op op,
> return U64_MAX;
> }
>
> -static int set_io_stripe(struct btrfs_fs_info *fs_info, enum btrfs_map_op op,
> - u64 logical, u64 *length, struct btrfs_io_stripe *dst,
> - struct map_lookup *map, u32 stripe_index,
> - u64 stripe_offset, u64 stripe_nr)
> +static void set_io_stripe(struct btrfs_io_stripe *dst, const struct map_lookup *map,
> + u32 stripe_index, u64 stripe_offset, u32 stripe_nr)
> {
> dst->dev = map->stripes[stripe_index].dev;
> -
> - if (op == BTRFS_MAP_READ &&
> - btrfs_use_raid_stripe_tree(fs_info, map->type))
> - return btrfs_get_raid_extent_offset(fs_info, logical, length,
> - map->type, stripe_index,
> - dst);
> -
> dst->physical = map->stripes[stripe_index].physical +
> stripe_offset + ((u64)stripe_nr << BTRFS_STRIPE_LEN_SHIFT);
> - return 0;
> }
>
> int btrfs_map_block(struct btrfs_fs_info *fs_info, enum btrfs_map_op op,
> @@ -6423,15 +6413,24 @@ int btrfs_map_block(struct btrfs_fs_info *fs_info, enum btrfs_map_op op,
> * physical block information on the stack instead of allocating an
> * I/O context structure.
> */
> - if (smap && num_alloc_stripes == 1 &&
> - !(btrfs_use_raid_stripe_tree(fs_info, map->type) &&
> - op != BTRFS_MAP_READ) &&
> - !((map->type & BTRFS_BLOCK_GROUP_RAID56_MASK) && mirror_num > 1)) {
> - ret = set_io_stripe(fs_info, op, logical, length, smap, map,
> - stripe_index, stripe_offset, stripe_nr);
> - *mirror_num_ret = mirror_num;
> - *bioc_ret = NULL;
> - goto out;
> + if (smap && num_alloc_stripes == 1) {
> + if (op == BTRFS_MAP_READ &&
> + btrfs_use_raid_stripe_tree(fs_info, map->type)) {
> + ret = btrfs_get_raid_extent_offset(fs_info, logical,
> + length, map->type,
> + stripe_index, smap);
> + *mirror_num_ret = mirror_num;
> + *bioc_ret = NULL;
> + goto out;
> + } else if (!(map->type & BTRFS_BLOCK_GROUP_RAID56_MASK) ||
> + mirror_num == 0) {
> + set_io_stripe(smap, map, stripe_index, stripe_offset,
> + stripe_nr);
> + *mirror_num_ret = mirror_num;
> + *bioc_ret = NULL;
> + ret = 0;
> + goto out;
> + }
> }
>
> bioc = alloc_btrfs_io_context(fs_info, logical, num_alloc_stripes);
> @@ -6448,6 +6447,8 @@ int btrfs_map_block(struct btrfs_fs_info *fs_info, enum btrfs_map_op op,
> *
> * It's still mostly the same as other profiles, just with extra rotation.
> */
> + ASSERT(op != BTRFS_MAP_READ ||
> + btrfs_use_raid_stripe_tree(fs_info, map->type));
> if (map->type & BTRFS_BLOCK_GROUP_RAID56_MASK && need_raid_map &&
> (op != BTRFS_MAP_READ || mirror_num > 1)) {
> /*
> @@ -6461,29 +6462,21 @@ int btrfs_map_block(struct btrfs_fs_info *fs_info, enum btrfs_map_op op,
> bioc->full_stripe_logical = em->start +
> ((stripe_nr * data_stripes) << BTRFS_STRIPE_LEN_SHIFT);
> for (i = 0; i < num_stripes; i++)
> - ret = set_io_stripe(fs_info, op, logical, length,
> - &bioc->stripes[i], map,
> - (i + stripe_nr) % num_stripes,
> - stripe_offset, stripe_nr);
> + set_io_stripe(&bioc->stripes[i], map,
> + (i + stripe_nr) % num_stripes,
> + stripe_offset, stripe_nr);
> } else {
> /*
> * For all other non-RAID56 profiles, just copy the target
> * stripe into the bioc.
> */
> for (i = 0; i < num_stripes; i++) {
> - ret = set_io_stripe(fs_info, op, logical, length,
> - &bioc->stripes[i], map, stripe_index,
> - stripe_offset, stripe_nr);
> + set_io_stripe(&bioc->stripes[i], map, stripe_index,
> + stripe_offset, stripe_nr);
> stripe_index++;
> }
> }
>
> - if (ret) {
> - *bioc_ret = NULL;
> - btrfs_put_bioc(bioc);
> - goto out;
> - }
> -
> if (op != BTRFS_MAP_READ)
> max_errors = btrfs_chunk_max_errors(map);
>
Powered by blists - more mailing lists