All of lore.kernel.org
 help / color / mirror / Atom feed
From: David Sterba <dsterba@suse.cz>
To: Qu Wenruo <wqu@suse.com>
Cc: linux-btrfs@vger.kernel.org, hch@lst.de, linux-raid@vger.kernel.org
Subject: Re: [PATCH 3/3] btrfs: disguise single-data-RAID56 as RAID1/RAID1C3
Date: Mon, 25 May 2026 12:12:36 +0200	[thread overview]
Message-ID: <20260525101236.GU12792@suse.cz> (raw)
In-Reply-To: <1b5c39fe-df28-4a21-bace-fc742498945e@suse.com>

On Sun, May 24, 2026 at 01:43:54PM +0930, Qu Wenruo wrote:
> I tried the idea, and it doesn't look elegant at all, we need a lot of 
> if () checks around every xor_gen()/raid6_call.
> 
> Sure the end result is not that huge (and I have only compile tested 
> it), but if we had some parameters passed wrongly, or some corner case 
> missed, it can be very hard to notice.
> 
> Just paste the adhoc fix for reference:

I ended up with something similar, but replaced the open coded ifs with
a helper so there's not that much churn at the call sites.

> diff --git a/fs/btrfs/raid56.c b/fs/btrfs/raid56.c
> index 08ee8f316d96..50fefe33bbe3 100644
> --- a/fs/btrfs/raid56.c
> +++ b/fs/btrfs/raid56.c
> @@ -1410,11 +1410,17 @@ static void generate_pq_vertical_step(struct 
> btrfs_raid_bio *rbio, unsigned int
>   				rbio_qstripe_paddr(rbio, sector_nr, step_nr));
> 
>   		assert_rbio(rbio);
> -		raid6_call.gen_syndrome(rbio->real_stripes, step, pointers);
> +		if (rbio->nr_data > 1) {
> +			raid6_call.gen_syndrome(rbio->real_stripes, step, pointers);
> +		} else {
> +			memcpy(pointers[1], pointers[0], step);
> +			memcpy(pointers[2], pointers[0], step);
> +		}
>   	} else {
>   		/* raid5 */
>   		memcpy(pointers[rbio->nr_data], pointers[0], step);
> -		xor_gen(pointers[rbio->nr_data], pointers + 1, rbio->nr_data - 1,
> +		if (rbio->nr_data > 1)
> +			xor_gen(pointers[rbio->nr_data], pointers + 1, rbio->nr_data - 1,
>   				step);
>   	}
>   	for (stripe = stripe - 1; stripe >= 0; stripe--)
> @@ -1987,11 +1993,17 @@ static void recover_vertical_step(struct 
> btrfs_raid_bio *rbio,
>   		}
> 
>   		if (failb == rbio->real_stripes - 2) {
> -			raid6_datap_recov(rbio->real_stripes, step,
> -					  faila, pointers);
> +			if (unlikely(rbio->nr_data == 1))
> +				memcpy(pointers[0], pointers[rbio->real_stripes - 1], step);
> +			else
> +				raid6_datap_recov(rbio->real_stripes, step,
> +						  faila, pointers);
>   		} else {
> -			raid6_2data_recov(rbio->real_stripes, step,
> -					  faila, failb, pointers);
> +			if (unlikely(rbio->nr_data == 1))
> +				memcpy(pointers[0], pointers[rbio->real_stripes - 2], step);

The 2data recovery needs to do 2 memcpy

> +			else
> +				raid6_2data_recov(rbio->real_stripes, step,
> +						  faila, failb, pointers);
>   		}
>   	} else {
>   		void *p;
> @@ -2002,6 +2014,9 @@ static void recover_vertical_step(struct 
> btrfs_raid_bio *rbio,
>   		/* Copy parity block into failed block to start with */
>   		memcpy(pointers[faila], pointers[rbio->nr_data], step);
> 
> +		if (rbio->nr_data == 1)
> +			goto cleanup;
> +
>   		/* Rearrange the pointer array */
>   		p = pointers[faila];
>   		for (stripe_nr = faila; stripe_nr < rbio->nr_data - 1;
> @@ -2644,11 +2659,17 @@ static bool verify_one_parity_step(struct 
> btrfs_raid_bio *rbio,
>   	if (has_qstripe) {
>   		assert_rbio(rbio);
>   		/* RAID6, call the library function to fill in our P/Q. */
> -		raid6_call.gen_syndrome(rbio->real_stripes, step, pointers);
> +		if (rbio->nr_data > 1) {
> +			raid6_call.gen_syndrome(rbio->real_stripes, step, pointers);
> +		} else {
> +			memcpy(pointers[1], pointers[0], step);
> +			memcpy(pointers[2], pointers[0], step);
> +		}
>   	} else {
>   		/* RAID5. */
>   		memcpy(pointers[nr_data], pointers[0], step);
> -		xor_gen(pointers[nr_data], pointers + 1, nr_data - 1, step);
> +		if (rbio->nr_data > 1)
> +			xor_gen(pointers[nr_data], pointers + 1, nr_data - 1, step);
>   	}
> 
>   	/* Check scrubbing parity and repair it. */
> --
> 2.54.0

  reply	other threads:[~2026-05-25 10:12 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-22  9:23 [PATCH 0/3] btrfs: disguise single-data-RAID56 as RAID1/RAID1C3 Qu Wenruo
2026-05-22  9:23 ` [PATCH 1/3] btrfs: remove btrfs_chunk_map::io_(align|width) members Qu Wenruo
2026-05-22  9:23 ` [PATCH 2/3] btrfs: remove duplicated block group type assignment Qu Wenruo
2026-05-22  9:23 ` [PATCH 3/3] btrfs: disguise single-data-RAID56 as RAID1/RAID1C3 Qu Wenruo
2026-05-23 14:23   ` David Sterba
2026-05-23 22:46     ` Qu Wenruo
2026-05-24  4:13       ` Qu Wenruo
2026-05-25 10:12         ` David Sterba [this message]
2026-05-25 10:25           ` Qu Wenruo
2026-05-25  6:00     ` Christoph Hellwig
2026-05-25 10:17       ` David Sterba
2026-05-25 10:39         ` Qu Wenruo
2026-05-26  6:46           ` Christoph Hellwig
2026-05-26  6:45         ` Christoph Hellwig

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260525101236.GU12792@suse.cz \
    --to=dsterba@suse.cz \
    --cc=hch@lst.de \
    --cc=linux-btrfs@vger.kernel.org \
    --cc=linux-raid@vger.kernel.org \
    --cc=wqu@suse.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.