[PATCH 1/9] btrfs: Add support for reading a filesystem with a RAID 5 or RAID 6 profile.

[Goffredo Baroncelli]

From: Goffredo Baroncelli 

Signed-off-by: Goffredo Baroncelli 
Signed-off-by: Daniel Kiper 
Reviewed-by: Daniel Kiper 
---
 grub-core/fs/btrfs.c | 73 
 1 file changed, 73 insertions(+)

diff --git a/grub-core/fs/btrfs.c b/grub-core/fs/btrfs.c
index be195448d..9122169aa 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_DUPLICATED0x20
 #define GRUB_BTRFS_CHUNK_TYPE_RAID100x40
+#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,77 @@ 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;
+   }
+
+ /*
+  * RAID 6 layout consists of several stripes spread over
+  * the disks, e.g.:
+  *
+  *   Disk_0  Disk_1  Disk_2  Disk_3
+  * A0  B0  P0  Q0
+  * Q1  A1  B1  P1
+  * P2  Q2  A2  B2
+  *
+  * Note: placement of the parities depend on row number.
+  *
+  * Pay attention that the btrfs terminology may differ from
+  * terminology used in other RAID implementations, e.g. LVM,
+  * dm or md. The main difference is that btrfs calls contiguous
+  * block of data on a given disk, e.g. A0, stripe instead of 
chunk.
+  *
+  * The variables listed below have following meaning:
+  *   - stripe_nr is the stripe number excluding the parities
+  * (A0 = 0, B0 = 1, A1 = 2, B1 = 3, etc.),
+  *   - high is the row number (0 for A0...Q0, 1 for Q1...P1, 
etc.),
+  *   - stripen is the disk number in a row (0 for A0, Q1, P2,
+  * 1 for B0, A1, Q2, etc.),
+  *   - off is the logical address to read,
+  *   - chunk_stripe_length is the size of a stripe (typically 64 
KiB),
+  *   - nstripes is the number of disks in a row,
+  *   - low is the offset of the data inside a stripe,
+  *   - stripe_offset is the data offset in an array,
+  *   - csize is the "potential" data to read; it will be reduced
+  * to size if the latter is smaller,
+  *   - nparities is the number of parities (1 for RAID 5, 2 for
+  * RAID 6); used only in RAID 5/6 code.
+  */
+ stripe_nr = grub_divmod64 (off, chunk_stripe_length, );
+
+ /*
+  * stripen is computed without the parities
+  * (0 for A0, A1, A2, 1 for B0, B1, B2, etc.).
+  */
+ high = grub_divmod64 (stripe_nr, nstripes - nparities, );
+
+ /*
+  * The stripes are spread over the disks. Every each row their
+  * positions are shifted by 1 place. So, the real disks number
+  * change. Hence, we have to take into account current row number
+  * modulo nstripes (0 for A0, 1 for A1, 2 for A2, etc.).
+  */
+ grub_divmod64 (high + stripen, nstripes, );
+
+ stripe_offset = low + chunk_stripe_length * high;
+ csize = chunk_stripe_length - low;
+
  break;
}
  default:
-- 
2.19.1


___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

[PATCH 1/9] btrfs: Add support for reading a filesystem with a RAID 5 or RAID 6 profile.

[Goffredo Baroncelli]

From: Goffredo Baroncelli 

Signed-off-by: Goffredo Baroncelli 
Signed-off-by: Daniel Kiper 
Reviewed-by: Daniel Kiper 
---
 grub-core/fs/btrfs.c | 73 
 1 file changed, 73 insertions(+)

diff --git a/grub-core/fs/btrfs.c b/grub-core/fs/btrfs.c
index be195448d..9122169aa 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_DUPLICATED0x20
 #define GRUB_BTRFS_CHUNK_TYPE_RAID100x40
+#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,77 @@ 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;
+   }
+
+ /*
+  * RAID 6 layout consists of several stripes spread over
+  * the disks, e.g.:
+  *
+  *   Disk_0  Disk_1  Disk_2  Disk_3
+  * A0  B0  P0  Q0
+  * Q1  A1  B1  P1
+  * P2  Q2  A2  B2
+  *
+  * Note: placement of the parities depend on row number.
+  *
+  * Pay attention that the btrfs terminology may differ from
+  * terminology used in other RAID implementations, e.g. LVM,
+  * dm or md. The main difference is that btrfs calls contiguous
+  * block of data on a given disk, e.g. A0, stripe instead of 
chunk.
+  *
+  * The variables listed below have following meaning:
+  *   - stripe_nr is the stripe number excluding the parities
+  * (A0 = 0, B0 = 1, A1 = 2, B1 = 3, etc.),
+  *   - high is the row number (0 for A0...Q0, 1 for Q1...P1, 
etc.),
+  *   - stripen is the disk number in a row (0 for A0, Q1, P2,
+  * 1 for B0, A1, Q2, etc.),
+  *   - off is the logical address to read,
+  *   - chunk_stripe_length is the size of a stripe (typically 64 
KiB),
+  *   - nstripes is the number of disks in a row,
+  *   - low is the offset of the data inside a stripe,
+  *   - stripe_offset is the data offset in an array,
+  *   - csize is the "potential" data to read; it will be reduced
+  * to size if the latter is smaller,
+  *   - nparities is the number of parities (1 for RAID 5, 2 for
+  * RAID 6); used only in RAID 5/6 code.
+  */
+ stripe_nr = grub_divmod64 (off, chunk_stripe_length, );
+
+ /*
+  * stripen is computed without the parities
+  * (0 for A0, A1, A2, 1 for B0, B1, B2, etc.).
+  */
+ high = grub_divmod64 (stripe_nr, nstripes - nparities, );
+
+ /*
+  * The stripes are spread over the disks. Every each row their
+  * positions are shifted by 1 place. So, the real disks number
+  * change. Hence, we have to take into account current row number
+  * modulo nstripes (0 for A0, 1 for A1, 2 for A2, etc.).
+  */
+ grub_divmod64 (high + stripen, nstripes, );
+
+ stripe_offset = low + chunk_stripe_length * high;
+ csize = chunk_stripe_length - low;
+
  break;
}
  default:
-- 
2.19.1


___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH 1/9] btrfs: Add support for reading a filesystem with a RAID 5 or RAID 6 profile.

[Daniel Kiper]

On Thu, Oct 11, 2018 at 08:50:55PM +0200, Goffredo Baroncelli wrote:
> From: Goffredo Baroncelli 
>
> Signed-off-by: Goffredo Baroncelli 
> Signed-off-by: Daniel Kiper 

One nit pick below...

Otherwise you can add Reviewed-by: Daniel Kiper 

> ---
>  grub-core/fs/btrfs.c | 73 
>  1 file changed, 73 insertions(+)
>
> diff --git a/grub-core/fs/btrfs.c b/grub-core/fs/btrfs.c
> index be195448d..933a57d3b 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_DUPLICATED0x20
>  #define GRUB_BTRFS_CHUNK_TYPE_RAID100x40
> +#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,77 @@ 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;
> + }
> +
> +   /*
> +* RAID 6 layout consists of several stripes spread over
> +* the disks, e.g.:
> +*
> +*   Disk_0  Disk_1  Disk_2  Disk_3
> +* A0  B0  P0  Q0
> +* Q1  A1  B1  P1
> +* P2  Q2  A2  B2
> +*
> +* Note: placement of the parities depend on row number.
> +*
> +* Pay attention that the btrfs terminology may differ from
> +* terminology used in other RAID implementations, e.g. LVM,
> +* dm or md. The main difference is that btrfs calls contiguous
> +* block of data on a given disk, e.g. A0, stripe instead of 
> chunk.
> +*
> +* The variables listed below have following meaning:
> +*   - stripe_nr is the stripe number excluding the parities
> +* (A0 = 0, B0 = 1, A1 = 2, B1 = 3, etc.),
> +*   - high is the row number (0 for A0...Q0, 1 for Q1...P1, 
> etc.),
> +*   - stripen is the disk number in a row (0 for A0, Q1, P2,
> +* 1 for B0, A1, Q2, etc.),
> +*   - off is the logical address to read,
> +*   - chunk_stripe_length is the size of a stripe (typically 64 
> KiB),
> +*   - nstripes is the number of disks in a row,
> +*   - low is the offset of the data inside a stripe,
> +*   - stripe_offset is the data offset in an array,
> +*   - csize is the "potential" data to read; it will be reduced
> +* to size if the latter is smaller,
> +*   - nparities is the number of parities (1 for RAID 5, 2 for
> +* RAID 6); used only in RAID 5/6 code.
> +*/
> +   stripe_nr = grub_divmod64 (off, chunk_stripe_length, );
> +
> +   /*
> +* stripen is computed without the parities
> +* (0 for A0, A1, A2, 1 for B0, B1, B2, etc.).
> +*/
> +   high = grub_divmod64 (stripe_nr, nstripes - nparities, );
> +
> +   /*
> +* The stripes are spread over the disks. Every each row their
> +* positions are shifted by 1 place. So, the real disks number
> +* change. Hence, we have to take current row number modulo
> +* nstripes into account (0 for A0, 1 for A1, 2 for A2, etc.).

s/current row number modulo nstripes into account/into account current row 
number modulo nstripes/

Daniel

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

[PATCH 1/9] btrfs: Add support for reading a filesystem with a RAID 5 or RAID 6 profile.

[Goffredo Baroncelli]

From: Goffredo Baroncelli 

Signed-off-by: Goffredo Baroncelli 
Signed-off-by: Daniel Kiper 
---
 grub-core/fs/btrfs.c | 73 
 1 file changed, 73 insertions(+)

diff --git a/grub-core/fs/btrfs.c b/grub-core/fs/btrfs.c
index be195448d..933a57d3b 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_DUPLICATED0x20
 #define GRUB_BTRFS_CHUNK_TYPE_RAID100x40
+#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,77 @@ 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;
+   }
+
+ /*
+  * RAID 6 layout consists of several stripes spread over
+  * the disks, e.g.:
+  *
+  *   Disk_0  Disk_1  Disk_2  Disk_3
+  * A0  B0  P0  Q0
+  * Q1  A1  B1  P1
+  * P2  Q2  A2  B2
+  *
+  * Note: placement of the parities depend on row number.
+  *
+  * Pay attention that the btrfs terminology may differ from
+  * terminology used in other RAID implementations, e.g. LVM,
+  * dm or md. The main difference is that btrfs calls contiguous
+  * block of data on a given disk, e.g. A0, stripe instead of 
chunk.
+  *
+  * The variables listed below have following meaning:
+  *   - stripe_nr is the stripe number excluding the parities
+  * (A0 = 0, B0 = 1, A1 = 2, B1 = 3, etc.),
+  *   - high is the row number (0 for A0...Q0, 1 for Q1...P1, 
etc.),
+  *   - stripen is the disk number in a row (0 for A0, Q1, P2,
+  * 1 for B0, A1, Q2, etc.),
+  *   - off is the logical address to read,
+  *   - chunk_stripe_length is the size of a stripe (typically 64 
KiB),
+  *   - nstripes is the number of disks in a row,
+  *   - low is the offset of the data inside a stripe,
+  *   - stripe_offset is the data offset in an array,
+  *   - csize is the "potential" data to read; it will be reduced
+  * to size if the latter is smaller,
+  *   - nparities is the number of parities (1 for RAID 5, 2 for
+  * RAID 6); used only in RAID 5/6 code.
+  */
+ stripe_nr = grub_divmod64 (off, chunk_stripe_length, );
+
+ /*
+  * stripen is computed without the parities
+  * (0 for A0, A1, A2, 1 for B0, B1, B2, etc.).
+  */
+ high = grub_divmod64 (stripe_nr, nstripes - nparities, );
+
+ /*
+  * The stripes are spread over the disks. Every each row their
+  * positions are shifted by 1 place. So, the real disks number
+  * change. Hence, we have to take current row number modulo
+  * nstripes into account (0 for A0, 1 for A1, 2 for A2, etc.).
+  */
+ grub_divmod64 (high + stripen, nstripes, );
+
+ stripe_offset = low + chunk_stripe_length * high;
+ csize = chunk_stripe_length - low;
+
  break;
}
  default:
-- 
2.19.1


___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH 1/9] btrfs: Add support for reading a filesystem with a RAID 5 or RAID 6 profile.

[Daniel Kiper]

On Tue, Oct 09, 2018 at 07:51:01PM +0200, Daniel Kiper wrote:
> On Thu, Sep 27, 2018 at 08:34:56PM +0200, Goffredo Baroncelli wrote:
> > From: Goffredo Baroncelli 
> >
> > Signed-off-by: Goffredo Baroncelli 
> 
> Code LGTM. Though comment begs improvement. I will send you updated
> comment for approval shortly.

Below you can find updated patch. Please check I have not messed up something.

Daniel

>From ecefb12a10d39bdd09e1d2b8fbbcbdb1b35274f8 Mon Sep 17 00:00:00 2001
From: Goffredo Baroncelli 
Date: Thu, 27 Sep 2018 20:34:56 +0200
Subject: [PATCH 1/1] btrfs: Add support for reading a filesystem with a RAID
 5 or RAID 6 profile.

Signed-off-by: Goffredo Baroncelli 
Signed-off-by: Daniel Kiper 
---
 grub-core/fs/btrfs.c |   73 ++
 1 file changed, 73 insertions(+)

diff --git a/grub-core/fs/btrfs.c b/grub-core/fs/btrfs.c
index be19544..933a57d 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_DUPLICATED0x20
 #define GRUB_BTRFS_CHUNK_TYPE_RAID100x40
+#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;
@@ -766,6 +768,77 @@ grub_btrfs_read_logical (struct grub_btrfs_data *data, 
grub_disk_addr_t addr,
  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;
+   }
+
+ /*
+  * RAID 6 layout consists of several stripes spread over
+  * the disks, e.g.:
+  *
+  *   Disk_0  Disk_1  Disk_2  Disk_3
+  * A0  B0  P0  Q0
+  * Q1  A1  B1  P1
+  * P2  Q2  A2  B2
+  *
+  * Note: placement of the parities depend on row number.
+  *
+  * Pay attention that the btrfs terminology may differ from
+  * terminology used in other RAID implementations, e.g. LVM,
+  * dm or md. The main difference is that btrfs calls contiguous
+  * block of data on a given disk, e.g. A0, stripe instead of 
chunk.
+  *
+  * The variables listed below have following meaning:
+  *   - stripe_nr is the stripe number excluding the parities
+  * (A0 = 0, B0 = 1, A1 = 2, B1 = 3, etc.),
+  *   - high is the row number (0 for A0...Q0, 1 for Q1...P1, 
etc.),
+  *   - stripen is the disk number in a row (0 for A0, Q1, P2,
+  * 1 for B0, A1, Q2, etc.),
+  *   - off is the logical address to read,
+  *   - chunk_stripe_length is the size of a stripe (typically 64 
KiB),
+  *   - nstripes is the number of disks in a row,
+  *   - low is the offset of the data inside a stripe,
+  *   - stripe_offset is the data offset in an array,
+  *   - csize is the "potential" data to read; it will be reduced
+  * to size if the latter is smaller,
+  *   - nparities is the number of parities (1 for RAID 5, 2 for
+  * RAID 6); used only in RAID 5/6 code.
+  */
+ stripe_nr = grub_divmod64 (off, chunk_stripe_length, );
+
+ /*
+  * stripen is computed without the parities
+  * (0 for A0, A1, A2, 1 for B0, B1, B2, etc.).
+  */
+ high = grub_divmod64 (stripe_nr, nstripes - nparities, );
+
+ /*
+  * The stripes are spread over the disks. Every each row their
+  * positions are shifted by 1 place. So, the real disks number
+  * change. Hence, we have to take current row number modulo
+  * nstripes into account (0 for A0, 1 for A1, 2 for A2, etc.).
+  */
+ grub_divmod64 (high + stripen, nstripes, );
+
+ stripe_offset = low + chunk_stripe_length * high;
+ csize = chunk_stripe_length - low;
+
+ break;
+   }
  default:
grub_dprintf ("btrfs", "unsupported RAID\n");
return grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET,
-- 
1.7.10.4

___
Grub-devel mailing list

Re: [PATCH 1/9] btrfs: Add support for reading a filesystem with a RAID 5 or RAID 6 profile.

[Daniel Kiper]

On Thu, Sep 27, 2018 at 08:34:56PM +0200, Goffredo Baroncelli wrote:
> From: Goffredo Baroncelli 
>
> Signed-off-by: Goffredo Baroncelli 

Code LGTM. Though comment begs improvement. I will send you updated
comment for approval shortly.

Daniel

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

[PATCH 1/9] btrfs: Add support for reading a filesystem with a RAID 5 or RAID 6 profile.

[Goffredo Baroncelli]

From: Goffredo Baroncelli 

Signed-off-by: Goffredo Baroncelli 
---
 grub-core/fs/btrfs.c | 74 
 1 file changed, 74 insertions(+)

diff --git a/grub-core/fs/btrfs.c b/grub-core/fs/btrfs.c
index be195448d..9bc6d399d 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_DUPLICATED0x20
 #define GRUB_BTRFS_CHUNK_TYPE_RAID100x40
+#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,78 @@ 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 of several stripes spread on the
+  * disks, following a layout like the one below
+  *
+  *   Disk0  Disk1  Disk2  Disk3
+  *
+  *A1 B1 P1 Q1
+  *Q2 A2 B2 P2
+  *P3 Q3 A3 B3
+  *  [...]
+  *
+  *  Note that the placement of the parities depends on row index.
+  *  Pay attention that the BTRFS terminolgy may be different
+  *  from others RAID implementation (e.g. lvm/dm or md). In BTRFS
+  *  a contiguous block of data of a disk (like A1) is called
+  *  stripe.
+  *  In the code below:
+  *  - stripe_nr is the stripe number without 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 disk number in a row (0 for A1,Q2,P3, 1 for
+  *B1...),
+  *  - off is the logical address to read,
+  *  - chunk_stripe_length is the size of a stripe (typically 64k),
+  *  - nstripes is the number of disks of a row
+  *  - low is the offset of the data inside a stripe,
+  *  - stripe_offset is the data offset in an array,
+  *  - csize is the "potential" data to read. It will be reduced to
+  *size if the latter is smaller.
+  *  - nparities is the number of parities (1 for RAID5, 2 for
+  *RAID6); used only in RAID5/6 code.
+  */
+ stripe_nr = grub_divmod64 (off, chunk_stripe_length, );
+
+ /*
+  * stripen is computed without the parities (0 for A1, A2, A3...
+  * 1 for B1, B2...).
+  */
+ high = grub_divmod64 (stripe_nr, nstripes - nparities, );
+
+ /*
+  * the stripes are spread across the disks, each row their
+  * position is shifted by 1 place. So to compute the real disk
+  * number occuped by a stripe, we need to sum also the
+  * "row number" in modulo nstripes (0 for A1, 1 for A2, 2 for
+  * A3).
+  */
+ grub_divmod64 (high + stripen, nstripes, );
+
+ stripe_offset = low + chunk_stripe_length * high;
+ csize = chunk_stripe_length - low;
+
  break;
}
  default:
-- 
2.19.0


___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH 1/9] btrfs: Add support for reading a filesystem with a RAID 5 or RAID 6 profile.

[Daniel Kiper]

On Wed, Sep 26, 2018 at 10:40:32PM +0200, Goffredo Baroncelli wrote:
> On 25/09/2018 17.31, Daniel Kiper wrote:
> > On Wed, Sep 19, 2018 at 08:40:32PM +0200, Goffredo Baroncelli wrote:
> >> From: Goffredo Baroncelli 
> >>
> >> Signed-off-by: Goffredo Baroncelli 
> >> ---
> >>  grub-core/fs/btrfs.c | 66 
> >>  1 file changed, 66 insertions(+)
> >>
> >> diff --git a/grub-core/fs/btrfs.c b/grub-core/fs/btrfs.c
> >> index be195448d..56c42746d 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_DUPLICATED0x20
> >>  #define GRUB_BTRFS_CHUNK_TYPE_RAID100x40
> >> +#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,70 @@ 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, block_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 of several blocks spread on the disks.
> >> + * The raid terminology is used to call all the blocks of a row
> >> + * "stripe". Unfortunately the BTRFS terminology confuses block
> >
> > Stripe is data set or parity (parity stripe) on one disk. Block has
> > different meaning. Please stick to btrfs terminology and say it clearly
> > in the comment. And even add a link to btrfs wiki page to ease reading.
> >
> > I think about this one:
> >   
> > https://btrfs.wiki.kernel.org/index.php/Manpage/mkfs.btrfs#BLOCK_GROUPS.2C_CHUNKS.2C_RAID
> >
> >> + * and stripe.
> >
> > I do not think so. Or at least not so much...
>
> Trust me, generally speaking stripe is the "row" in the disks (without the 
> parity); looking at the ext3 man page:
>
> 
>stride=stride-size
>   Configure  the  filesystem  for  a  RAID  array with
>   stride-size filesystem blocks. This is the number of
>   blocks  read or written to disk before moving to the
>   next disk, which is sometimes  referred  to  as  the
>   chunk   size.   This  mostly  affects  placement  of
>   filesystem metadata like bitmaps at mke2fs  time  to
>   avoid  placing them on a single disk, which can hurt
>   performance.  It may also be used by the block 
> allo???
>   cator.
>
>stripe_width=stripe-width
>   Configure  the  filesystem  for  a  RAID  array with
>   stripe-width filesystem blocks per stripe.  This  is
>   typically  stride-size * N, where N is the number of
>   data-bearing disks in the  RAID  (e.g.  for  RAID  5
>   there is one parity disk, so N will be the number of
>   disks in the array minus 1).  This allows the  block
>   allocator to prevent read-modify-write of the parity
>   in a RAID stripe if possible when the data is  
> writ???
>   ten.
>
> 
> Looking at the RAID5 wikipedia page, it seems that the term "stripe"
> is coherent with the ext3 man page.

Ugh... It looks that I have messed up things. Sorry about that.

> I suspect that the confusion is due to the fact that in RAID1 a stripe
> is in ONE disk (because the others are like parities). In BTRFS the
> RAID5/6 code uses the structure of RAID1 with some minimal
> extensions...
>
> To be clear, I don't have problem to be adherent to the BTRFS
> terminology. However I found this very confusing because I was used to
> a different terminology. I am bit worried about the fact that grub

Yeah, I have the same feeling. However, I think that in btrfs code we
should stay with btrfs terms. Though I think that it make sense to
underline differences between btrfs and well known RAID here.

> uses both MD/DM code and BTRFS code; a 

Re: [PATCH 1/9] btrfs: Add support for reading a filesystem with a RAID 5 or RAID 6 profile.

[Goffredo Baroncelli]

On 25/09/2018 17.31, Daniel Kiper wrote:
> On Wed, Sep 19, 2018 at 08:40:32PM +0200, Goffredo Baroncelli wrote:
>> From: Goffredo Baroncelli 
>>
>> Signed-off-by: Goffredo Baroncelli 
>> ---
>>  grub-core/fs/btrfs.c | 66 
>>  1 file changed, 66 insertions(+)
>>
>> diff --git a/grub-core/fs/btrfs.c b/grub-core/fs/btrfs.c
>> index be195448d..56c42746d 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_DUPLICATED0x20
>>  #define GRUB_BTRFS_CHUNK_TYPE_RAID100x40
>> +#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,70 @@ 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, block_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 of several blocks spread on the disks.
>> +   * The raid terminology is used to call all the blocks of a row
>> +   * "stripe". Unfortunately the BTRFS terminology confuses block
> 
> Stripe is data set or parity (parity stripe) on one disk. Block has
> different meaning. Please stick to btrfs terminology and say it clearly
> in the comment. And even add a link to btrfs wiki page to ease reading.
> 
> I think about this one:
>   
> https://btrfs.wiki.kernel.org/index.php/Manpage/mkfs.btrfs#BLOCK_GROUPS.2C_CHUNKS.2C_RAID
> 
>> +   * and stripe.
> 
> I do not think so. Or at least not so much...

Trust me, generally speaking stripe is the "row" in the disks (without the 
parity); looking at the ext3 man page:


   stride=stride-size
  Configure  the  filesystem  for  a  RAID  array with
  stride-size filesystem blocks. This is the number of
  blocks  read or written to disk before moving to the
  next disk, which is sometimes  referred  to  as  the
  chunk   size.   This  mostly  affects  placement  of
  filesystem metadata like bitmaps at mke2fs  time  to
  avoid  placing them on a single disk, which can hurt
  performance.  It may also be used by the block allo‐
  cator.

   stripe_width=stripe-width
  Configure  the  filesystem  for  a  RAID  array with
  stripe-width filesystem blocks per stripe.  This  is
  typically  stride-size * N, where N is the number of
  data-bearing disks in the  RAID  (e.g.  for  RAID  5
  there is one parity disk, so N will be the number of
  disks in the array minus 1).  This allows the  block
  allocator to prevent read-modify-write of the parity
  in a RAID stripe if possible when the data is  writ‐
  ten.


Looking at the RAID5 wikipedia page, it seems that the term "stripe" is 
coherent with the ext3 man page.

I suspect that the confusion is due to the fact that in RAID1 a stripe is in 
ONE disk (because the others are like parities). In BTRFS the RAID5/6 code uses 
the structure of RAID1 with some minimal extensions...

To be clear, I don't have problem to be adherent to the BTRFS terminology. 
However I found this very confusing because I was used to a different 
terminology. I am bit worried about the fact that grub uses both MD/DM code and 
BTRFS code; a quick look to the code (eg ./grub-core/disk/diskfilter.c) shows 
that the stripe_size field seems to be related to a disks row without parities.

And... yes in BTRFS "chunk" is a completely different beast than what it is 
reported in the ext3 man page :-)


> 
>> +   *
>> +   *   Disk0  Disk1  Disk2  Disk3
>> +   *
>> +   *A1 B1 P1 Q1
>> +   *Q2 A2 B2 P2
>> +   *P3 Q3 A3 B3
>> +   

Re: [PATCH 1/9] btrfs: Add support for reading a filesystem with a RAID 5 or RAID 6 profile.

[Daniel Kiper]

On Wed, Sep 19, 2018 at 08:40:32PM +0200, Goffredo Baroncelli wrote:
> From: Goffredo Baroncelli 
>
> Signed-off-by: Goffredo Baroncelli 
> ---
>  grub-core/fs/btrfs.c | 66 
>  1 file changed, 66 insertions(+)
>
> diff --git a/grub-core/fs/btrfs.c b/grub-core/fs/btrfs.c
> index be195448d..56c42746d 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_DUPLICATED0x20
>  #define GRUB_BTRFS_CHUNK_TYPE_RAID100x40
> +#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,70 @@ 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, block_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 of several blocks spread on the disks.
> +* The raid terminology is used to call all the blocks of a row
> +* "stripe". Unfortunately the BTRFS terminology confuses block

Stripe is data set or parity (parity stripe) on one disk. Block has
different meaning. Please stick to btrfs terminology and say it clearly
in the comment. And even add a link to btrfs wiki page to ease reading.

I think about this one:
  
https://btrfs.wiki.kernel.org/index.php/Manpage/mkfs.btrfs#BLOCK_GROUPS.2C_CHUNKS.2C_RAID

> +* and stripe.

I do not think so. Or at least not so much...

> +*
> +*   Disk0  Disk1  Disk2  Disk3
> +*
> +*A1 B1 P1 Q1
> +*Q2 A2 B2 P2
> +*P3 Q3 A3 B3
> +*  [...]
> +*
> +*  Note that the placement of the parities depends on row index.
> +*  In the code below:
> +*  - block_nr is the block number without the parities

Well, it seems to me that the btrfs code introduces confusion not the
spec itself. I would leave code as is but s/block number/stripe number/.

> +*(A1 = 0, B1 = 1, A2 = 2, B2 = 3, ...),
> +*  - high is the row number (0 for A1...Q1, 1 for Q2...P2, ...),
> +*  - stripen is the disk number (0 for A1,Q2,P3, 1 for B1...),

s/disk number/disk number in a row/

> +*  - off is the logical address to read
> +*  - chunk_stripe_length is the size of a block (typically 64k),

s/a block/a stripe/

> +*  - nstripes is the number of disks,

s/number of disks/number of disks in a row/

I miss the description of nparities here...

> +*  - low is the offset of the data inside a stripe,
> +*  - stripe_offset is the disk offset,

s/the disk offset/the data offset in an array/?

> +*  - csize is the "potential" data to read. It will be reduced to
> +*size if the latter is smaller.
> +*/
> +   block_nr = grub_divmod64 (off, chunk_stripe_length, );
> +
> +   /*
> +* stripen is computed without the parities (0 for A1, A2, A3...
> +* 1 for B1, B2...).
> +*/
> +   high = grub_divmod64 (block_nr, nstripes - nparities, );

This is clear...

> +   /*
> +* stripen is recomputed considering the parities (0 for A1, 1 for
> +* A2, 2 for A3).
> +*/
> +   grub_divmod64 (high + stripen, nstripes, );

... but this looks strange... You add disk number to row number. Hmmm...
It looks that it works but this is not obvious at first sight. Could you
explain that?

> +   stripe_offset = low + chunk_stripe_length * high;
> +   csize = chunk_stripe_length - low;
> +
> break;
>   }
> default:

Daniel

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH 1/9] btrfs: Add support for reading a filesystem with a RAID 5 or RAID 6 profile.

[Goffredo Baroncelli]

Please ignore this email

On 19/09/2018 20.36, Goffredo Baroncelli wrote:
> From: Goffredo Baroncelli 
> 
> Signed-off-by: Goffredo Baroncelli 
> ---
>  grub-core/fs/btrfs.c | 66 
>  1 file changed, 66 insertions(+)
> 
> diff --git a/grub-core/fs/btrfs.c b/grub-core/fs/btrfs.c
> index be195448d..56c42746d 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_DUPLICATED0x20
>  #define GRUB_BTRFS_CHUNK_TYPE_RAID100x40
> +#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,70 @@ 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, block_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 of several blocks spread on the disks.
> +* The raid terminology is used to call all the blocks of a row
> +* "stripe". Unfortunately the BTRFS terminology confuses block
> +* and stripe.
> +*
> +*   Disk0  Disk1  Disk2  Disk3
> +*
> +*A1 B1 P1 Q1
> +*Q2 A2 B2 P2
> +*P3 Q3 A3 B3
> +*  [...]
> +*
> +*  Note that the placement of the parities depends on row index.
> +*  In the code below:
> +*  - block_nr is the block number without 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 disk number (0 for A1,Q2,P3, 1 for B1...),
> +*  - off is the logical address to read
> +*  - chunk_stripe_length is the size of a block (typically 64k),
> +*  - nstripes is the number of disks,
> +*  - low is the offset of the data inside a stripe,
> +*  - stripe_offset is the disk offset,
> +*  - csize is the "potential" data to read. It will be reduced to
> +*size if the latter is smaller.
> +*/
> +   block_nr = grub_divmod64 (off, chunk_stripe_length, );
> +
> +   /*
> +* stripen is computed without the parities (0 for A1, A2, A3...
> +* 1 for B1, B2...).
> +*/
> +   high = grub_divmod64 (block_nr, nstripes - nparities, );
> +
> +   /*
> +* stripen is recomputed considering the parities (0 for A1, 1 for
> +* A2, 2 for A3).
> +*/
> +   grub_divmod64 (high + stripen, nstripes, );
> +
> +   stripe_offset = low + chunk_stripe_length * high;
> +   csize = chunk_stripe_length - low;
> +
> break;
>   }
> default:
> 


-- 
gpg @keyserver.linux.it: Goffredo Baroncelli 
Key fingerprint BBF5 1610 0B64 DAC6 5F7D  17B2 0EDA 9B37 8B82 E0B5

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

[PATCH 1/9] btrfs: Add support for reading a filesystem with a RAID 5 or RAID 6 profile.

[Goffredo Baroncelli]

From: Goffredo Baroncelli 

Signed-off-by: Goffredo Baroncelli 
---
 grub-core/fs/btrfs.c | 66 
 1 file changed, 66 insertions(+)

diff --git a/grub-core/fs/btrfs.c b/grub-core/fs/btrfs.c
index be195448d..56c42746d 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_DUPLICATED0x20
 #define GRUB_BTRFS_CHUNK_TYPE_RAID100x40
+#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,70 @@ 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, block_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 of several blocks spread on the disks.
+  * The raid terminology is used to call all the blocks of a row
+  * "stripe". Unfortunately the BTRFS terminology confuses block
+  * and stripe.
+  *
+  *   Disk0  Disk1  Disk2  Disk3
+  *
+  *A1 B1 P1 Q1
+  *Q2 A2 B2 P2
+  *P3 Q3 A3 B3
+  *  [...]
+  *
+  *  Note that the placement of the parities depends on row index.
+  *  In the code below:
+  *  - block_nr is the block number without 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 disk number (0 for A1,Q2,P3, 1 for B1...),
+  *  - off is the logical address to read
+  *  - chunk_stripe_length is the size of a block (typically 64k),
+  *  - nstripes is the number of disks,
+  *  - low is the offset of the data inside a stripe,
+  *  - stripe_offset is the disk offset,
+  *  - csize is the "potential" data to read. It will be reduced to
+  *size if the latter is smaller.
+  */
+ block_nr = grub_divmod64 (off, chunk_stripe_length, );
+
+ /*
+  * stripen is computed without the parities (0 for A1, A2, A3...
+  * 1 for B1, B2...).
+  */
+ high = grub_divmod64 (block_nr, nstripes - nparities, );
+
+ /*
+  * stripen is recomputed considering the parities (0 for A1, 1 for
+  * A2, 2 for A3).
+  */
+ grub_divmod64 (high + stripen, nstripes, );
+
+ stripe_offset = low + chunk_stripe_length * high;
+ csize = chunk_stripe_length - low;
+
  break;
}
  default:
-- 
2.19.0


___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

[PATCH 1/9] btrfs: Add support for reading a filesystem with a RAID 5 or RAID 6 profile.

[Goffredo Baroncelli]

From: Goffredo Baroncelli 

Signed-off-by: Goffredo Baroncelli 
---
 grub-core/fs/btrfs.c | 66 
 1 file changed, 66 insertions(+)

diff --git a/grub-core/fs/btrfs.c b/grub-core/fs/btrfs.c
index be195448d..56c42746d 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_DUPLICATED0x20
 #define GRUB_BTRFS_CHUNK_TYPE_RAID100x40
+#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,70 @@ 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, block_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 of several blocks spread on the disks.
+  * The raid terminology is used to call all the blocks of a row
+  * "stripe". Unfortunately the BTRFS terminology confuses block
+  * and stripe.
+  *
+  *   Disk0  Disk1  Disk2  Disk3
+  *
+  *A1 B1 P1 Q1
+  *Q2 A2 B2 P2
+  *P3 Q3 A3 B3
+  *  [...]
+  *
+  *  Note that the placement of the parities depends on row index.
+  *  In the code below:
+  *  - block_nr is the block number without 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 disk number (0 for A1,Q2,P3, 1 for B1...),
+  *  - off is the logical address to read
+  *  - chunk_stripe_length is the size of a block (typically 64k),
+  *  - nstripes is the number of disks,
+  *  - low is the offset of the data inside a stripe,
+  *  - stripe_offset is the disk offset,
+  *  - csize is the "potential" data to read. It will be reduced to
+  *size if the latter is smaller.
+  */
+ block_nr = grub_divmod64 (off, chunk_stripe_length, );
+
+ /*
+  * stripen is computed without the parities (0 for A1, A2, A3...
+  * 1 for B1, B2...).
+  */
+ high = grub_divmod64 (block_nr, nstripes - nparities, );
+
+ /*
+  * stripen is recomputed considering the parities (0 for A1, 1 for
+  * A2, 2 for A3).
+  */
+ grub_divmod64 (high + stripen, nstripes, );
+
+ stripe_offset = low + chunk_stripe_length * high;
+ csize = chunk_stripe_length - low;
+
  break;
}
  default:
-- 
2.19.0


___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH 1/9] btrfs: Add support for reading a filesystem with a RAID 5 or RAID 6 profile.

[Daniel Kiper]

On Wed, Jul 18, 2018 at 08:24:50AM +0200, Goffredo Baroncelli wrote:
> On 07/12/2018 03:46 PM, Daniel Kiper wrote:
> > On Tue, Jun 19, 2018 at 07:39:48PM +0200, Goffredo Baroncelli wrote:
> >> Signed-off-by: Goffredo Baroncelli 
> >> ---
> >>  grub-core/fs/btrfs.c | 70 
> >>  1 file changed, 70 insertions(+)
> >>
> >> diff --git a/grub-core/fs/btrfs.c b/grub-core/fs/btrfs.c
> >> index be195448d..fbabaebbe 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_DUPLICATED0x20
> >>  #define GRUB_BTRFS_CHUNK_TYPE_RAID100x40
> >> +#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,74 @@ 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;
> >> +  }
> >> +
> >> +/*
> >> + * Below is an example of a RAID 6 layout and the meaning of the
> >> + * variables. The same applies to RAID 5. The only differences is
> >> + * that there is only one parity disk instead of two.
> >> + *
> >> + * A RAID 6 layout consists of several stripes spread
> >> + * on the disks, following a layout like the one below
> >> + *
> >> + *   Disk0  Disk1  Disk2  Ddisk3
> >
> > s/Ddisk3/Disk3/
> >
> >> + *
> >> + *A1 B1 P1 Q1
> >> + *Q2 A2 B2 P2
> >> + *P3 Q3 A3 B3
> >> + *  [...]
> >> + *
> >> + *  Note that the placement of the parities depends on row 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 disk number),
> >> + *  - off is the logical address to read (from the beginning of
> >> + *the chunk space),
> >
> > What do you mean by chunk?
>
> Is a specific btrfs data structure (see
> https://btrfs.wiki.kernel.org/index.php/Manpage/mkfs.btrfs#BLOCK_GROUPS.2C_CHUNKS.2C_RAID).
> Most of the BTRFS internal structures (e.g. where the file data is
> stored) are in terms of *logical address*. At this level there is no
> concept of redundancy or raid nor disks. You can see the logical
> address as a flat space, large as the sum of the disks sizes (minus
> the space used by the parities or mirroring).
>
> The "logical space" is divided in "slices" called chunk. A chunk
> typically has a size in terms of hundred of megabytes (IIRC up to 1GB)
>
> A chunk (or block group) is a mapping. A chunk maps a "logical
> address" to a "physical address" on the disks.
>
> You can think a chunk as a structure like:
>
>   u64 profile
>   u64 logical_start, logical_end
>   u64 disk1_id, disk1_start, disk1_end
>   u64 disk2_id, disk2_start, disk2_end
>   u64 disk3_id, disk3_start, disk3_end
>   [...]
>
> In order to translate a "logical_address" to the pair of "disk" and "physical 
> address:
> - find the chunk which maps the logical address (looking to the 
> "logical_start", "logical_end")
> - then on the basis of "profile" value you can have the following cases:
>   profile == SINGLE:
>   is the simplest one: the logical address is mapped to the range
>   (disk1_start...disk1_end) of the disk "disk_id1"
>
>   profile == RAID1
>   like the previous one; however if the disk1 is not available,
>   you can find the data in the range
>   (disk2_start...disk2_end) of the disk "disk_id2". NOTE: the two 
> ranges
>   must have the same sizes, but may have different starts
>
>   profile == RAID0
>   the data is without redundancy and it is spread on different 
> disk each
>   "chunk_stripe_length" bytes; so:
>   off = 

[PATCH 1/9] btrfs: Add support for reading a filesystem with a RAID 5 or RAID 6 profile.

[Goffredo Baroncelli]

On 07/12/2018 03:46 PM, Daniel Kiper wrote:
> On Tue, Jun 19, 2018 at 07:39:48PM +0200, Goffredo Baroncelli wrote:
>> Signed-off-by: Goffredo Baroncelli 
>> ---
>>  grub-core/fs/btrfs.c | 70 
>>  1 file changed, 70 insertions(+)
>>
>> diff --git a/grub-core/fs/btrfs.c b/grub-core/fs/btrfs.c
>> index be195448d..fbabaebbe 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_DUPLICATED0x20
>>  #define GRUB_BTRFS_CHUNK_TYPE_RAID100x40
>> +#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,74 @@ 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;
>> +}
>> +
>> +  /*
>> +   * Below is an example of a RAID 6 layout and the meaning of the
>> +   * variables. The same applies to RAID 5. The only differences is
>> +   * that there is only one parity disk instead of two.
>> +   *
>> +   * A RAID 6 layout consists of several stripes spread
>> +   * on the disks, following a layout like the one below
>> +   *
>> +   *   Disk0  Disk1  Disk2  Ddisk3
> 
> s/Ddisk3/Disk3/
> 
>> +   *
>> +   *A1 B1 P1 Q1
>> +   *Q2 A2 B2 P2
>> +   *P3 Q3 A3 B3
>> +   *  [...]
>> +   *
>> +   *  Note that the placement of the parities depends on row 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 disk number),
>> +   *  - off is the logical address to read (from the beginning of
>> +   *the chunk space),
> 
> What do you mean by chunk?

Is a specific btrfs data structure (see 
https://btrfs.wiki.kernel.org/index.php/Manpage/mkfs.btrfs#BLOCK_GROUPS.2C_CHUNKS.2C_RAID).
 Most of the BTRFS internal structures (e.g. where the file data is stored) are 
in terms of *logical address*. At this level there is no concept of redundancy 
or raid nor disks. You can see the logical address as a flat space, large as 
the sum of the disks sizes (minus the space used by the parities or mirroring).

The "logical space" is divided in "slices" called chunk. A chunk typically has 
a size in terms of hundred of megabytes (IIRC up to 1GB)

A chunk (or block group) is a mapping. A chunk maps a "logical address" to a 
"physical address" on the disks.

You can think a chunk as a structure like:

u64 profile
u64 logical_start, logical_end
u64 disk1_id, disk1_start, disk1_end
u64 disk2_id, disk2_start, disk2_end
u64 disk3_id, disk3_start, disk3_end
[...]

In order to translate a "logical_address" to the pair of "disk" and "physical 
address:
- find the chunk which maps the logical address (looking to the 
"logical_start", "logical_end")
- then on the basis of "profile" value you can have the following cases:
profile == SINGLE:
is the simplest one: the logical address is mapped to the range
(disk1_start...disk1_end) of the disk "disk_id1"

profile == RAID1
like the previous one; however if the disk1 is not available, 
you can find the data in the range 
(disk2_start...disk2_end) of the disk "disk_id2". NOTE: the two 
ranges
must have the same sizes, but may have different starts

profile == RAID0
the data is without redundancy and it is spread on different 
disk each
"chunk_stripe_length" bytes; so:
off = logical_address - logical_start // this is what I 
call
  // address from 
the
   

Re: [PATCH 1/9] btrfs: Add support for reading a filesystem with a RAID 5 or RAID 6 profile.

[Daniel Kiper]

On Tue, Jun 19, 2018 at 07:39:48PM +0200, Goffredo Baroncelli wrote:
> Signed-off-by: Goffredo Baroncelli 
> ---
>  grub-core/fs/btrfs.c | 70 
>  1 file changed, 70 insertions(+)
>
> diff --git a/grub-core/fs/btrfs.c b/grub-core/fs/btrfs.c
> index be195448d..fbabaebbe 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_DUPLICATED0x20
>  #define GRUB_BTRFS_CHUNK_TYPE_RAID100x40
> +#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,74 @@ 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;
> + }
> +
> +   /*
> +* Below is an example of a RAID 6 layout and the meaning of the
> +* variables. The same applies to RAID 5. The only differences is
> +* that there is only one parity disk instead of two.
> +*
> +* A RAID 6 layout consists of several stripes spread
> +* on the disks, following a layout like the one below
> +*
> +*   Disk0  Disk1  Disk2  Ddisk3

s/Ddisk3/Disk3/

> +*
> +*A1 B1 P1 Q1
> +*Q2 A2 B2 P2
> +*P3 Q3 A3 B3
> +*  [...]
> +*
> +*  Note that the placement of the parities depends on row 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 disk number),
> +*  - off is the logical address to read (from the beginning of
> +*the chunk space),

What do you mean by chunk?

> +*  - chunk_stripe_length is the size of a stripe (typically 64k),
> +*  - nstripes is the number of disks,
> +*  - low is the offset of the data inside a stripe,
> +*  - stripe_offset is the disk offset from the beginning
> +*of the disk chunk mapping start,

Err... I am confused here...

> +*  - csize is the "potential" data to read. It will be reduced to
> +*size if the latter is smaller.
> +*/
> +   stripe_nr = grub_divmod64 (off, chunk_stripe_length, );

Here it seems to me that off is from the beginning of RAID volume space.
So, it does not agree with the comment above...

> +   /*
> +* stripen is evaluated without considering
> +* the parities (0 for A1, A2, A3... 1 for B1, B2...).
> +*/
> +   high = grub_divmod64 (stripe_nr, nstripes - nparities, );

This looks good.

> +   /*
> +* stripen now considers also the parities (0 for A1, 1 for A2,
> +* 2 for A3). The math is performed modulo number of disks.
> +*/
> +   grub_divmod64 (high + stripen, nstripes, );

Ditto.

> +   stripe_offset = low + chunk_stripe_length * high;

And here I am confused. Why "* high"? If chunk_stripe_length is stripe
length then stripe_offset leads to nowhere. Am I missing something?
I have a feeling that comments does not agree with the code somehow.

Daniel

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH 1/9] btrfs: Add support for reading a filesystem with a RAID 5 or RAID 6 profile.

[Goffredo Baroncelli]

On 07/09/2018 12:20 PM, Daniel Kiper wrote:
> On Sun, Jul 08, 2018 at 05:51:37PM +0200, Goffredo Baroncelli wrote:
>> A gentle ping.
> 
> Sorry for delay. I remember about this patchset but I am swamped with other 
> stuff.
> I will take a look at it this week.
> 
> Daniel
> 
Thanks for the feedback: I was worried that my emails were lost somewhere 
in/with the spams :-)

BR
G.Baroncelli

-- 
gpg @keyserver.linux.it: Goffredo Baroncelli 
Key fingerprint BBF5 1610 0B64 DAC6 5F7D  17B2 0EDA 9B37 8B82 E0B5

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH 1/9] btrfs: Add support for reading a filesystem with a RAID 5 or RAID 6 profile.

[Daniel Kiper]

On Sun, Jul 08, 2018 at 05:51:37PM +0200, Goffredo Baroncelli wrote:
> A gentle ping.

Sorry for delay. I remember about this patchset but I am swamped with other 
stuff.
I will take a look at it this week.

Daniel

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH 1/9] btrfs: Add support for reading a filesystem with a RAID 5 or RAID 6 profile.

[Goffredo Baroncelli]

A gentle ping.

BR
G.Baroncelli


On 06/19/2018 07:39 PM, Goffredo Baroncelli wrote:
> Signed-off-by: Goffredo Baroncelli 
> ---
>  grub-core/fs/btrfs.c | 70 
>  1 file changed, 70 insertions(+)
> 
> diff --git a/grub-core/fs/btrfs.c b/grub-core/fs/btrfs.c
> index be195448d..fbabaebbe 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_DUPLICATED0x20
>  #define GRUB_BTRFS_CHUNK_TYPE_RAID100x40
> +#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,74 @@ 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;
> + }
> +
> +   /*
> +* Below is an example of a RAID 6 layout and the meaning of the
> +* variables. The same applies to RAID 5. The only differences is
> +* that there is only one parity disk instead of two.
> +*
> +* A RAID 6 layout consists of several stripes spread
> +* on the disks, following a layout like the one below
> +*
> +*   Disk0  Disk1  Disk2  Ddisk3
> +*
> +*A1 B1 P1 Q1
> +*Q2 A2 B2 P2
> +*P3 Q3 A3 B3
> +*  [...]
> +*
> +*  Note that the placement of the parities depends on row 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 disk number),
> +*  - off is the logical address to read (from the beginning of
> +*the chunk space),
> +*  - chunk_stripe_length is the size of a stripe (typically 64k),
> +*  - nstripes is the number of disks,
> +*  - low is the offset of the data inside a stripe,
> +*  - stripe_offset is the disk offset from the beginning
> +*of the disk chunk mapping start,
> +*  - csize is the "potential" data to read. It will be reduced to
> +*size if the latter is smaller.
> +*/
> +   stripe_nr = grub_divmod64 (off, chunk_stripe_length, );
> +
> +   /*
> +* stripen is evaluated without considering
> +* the parities (0 for A1, A2, A3... 1 for B1, B2...).
> +*/
> +   high = grub_divmod64 (stripe_nr, nstripes - nparities, );
> +
> +   /*
> +* stripen now considers also the parities (0 for A1, 1 for A2,
> +* 2 for A3). The math is performed modulo number of disks.
> +*/
> +   grub_divmod64 (high + stripen, nstripes, );
> +
> +   stripe_offset = low + chunk_stripe_length * high;
> +   csize = chunk_stripe_length - low;
> +
> break;
>   }
> default:
> 


-- 
gpg @keyserver.linux.it: Goffredo Baroncelli 
Key fingerprint BBF5 1610 0B64 DAC6 5F7D  17B2 0EDA 9B37 8B82 E0B5

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

[PATCH 1/9] btrfs: Add support for reading a filesystem with a RAID 5 or RAID 6 profile.

[Goffredo Baroncelli]

Signed-off-by: Goffredo Baroncelli 
---
 grub-core/fs/btrfs.c | 70 
 1 file changed, 70 insertions(+)

diff --git a/grub-core/fs/btrfs.c b/grub-core/fs/btrfs.c
index be195448d..fbabaebbe 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_DUPLICATED0x20
 #define GRUB_BTRFS_CHUNK_TYPE_RAID100x40
+#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,74 @@ 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;
+   }
+
+ /*
+  * Below is an example of a RAID 6 layout and the meaning of the
+  * variables. The same applies to RAID 5. The only differences is
+  * that there is only one parity disk instead of two.
+  *
+  * A RAID 6 layout consists of several stripes spread
+  * on the disks, following a layout like the one below
+  *
+  *   Disk0  Disk1  Disk2  Ddisk3
+  *
+  *A1 B1 P1 Q1
+  *Q2 A2 B2 P2
+  *P3 Q3 A3 B3
+  *  [...]
+  *
+  *  Note that the placement of the parities depends on row 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 disk number),
+  *  - off is the logical address to read (from the beginning of
+  *the chunk space),
+  *  - chunk_stripe_length is the size of a stripe (typically 64k),
+  *  - nstripes is the number of disks,
+  *  - low is the offset of the data inside a stripe,
+  *  - stripe_offset is the disk offset from the beginning
+  *of the disk chunk mapping start,
+  *  - csize is the "potential" data to read. It will be reduced to
+  *size if the latter is smaller.
+  */
+ stripe_nr = grub_divmod64 (off, chunk_stripe_length, );
+
+ /*
+  * stripen is evaluated without considering
+  * the parities (0 for A1, A2, A3... 1 for B1, B2...).
+  */
+ high = grub_divmod64 (stripe_nr, nstripes - nparities, );
+
+ /*
+  * stripen now considers also the parities (0 for A1, 1 for A2,
+  * 2 for A3). The math is performed modulo number of disks.
+  */
+ grub_divmod64 (high + stripen, nstripes, );
+
+ stripe_offset = low + chunk_stripe_length * high;
+ csize = chunk_stripe_length - low;
+
  break;
}
  default:
-- 
2.18.0.rc2


___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH 1/9] btrfs: Add support for reading a filesystem with a RAID 5 or RAID 6 profile.

[Goffredo Baroncelli]

On 06/14/2018 08:55 PM, Goffredo Baroncelli wrote:
>>> +  *  - stripe_offset is the offset from the beginning of the chunk
>>> +  *disks physical address,
>> I am not sure that I understand. Could clarify this?
> - stripe_offset is the offset (in bytes) from the beginning of the chunk 
> portion 
>   stored on disk.
> 
> You can think "stripe_offset" as the "row" in the drawing, but measured in 
> bytes.
> 

After some thoughts, I will update this description as:

-  *  - stripe_offset is the offset from the beginning of the chunk
-  *disks physical address,
+  *  - stripe_offset is the disk offset from the beginning
+  *of the disk chunk mapping start,

-- 
gpg @keyserver.linux.it: Goffredo Baroncelli 
Key fingerprint BBF5 1610 0B64 DAC6 5F7D  17B2 0EDA 9B37 8B82 E0B5

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH 1/9] btrfs: Add support for reading a filesystem with a RAID 5 or RAID 6 profile.

[Goffredo Baroncelli]

On 06/14/2018 01:17 PM, Daniel Kiper wrote:
> On Sun, Jun 03, 2018 at 08:53:40PM +0200, Goffredo Baroncelli wrote:
>> Signed-off-by: Goffredo Baroncelli 
>> ---
>>  grub-core/fs/btrfs.c | 70 
>>  1 file changed, 70 insertions(+)
>>
>> diff --git a/grub-core/fs/btrfs.c b/grub-core/fs/btrfs.c
>> index be195448d..4d418859b 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_DUPLICATED0x20
>>  #define GRUB_BTRFS_CHUNK_TYPE_RAID100x40
>> +#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,74 @@ 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;
>> +}
>> +
>> +  /*
>> +   * Below is an example of a RAID 6 layout and the meaning of the
>> +   * variables. The same applies to RAID 5. The only differences is
>> +   * that there is only one parity disk instead of two.
>> +   *
>> +   * A RAID 6 layout consists of several stripes spread
>> +   * on the disks, following a layout like the one below
>> +   *
>> +   *   Disk1  Disk2  Disk3  Ddisk4
> 
> Numbering seems confusing to me. I think that it should be
>Disk0  Disk1  Disk2  Disk3
> 
>> +   *
>> +   *A1 B1 P1 Q1
>> +   *Q2 A2 B2 P2
>> +   *P3 Q3 A3 B3
>> +   *  [...]
>> +   *
>> +   *  Note that the placement of the parities depends on row index.
>> +   *  In the code below:
>> +   *  - stripe_nr is the stripe number not considering the parities
>> +   *(A1=0, B1=1, A2 = 2, B2 = 3, ...),
> 
> Please be consistent. A1 = 0, B1 = 1, A2 = 2, B2 = 3, ...
> 
>> +   *  - high is the row number (0 for A1...Q1, 1 for Q2..P2, ...),
> 
> Ditto. Please always use "..." not "..".
> 
>> +   *  - stripen is the column number (or disk number),
> 
> AIUI starting from 0. Right? If yes then I think that
> drawing above requires disks/columns renumbering.
right
> 
>> +   *  - off is the logical address to read (from the beginning of
>> +   *the chunk space),
> 
> s/chunk space/chunk/?
> 
>> +   *  - chunk_stripe_length is the size of a stripe (typically 64k),
>> +   *  - nstripes is the number of disks,
>> +   *  - low is the offset of the data inside a stripe,
>> +   *  - stripe_offset is the offset from the beginning of the chunk
>> +   *disks physical address,
> 
> I am not sure that I understand. Could clarify this?
- stripe_offset is the offset (in bytes) from the beginning of the chunk 
portion 
  stored on disk.

You can think "stripe_offset" as the "row" in the drawing, but measured in 
bytes.

> 
>> +   *  - csize is the "potential" data to read. It will be reduced to
>> +   *size if the latter is smaller.
>> +   */
>> +  stripe_nr = grub_divmod64 (off, chunk_stripe_length, );
> 
> OK.
> 
>> +  /*
>> +   * stripen is evaluated without considering
>> +   * the parities (0 for A1, A2, A3... 1 for B1, B2...).
>> +   */
>> +  high = grub_divmod64 (stripe_nr, nstripes - nparities, );
> 
> OK.
> 
>> +  /*
>> +   * stripen now considers also the parities (0 for A1, 1 for A2,
>> +   * 2 for A3). The math is performed modulo number of disks.
>> +   */
>> +  grub_divmod64 (high + stripen, nstripes, );
> 
> OK.
> 
>> +  stripe_offset = low + chunk_stripe_length * high;
> 
> Hmmm... I am confused. What does it mean?
> 
> Daniel
> 
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
> 


-- 
gpg @keyserver.linux.it: Goffredo Baroncelli 
Key fingerprint BBF5 1610 0B64 DAC6 5F7D  17B2 0EDA 9B37 8B82 E0B5

Re: [PATCH 1/9] btrfs: Add support for reading a filesystem with a RAID 5 or RAID 6 profile.

[Daniel Kiper]

On Sun, Jun 03, 2018 at 08:53:40PM +0200, Goffredo Baroncelli wrote:
> Signed-off-by: Goffredo Baroncelli 
> ---
>  grub-core/fs/btrfs.c | 70 
>  1 file changed, 70 insertions(+)
>
> diff --git a/grub-core/fs/btrfs.c b/grub-core/fs/btrfs.c
> index be195448d..4d418859b 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_DUPLICATED0x20
>  #define GRUB_BTRFS_CHUNK_TYPE_RAID100x40
> +#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,74 @@ 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;
> + }
> +
> +   /*
> +* Below is an example of a RAID 6 layout and the meaning of the
> +* variables. The same applies to RAID 5. The only differences is
> +* that there is only one parity disk instead of two.
> +*
> +* A RAID 6 layout consists of several stripes spread
> +* on the disks, following a layout like the one below
> +*
> +*   Disk1  Disk2  Disk3  Ddisk4

Numbering seems confusing to me. I think that it should be
   Disk0  Disk1  Disk2  Disk3

> +*
> +*A1 B1 P1 Q1
> +*Q2 A2 B2 P2
> +*P3 Q3 A3 B3
> +*  [...]
> +*
> +*  Note that the placement of the parities depends on row index.
> +*  In the code below:
> +*  - stripe_nr is the stripe number not considering the parities
> +*(A1=0, B1=1, A2 = 2, B2 = 3, ...),

Please be consistent. A1 = 0, B1 = 1, A2 = 2, B2 = 3, ...

> +*  - high is the row number (0 for A1...Q1, 1 for Q2..P2, ...),

Ditto. Please always use "..." not "..".

> +*  - stripen is the column number (or disk number),

AIUI starting from 0. Right? If yes then I think that
drawing above requires disks/columns renumbering.

> +*  - off is the logical address to read (from the beginning of
> +*the chunk space),

s/chunk space/chunk/?

> +*  - chunk_stripe_length is the size of a stripe (typically 64k),
> +*  - nstripes is the number of disks,
> +*  - low is the offset of the data inside a stripe,
> +*  - stripe_offset is the offset from the beginning of the chunk
> +*disks physical address,

I am not sure that I understand. Could clarify this?

> +*  - csize is the "potential" data to read. It will be reduced to
> +*size if the latter is smaller.
> +*/
> +   stripe_nr = grub_divmod64 (off, chunk_stripe_length, );

OK.

> +   /*
> +* stripen is evaluated without considering
> +* the parities (0 for A1, A2, A3... 1 for B1, B2...).
> +*/
> +   high = grub_divmod64 (stripe_nr, nstripes - nparities, );

OK.

> +   /*
> +* stripen now considers also the parities (0 for A1, 1 for A2,
> +* 2 for A3). The math is performed modulo number of disks.
> +*/
> +   grub_divmod64 (high + stripen, nstripes, );

OK.

> +   stripe_offset = low + chunk_stripe_length * high;

Hmmm... I am confused. What does it mean?

Daniel

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

[PATCH 1/9] btrfs: Add support for reading a filesystem with a RAID 5 or RAID 6 profile.

[Goffredo Baroncelli]

Signed-off-by: Goffredo Baroncelli 
---
 grub-core/fs/btrfs.c | 70 
 1 file changed, 70 insertions(+)

diff --git a/grub-core/fs/btrfs.c b/grub-core/fs/btrfs.c
index be195448d..4d418859b 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_DUPLICATED0x20
 #define GRUB_BTRFS_CHUNK_TYPE_RAID100x40
+#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,74 @@ 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;
+   }
+
+ /*
+  * Below is an example of a RAID 6 layout and the meaning of the
+  * variables. The same applies to RAID 5. The only differences is
+  * that there is only one parity disk instead of two.
+  *
+  * A RAID 6 layout consists of several stripes spread
+  * 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 on row 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 disk number),
+  *  - off is the logical address to read (from the beginning of
+  *the chunk space),
+  *  - chunk_stripe_length is the size of a stripe (typically 64k),
+  *  - nstripes is the number of disks,
+  *  - low is the offset of the data inside a stripe,
+  *  - stripe_offset is the offset from the beginning of the chunk
+  *disks physical address,
+  *  - csize is the "potential" data to read. It will be reduced to
+  *size if the latter is smaller.
+  */
+ stripe_nr = grub_divmod64 (off, chunk_stripe_length, );
+
+ /*
+  * stripen is evaluated without considering
+  * the parities (0 for A1, A2, A3... 1 for B1, B2...).
+  */
+ high = grub_divmod64 (stripe_nr, nstripes - nparities, );
+
+ /*
+  * stripen now considers also the parities (0 for A1, 1 for A2,
+  * 2 for A3). The math is performed modulo number of disks.
+  */
+ grub_divmod64 (high + stripen, nstripes, );
+
+ stripe_offset = low + chunk_stripe_length * high;
+ csize = chunk_stripe_length - low;
+
  break;
}
  default:
-- 
2.17.1


___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH 1/9] btrfs: add support for reading a filesystem with a RAID 5 or RAID 6 profile.

[Goffredo Baroncelli]

Hi Daniel,

my comments below
On 05/30/2018 12:07 PM, Daniel Kiper wrote:
> On Wed, May 16, 2018 at 08:48:11PM +0200, Goffredo Baroncelli wrote:
>> Signed-off-by: Goffredo Baroncelli 
[...]

>> +   *  off -> logical address to read (from the beginning of the
>> +   * chunk space)
> 
> What do you mean by "chunk"? Is it e.g. A1 + B1 region? Please make it
> clear what do you mean by "chunk".

Chunk is a very pervasive concept in BTRFS. A reader who looks to a BTRFS
raid code, must know very well this concepts. I am not sure that GRUB code
is the right place to contain a full BTRFS chunk description.

Basically, the BTRFS logical space is mapped to the physical one via the
chunks (aka block group). Each chunk maps a range of the logical address to a
range in the disk(s). If more disks are involved, different profile
might be used (linear, raid0... raid5/6). E.g.: a chunk maps a logical
address (say 0.1GB) to a physical address (say 1GB..2GB of disk1, 3GB..to 
4GB of disk2, in raid1 mode).
The chunks are a low layer. All the BTRFS metadata are in term of the 
logical address (upper layer).

[...]
>> +  /*
>> +   * In the line below stripen is evaluated without considering
> 
> s/In the line below //
> 
>> +   * the parities (0 for A1, A2, A3... 1 for B1, B2...);
> 
> s/;/./
> 
>> +   */
>> +  high = grub_divmod64 (stripe_nr, nstripes - nparities, );
>> +
>> +  /*
>> +   * In the line below stripen, now consider also the parities (0
> 
> s/In the line below stripen, now/Now/

I think that "stripen" must be re-added

> 
>> +   * for A1, 1 for A2, 2 for A3); the math is done "modulo"
> 
> s/; the/. The/
> s/"modulo"/modulo/
> 
>> +   * number of disks
> 
> Full stop at the end of sentence please.
> 
> Daniel
> 


-- 
gpg @keyserver.linux.it: Goffredo Baroncelli 
Key fingerprint BBF5 1610 0B64 DAC6 5F7D  17B2 0EDA 9B37 8B82 E0B5

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH 1/9] btrfs: add support for reading a filesystem with a RAID 5 or RAID 6 profile.

[Daniel Kiper]

On Wed, May 16, 2018 at 08:48:11PM +0200, Goffredo Baroncelli wrote:
> Signed-off-by: Goffredo Baroncelli 
> ---
>  grub-core/fs/btrfs.c | 68 
>  1 file changed, 68 insertions(+)
>
> diff --git a/grub-core/fs/btrfs.c b/grub-core/fs/btrfs.c
> index be195448d..394611cbb 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_DUPLICATED0x20
>  #define GRUB_BTRFS_CHUNK_TYPE_RAID100x40
> +#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,72 @@ 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;
> + }
> +
> +   /*
> +* Below an example of a RAID6 layout and the meaning of the

s/an example of a RAID6/is an example of a RAID 6/

> +* variables. The same apply to RAID5. The only differences is 
> that

s/The same apply to RAID5/The same applies to RAID 5/

> +* there is only one parity instead of two.

s/parity/parity disk/

> +*
> +* A RAID6 6 layout consists in several stripes spread

s/RAID6 6/RAID 6/
s/consists in/consists of/

> +* on the disks, following a layout like the one below

s/on the disks/over the disks/

> +*
> +*   Disk1  Disk2  Disk3  Ddisk4
> +*
> +*A1 B1 P1 Q1
> +*Q2 A2 B2 P2
> +*P3 Q3 A3 B3
> +*  [...]
> +*
> +*  Note that the placement of the parities depends on row index;

Please, please, please, do not abuse ";". Full stop is preffered at the
end of sentence. Please fix this in all patches.

> +*  In the code below:
> +*  stripe_nr -> is the stripe number not considering the parities
> +*   (A1=0, B1=1, A2 = 2, B2 = 3, ...)

May I ask you to do this like that.

The variables below have following meaning:
  - 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, ...),
...
  - low is the offset of the data inside a stripe.

> +*  high -> is the row number (0 for A1...Q1, 1 for Q2..P2, ...)
> +*  stripen -> is the column number (or disk number)
> +*  off -> logical address to read (from the beginning of the
> +* chunk space)

What do you mean by "chunk"? Is it e.g. A1 + B1 region? Please make it
clear what do you mean by "chunk".

> +*  chunk_stripe_length -> size of a stripe (typically 64k)
> +*  nstripes -> number of disks
> +*  low -> offset of the data inside a stripe

It looks that stripe_offset and csize explanation is missing here.

> +*
> +*/
> +   stripe_nr = grub_divmod64 (off, chunk_stripe_length, );
> +
> +   /*
> +* In the line below stripen is evaluated without considering

s/In the line below //

> +* the parities (0 for A1, A2, A3... 1 for B1, B2...);

s/;/./

> +*/
> +   high = grub_divmod64 (stripe_nr, nstripes - nparities, );
> +
> +   /*
> +* In the line below stripen, now consider also the parities (0

s/In the line below stripen, now/Now/

> +* for A1, 1 for A2, 2 for A3); the math is done "modulo"

s/; the/. The/
s/"modulo"/modulo/

> +* number of disks

Full stop at the end of sentence please.

Daniel

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

[PATCH 1/9] btrfs: add support for reading a filesystem with a RAID 5 or RAID 6 profile.

[Goffredo Baroncelli]

Signed-off-by: Goffredo Baroncelli 
---
 grub-core/fs/btrfs.c | 68 
 1 file changed, 68 insertions(+)

diff --git a/grub-core/fs/btrfs.c b/grub-core/fs/btrfs.c
index be195448d..394611cbb 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_DUPLICATED0x20
 #define GRUB_BTRFS_CHUNK_TYPE_RAID100x40
+#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,72 @@ 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;
+   }
+
+ /*
+  * Below an example of a RAID6 layout and the meaning of the
+  * variables. The same apply to RAID5. The only differences is 
that
+  * there is only one parity instead of two.
+  *
+  * A RAID6 6 layout consists in several stripes spread
+  * 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 on row 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 disk number)
+  *  off -> logical address to read (from the beginning of the
+  * chunk space)
+  *  chunk_stripe_length -> size of a stripe (typically 64k)
+  *  nstripes -> number of disks
+  *  low -> offset of the data inside a stripe
+  * 
+  */
+ stripe_nr = grub_divmod64 (off, chunk_stripe_length, );
+
+ /*
+  * In the line below stripen is evaluated without considering
+  * the parities (0 for A1, A2, A3... 1 for B1, B2...);
+  */
+ high = grub_divmod64 (stripe_nr, nstripes - nparities, );
+
+ /*
+  * In the line below stripen, now consider also the parities (0
+  * for A1, 1 for A2, 2 for A3); the math is done "modulo"
+  * number of disks
+  */
+ grub_divmod64 (high + stripen, nstripes, );
+
+ stripe_offset = low + chunk_stripe_length * high;
+ csize = chunk_stripe_length - low;
+
  break;
}
  default:
-- 
2.17.0


___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel