md/raid5: Ensure batch_last is released before sleeping for quiesce
authorLogan Gunthorpe <logang@deltatee.com>
Wed, 27 Jul 2022 21:06:00 +0000 (15:06 -0600)
committerJens Axboe <axboe@kernel.dk>
Wed, 3 Aug 2022 03:08:53 +0000 (21:08 -0600)
A race condition exists where if raid5_quiesce() is called in the
middle of a request that has set batch_last, it will deadlock.

batch_last will hold a reference to a stripe when raid5_quiesce() is
called. This will cause the next raid5_get_active_stripe() call to
sleep waiting for the quiesce to finish, but the raid5_quiesce() thread
will wait for active_stripes to go to zero which will never happen
because request thread is waiting for the quiesce to stop.

Fix this by creating a special __raid5_get_active_stripe() function
which takes the request context and clears the last_batch before
sleeping.

While we're at it, change the arguments of raid5_get_active_stripe()
to bools.

Fixes: 3312e6c887fe ("md/raid5: Keep a reference to last stripe_head for batch")
Reported-by: David Sloan <David.Sloan@eideticom.com>
Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
Signed-off-by: Song Liu <song@kernel.org>
Signed-off-by: Jens Axboe <axboe@kernel.dk>
drivers/md/raid5.c
drivers/md/raid5.h

index 44df461362b2fb90f882635c26c75156c9819347..8d10b190bdbbe24088ecfa1bdadb2d5117958ce4 100644 (file)
@@ -800,9 +800,9 @@ static bool is_inactive_blocked(struct r5conf *conf, int hash)
        return active < (conf->max_nr_stripes * 3 / 4);
 }
 
-struct stripe_head *
-raid5_get_active_stripe(struct r5conf *conf, sector_t sector,
-                       int previous, int noblock, int noquiesce)
+static struct stripe_head *__raid5_get_active_stripe(struct r5conf *conf,
+               struct stripe_request_ctx *ctx, sector_t sector,
+               bool previous, bool noblock, bool noquiesce)
 {
        struct stripe_head *sh;
        int hash = stripe_hash_locks_hash(conf, sector);
@@ -812,9 +812,22 @@ raid5_get_active_stripe(struct r5conf *conf, sector_t sector,
        spin_lock_irq(conf->hash_locks + hash);
 
 retry:
-       wait_event_lock_irq(conf->wait_for_quiescent,
-                           conf->quiesce == 0 || noquiesce,
-                           *(conf->hash_locks + hash));
+       if (!noquiesce && conf->quiesce) {
+               /*
+                * Must release the reference to batch_last before waiting,
+                * on quiesce, otherwise the batch_last will hold a reference
+                * to a stripe and raid5_quiesce() will deadlock waiting for
+                * active_stripes to go to zero.
+                */
+               if (ctx && ctx->batch_last) {
+                       raid5_release_stripe(ctx->batch_last);
+                       ctx->batch_last = NULL;
+               }
+
+               wait_event_lock_irq(conf->wait_for_quiescent, !conf->quiesce,
+                                   *(conf->hash_locks + hash));
+       }
+
        sh = find_get_stripe(conf, sector, conf->generation - previous, hash);
        if (sh)
                goto out;
@@ -850,6 +863,13 @@ out:
        return sh;
 }
 
+struct stripe_head *raid5_get_active_stripe(struct r5conf *conf,
+               sector_t sector, bool previous, bool noblock, bool noquiesce)
+{
+       return __raid5_get_active_stripe(conf, NULL, sector, previous, noblock,
+                                        noquiesce);
+}
+
 static bool is_full_stripe_write(struct stripe_head *sh)
 {
        BUG_ON(sh->overwrite_disks > (sh->disks - sh->raid_conf->max_degraded));
@@ -5992,8 +6012,8 @@ static enum stripe_result make_stripe_request(struct mddev *mddev,
        pr_debug("raid456: %s, sector %llu logical %llu\n", __func__,
                 new_sector, logical_sector);
 
-       sh = raid5_get_active_stripe(conf, new_sector, previous,
-                                    (bi->bi_opf & REQ_RAHEAD), 0);
+       sh = __raid5_get_active_stripe(conf, ctx, new_sector, previous,
+                                      (bi->bi_opf & REQ_RAHEAD), 0);
        if (unlikely(!sh)) {
                /* cannot get stripe, just give-up */
                bi->bi_status = BLK_STS_IOERR;
index 638d29863503b836899d72dcf54be1af1b88b6b3..a5082bed83c863f0e2bbd939d8bc9d3aad9b1ccc 100644 (file)
@@ -812,7 +812,7 @@ extern sector_t raid5_compute_sector(struct r5conf *conf, sector_t r_sector,
                                     struct stripe_head *sh);
 extern struct stripe_head *
 raid5_get_active_stripe(struct r5conf *conf, sector_t sector,
-                       int previous, int noblock, int noquiesce);
+                       bool previous, bool noblock, bool noquiesce);
 extern int raid5_calc_degraded(struct r5conf *conf);
 extern int r5c_journal_mode_set(struct mddev *mddev, int journal_mode);
 #endif