From: Logan Gunthorpe <logang@deltatee.com>
To: linux-kernel@vger.kernel.org, linux-raid@vger.kernel.org,
Song Liu <song@kernel.org>
Cc: Christoph Hellwig <hch@infradead.org>,
Guoqing Jiang <guoqing.jiang@linux.dev>,
Stephen Bates <sbates@raithlin.com>,
Martin Oliveira <Martin.Oliveira@eideticom.com>,
David Sloan <David.Sloan@eideticom.com>,
Logan Gunthorpe <logang@deltatee.com>
Subject: [PATCH 5/5] md/raid5: Ensure batch_last is released before sleeping for quiesce
Date: Wed, 27 Jul 2022 15:06:00 -0600 [thread overview]
Message-ID: <20220727210600.120221-6-logang@deltatee.com> (raw)
In-Reply-To: <20220727210600.120221-1-logang@deltatee.com>
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: 4fcbd9abb6f2 ("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>
---
drivers/md/raid5.c | 36 ++++++++++++++++++++++++++++--------
drivers/md/raid5.h | 2 +-
2 files changed, 29 insertions(+), 9 deletions(-)
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index 0a8687fd1748..421bac221a74 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -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 @@ raid5_get_active_stripe(struct r5conf *conf, sector_t sector,
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;
diff --git a/drivers/md/raid5.h b/drivers/md/raid5.h
index 638d29863503..a5082bed83c8 100644
--- a/drivers/md/raid5.h
+++ b/drivers/md/raid5.h
@@ -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
--
2.30.2
next prev parent reply other threads:[~2022-07-27 21:08 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-07-27 21:05 [PATCH 0/5] Bug fix for recent batching change Logan Gunthorpe
2022-07-27 21:05 ` [PATCH 1/5] md/raid5: Refactor raid5_get_active_stripe() Logan Gunthorpe
2022-07-28 14:13 ` Christoph Hellwig
2022-07-29 22:48 ` Song Liu
2022-08-01 11:47 ` Logan Gunthorpe
2022-08-01 16:49 ` Song Liu
2022-08-01 17:15 ` Christoph Hellwig
2022-08-01 20:50 ` Song Liu
2022-07-27 21:05 ` [PATCH 2/5] md/raid5: Make is_inactive_blocked() helper Logan Gunthorpe
2022-07-28 14:14 ` Christoph Hellwig
2022-07-27 21:05 ` [PATCH 3/5] md/raid5: Drop unnecessary call to r5c_check_stripe_cache_usage() Logan Gunthorpe
2022-07-27 21:05 ` [PATCH 4/5] md/raid5: Move stripe_request_ctx up Logan Gunthorpe
2022-07-27 21:06 ` Logan Gunthorpe [this message]
2022-07-28 14:15 ` [PATCH 5/5] md/raid5: Ensure batch_last is released before sleeping for quiesce Christoph Hellwig
2022-07-28 5:55 ` [PATCH 0/5] Bug fix for recent batching change Song Liu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20220727210600.120221-6-logang@deltatee.com \
--to=logang@deltatee.com \
--cc=David.Sloan@eideticom.com \
--cc=Martin.Oliveira@eideticom.com \
--cc=guoqing.jiang@linux.dev \
--cc=hch@infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-raid@vger.kernel.org \
--cc=sbates@raithlin.com \
--cc=song@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.