From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.71) id 1fIHea-0005kc-Ef for mharc-grub-devel@gnu.org; Mon, 14 May 2018 13:52:56 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48308) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fIHeX-0005k9-DK for grub-devel@gnu.org; Mon, 14 May 2018 13:52:54 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fIHeU-0005p7-9H for grub-devel@gnu.org; Mon, 14 May 2018 13:52:53 -0400 Received: from boksu.net-space.pl ([185.15.1.105]:53330) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_3DES_EDE_CBC_SHA1:24) (Exim 4.71) (envelope-from ) id 1fIHeT-0005mN-Sn for grub-devel@gnu.org; Mon, 14 May 2018 13:52:50 -0400 Received: (from localhost user: 'dkiper' uid#4000 fake: STDIN (dkiper@boksu.net-space.pl)) by router-fw-old.local.net-space.pl id S1839424AbeENRwq (ORCPT ); Mon, 14 May 2018 19:52:46 +0200 Date: Mon, 14 May 2018 19:52:46 +0200 From: Daniel Kiper To: The development of GNU GRUB Cc: Goffredo Baroncelli , dkiper@net-space.pl Subject: Re: [PATCH 1/8] Add support for reading a filesystem with a raid5 or raid6 profile. Message-ID: <20180514175246.GB8760@router-fw-old.local.net-space.pl> References: <20180511192446.15942-1-kreijack@inwind.it> <20180511192446.15942-2-kreijack@inwind.it> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20180511192446.15942-2-kreijack@inwind.it> User-Agent: Mutt/1.3.28i X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 185.15.1.105 X-BeenThere: grub-devel@gnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: The development of GNU GRUB List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 14 May 2018 17:52:54 -0000 On Fri, May 11, 2018 at 09:24:39PM +0200, Goffredo Baroncelli wrote: > Signed-off-by: Goffredo Baroncelli > --- > grub-core/fs/btrfs.c | 61 ++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 61 insertions(+) > > diff --git a/grub-core/fs/btrfs.c b/grub-core/fs/btrfs.c > index be195448d..7e287d0ec 100644 > --- a/grub-core/fs/btrfs.c > +++ b/grub-core/fs/btrfs.c > @@ -119,6 +119,8 @@ struct grub_btrfs_chunk_item > #define GRUB_BTRFS_CHUNK_TYPE_RAID1 0x10 > #define GRUB_BTRFS_CHUNK_TYPE_DUPLICATED 0x20 > #define GRUB_BTRFS_CHUNK_TYPE_RAID10 0x40 > +#define GRUB_BTRFS_CHUNK_TYPE_RAID5 0x80 > +#define GRUB_BTRFS_CHUNK_TYPE_RAID6 0x100 > grub_uint8_t dummy2[0xc]; > grub_uint16_t nstripes; > grub_uint16_t nsubstripes; > @@ -764,6 +766,65 @@ grub_btrfs_read_logical (struct grub_btrfs_data *data, grub_disk_addr_t addr, > stripe_offset = low + chunk_stripe_length > * high; > csize = chunk_stripe_length - low; > + break; > + } > + case GRUB_BTRFS_CHUNK_TYPE_RAID5: > + case GRUB_BTRFS_CHUNK_TYPE_RAID6: > + { > + grub_uint64_t nparities, stripe_nr, high, low; > + > + redundancy = 1; /* no redundancy for now */ > + > + if (grub_le_to_cpu64 (chunk->type) & GRUB_BTRFS_CHUNK_TYPE_RAID5) > + { > + grub_dprintf ("btrfs", "RAID5\n"); > + nparities = 1; > + } > + else > + { > + grub_dprintf ("btrfs", "RAID6\n"); > + nparities = 2; > + } > + > + /* > + * A raid 6 layout consists in several stripes spread This line is confusing if you compare it with both cases a few lines above. I think that you should explain somewhere why this math works for RAID 5 and RAID 6. > + * on the disks, following a layout like the one below > + * > + * Disk1 Disk2 Disk3 Ddisk4 > + * > + * A1 B1 P1 Q1 > + * Q2 A2 B2 P2 > + * P3 Q3 A3 B3 > + * [...] > + * > + * Note that the placement of the parities depends by the row s/by the/on/? > + * index; > + * In the code below: > + * stripe_nr -> is the stripe number not considering the parities > + * (A1=0, B1=1, A2 = 2, B2 = 3, ...) > + * high -> is the row number (0 for A1...Q1, 1 for Q2..P2, ...) > + * stripen -> is the column number (or number of disk) s/number of disk/number of disk in row/? > + * off -> logical address to read (from teh beginning of the What is "teh"? nth? And it seems to me that off is the offset from the beginning of the array. > + * chunk space) I am not sure what you mean by "chunk space" here. > + * chunk_stripe_length -> size of a stripe (typically 64k) I assume that stripe does not cover P1 and Q1 (parity disks)? > + * nstripes -> number of disks > + * > + */ > + stripe_nr = grub_divmod64 (off, chunk_stripe_length, &low); What is the low? > + high = grub_divmod64 (stripe_nr, nstripes - nparities, &stripen); > + > + /* > + * until now stripen is evaluated without the parities (0 for A1, > + * A2, A3... 1 for B1, B2...); now consider also the parities (0 > + * for A1, 1 for A2, 2 for A3....); the math is done "modulo" > + * number of disk May I ask you to use proper English sentences in the comments? Daniel