Re: [PATCH] md/raid0: avoid RAID0 data corruption due to layout confusion.

[Guoqing Jiang]

On 9/9/19 8:57 AM, NeilBrown wrote:

If the drives in a RAID0 are not all the same size, the array is
divided into zones.
The first zone covers all drives, to the size of the smallest.
The second zone covers all drives larger than the smallest, up to
the size of the second smallest - etc.

A change in Linux 3.14 unintentionally changed the layout for the
second and subsequent zones.  All the correct data is still stored, but
each chunk may be assigned to a different device than in pre-3.14 kernels.
This can lead to data corruption.

It is not possible to determine what layout to use - it depends which
kernel the data was written by.
So we add a module parameter to allow the old (0) or new (1) layout to be
specified, and refused to assemble an affected array if that parameter is
not set.

Fixes: 20d0189b1012 ("block: Introduce new bio_split()")
cc: sta...@vger.kernel.org (3.14+)
Signed-off-by: NeilBrown <ne...@suse.de>
---

This and the next patch are my proposal for how to address
this problem.  I haven't actually tested .....

NeilBrown

  drivers/md/raid0.c | 28 +++++++++++++++++++++++++++-
  drivers/md/raid0.h | 14 ++++++++++++++
  2 files changed, 41 insertions(+), 1 deletion(-)

diff --git a/drivers/md/raid0.c b/drivers/md/raid0.c
index bf5cf184a260..a8888c12308a 100644
--- a/drivers/md/raid0.c
+++ b/drivers/md/raid0.c
@@ -19,6 +19,9 @@
  #include "raid0.h"
  #include "raid5.h"

+static int default_layout = -1;

+module_param(default_layout, int, 0644);
+
  #define UNSUPPORTED_MDDEV_FLAGS               \
        ((1L << MD_HAS_JOURNAL) | \
         (1L << MD_JOURNAL_CLEAN) |       \
@@ -139,6 +142,19 @@ static int create_strip_zones(struct mddev *mddev, struct 
r0conf **private_conf)
        }
        pr_debug("md/raid0:%s: FINAL %d zones\n",
                 mdname(mddev), conf->nr_strip_zones);
+
+       if (conf->nr_strip_zones == 1) {
+               conf->layout = RAID0_ORIG_LAYOUT;
+       } else if (default_layout == RAID0_ORIG_LAYOUT ||
+                  default_layout == RAID0_ALT_MULTIZONE_LAYOUT) {
+               conf->layout = default_layout;
+       } else {
+               pr_err("md/raid0:%s: cannot assemble multi-zone RAID0 with 
default_layout setting\n",
+                      mdname(mddev));
+               pr_err("md/raid0: please set raid.default_layout to 0 or 1\n");

Maybe "1 or 2" to align with the definition of below r0layout?

[snip]

+enum r0layout {
+       RAID0_ORIG_LAYOUT = 1,
+       RAID0_ALT_MULTIZONE_LAYOUT = 2,
+};
  struct r0conf {
        struct strip_zone       *strip_zone;
        struct md_rdev          **devlist; /* lists of rdevs, pointed to
                                            * by strip_zone->dev */
        int                     nr_strip_zones;
+       enum r0layout           layout;
  };

#endif

Thanks,
Guoqing