Re: [PATCH 02/10] btrfs: raid56: Remove redundant check in rbio_add_io_page

Johannes Thumshirn
2020-07-02T14:13:04Z

On 02/07/2020 15:46, Nikolay Borisov wrote:
> The merging logic is always executed if the current stripe's device
> is not missing. So there's no point in duplicating the check. Simply
> remove it, while at it reduce the scope of the 'last_end' variable.
>

Maybe add "If the current stripe's device is missing we fail the stripe early on"

Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>

> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
> ---
> fs/btrfs/raid56.c | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/fs/btrfs/raid56.c b/fs/btrfs/raid56.c
> index 4efd9ed1a30e..f21bab45b7ce 100644
> --- a/fs/btrfs/raid56.c
> +++ b/fs/btrfs/raid56.c
> @@ -1083,7 +1083,6 @@ static int rbio_add_io_page(struct btrfs_raid_bio *rbio,
> unsigned long bio_max_len)
> {
> struct bio *last = bio_list->tail;
> - u64 last_end = 0;
> int ret;
> struct bio *bio;
> struct btrfs_bio_stripe *stripe;
> @@ -1098,15 +1097,14 @@ static int rbio_add_io_page(struct btrfs_raid_bio *rbio,
>
> /* see if we can add this page onto our existing bio */
> if (last) {
> - last_end = (u64)last->bi_iter.bi_sector << 9;
> + u64 last_end = (u64)last->bi_iter.bi_sector << 9;
> last_end += last->bi_iter.bi_size;
>
> /*
> * we can't merge these if they are from different
> * devices or if they are not contiguous
> */
> - if (last_end == disk_start && stripe->dev->bdev &&
> - !last->bi_status &&
> + if (last_end == disk_start && !last->bi_status &&
> last->bi_disk == stripe->dev->bdev->bd_disk &&
> last->bi_partno == stripe->dev->bdev->bd_partno) {
> ret = bio_add_page(last, page, PAGE_SIZE, 0);
>