[dm-devel] [PATCH] dm raid: fix inclonclusive reshape layout induced on fast raid4/5/6 table reload sequences

[heinzm at redhat.com]

From: Heinz Mauelshagen <heinzm at redhat.com>

In case fast table reloads occur during an ongoing reshape of raid4/5/6 devices as e.g. for raid5
in lvm2 test lvconvert-raid-reshape-stripes-load-reload.sh, the target may read a superblock
in a race window with the MD resync thread causing an inconclusive reshape state
to be read in its constructor.  This causes BUG_ON() to trigger in md_run() as in
"kernel BUG at drivers/md/raid5.c:7567!".

Scenario triggering the bug:

1. the MD sync thread calls end_reshape() from raid5_sync_request() when done reshaping.
   However end_reshape() _only_ updates the reshape position to MaxSector keeping the
   changed layout configuration though, i.e. any delta disks, chunk sector or RAID
   algorithm changes.  That inconclusive configuration is stored in the superblock.

2. dm-raid constructs a mapping loading named inconsistent superblock as of step 1 before step 3 is
   able to finish resetting the reshape state completely and calls md_run() which leads to mentioned
   bug in raid5.c.

3. the MD RAID personality's finish_reshape() is called which resets the reshape information on
   chunk sectors, delta disks etc. eentirely.  This explains why the bug is rarely seen on multi-core
   machines as finish_reshape() races with the dm-raid constructor thus may update the superblock before
   it gets loaded as of step 2.

Fix identifies inconclusive superblock content in the constructor and resets it before calling md_run()
factoring out identifying checks into rs_is_layout_change() to share in existing rs_reshape_requested()
and new rs_reset_inclonclusive_reshape().  Also enhance a comment and remove an empty line.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1916891
Signed-off-by: Heinz Mauelshagen <heinzm at redhat.com>
---
 drivers/md/dm-raid.c | 34 +++++++++++++++++++++++++++++-----
 1 file changed, 29 insertions(+), 5 deletions(-)

diff --git a/drivers/md/dm-raid.c b/drivers/md/dm-raid.c
index cab12b2251ba..4443e277b005 100644
--- a/drivers/md/dm-raid.c
+++ b/drivers/md/dm-raid.c
@@ -1868,6 +1868,14 @@ static bool rs_takeover_requested(struct raid_set *rs)
 	return rs->md.new_level != rs->md.level;
 }
 
+/* True if layout is set to reshape. */
+static bool rs_is_layout_change(struct raid_set *rs, bool use_mddev)
+{
+	return (use_mddev ? rs->md.delta_disks : rs->delta_disks) ||
+	       rs->md.new_layout != rs->md.layout ||
+	       rs->md.new_chunk_sectors != rs->md.chunk_sectors;
+}
+
 /* True if @rs is requested to reshape by ctr */
 static bool rs_reshape_requested(struct raid_set *rs)
 {
@@ -1880,9 +1888,7 @@ static bool rs_reshape_requested(struct raid_set *rs)
 	if (rs_is_raid0(rs))
 		return false;
 
-	change = mddev->new_layout != mddev->layout ||
-		 mddev->new_chunk_sectors != mddev->chunk_sectors ||
-		 rs->delta_disks;
+	change = rs_is_layout_change(rs, false);
 
 	/* Historical case to support raid1 reshape without delta disks */
 	if (rs_is_raid1(rs)) {
@@ -2926,6 +2932,22 @@ static int rs_setup_reshape(struct raid_set *rs)
 	return r;
 }
 
+/*
+ * Reshape:
+ *
+ * if the md resync thread has updated superblock with max reshape position at the end of
+ * a reshape but not (yet) reset the layout configuration changes -> reset the latter.
+ */
+static void rs_reset_inconclusive_reshape(struct raid_set *rs)
+{
+	if (!rs_is_reshaping(rs) && rs_is_layout_change(rs, true)) {
+		WARN_ON(1);
+		rs_set_cur(rs);
+		rs->md.delta_disks = 0;
+		rs->md.reshape_backwards = 0;
+	}
+}
+
 /*
  * Enable/disable discard support on RAID set depending on
  * RAID level and discard properties of underlying RAID members.
@@ -3212,11 +3234,14 @@ static int raid_ctr(struct dm_target *ti, unsigned int argc, char **argv)
 	if (r)
 		goto bad;
 
+	/* Catch any inconclusive reshape superblock content. */
+	rs_reset_inconclusive_reshape(rs);
+
 	/* Start raid set read-only and assumed clean to change in raid_resume() */
 	rs->md.ro = 1;
 	rs->md.in_sync = 1;
 
-	/* Keep array frozen */
+	/* Keep array frozen until resume. */
 	set_bit(MD_RECOVERY_FROZEN, &rs->md.recovery);
 
 	/* Has to be held on running the array */
@@ -3230,7 +3255,6 @@ static int raid_ctr(struct dm_target *ti, unsigned int argc, char **argv)
 	}
 
 	r = md_start(&rs->md);
-
 	if (r) {
 		ti->error = "Failed to start raid array";
 		mddev_unlock(&rs->md);
-- 
2.30.2