[PATCH OLK-6.6 0/3] md/raid1,raid10: fix deadlock and bio accounting in read error path
Splitting a bio while executing in the raid1 thread can lead to recursion, as task->bio_list is NULL in this context. In addition, resubmitting an md_cloned_bio after splitting may lead to a deadlock if the array is suspended before the md driver calls percpu_ref_tryget_live(&mddev->active_io) on it's path to pers->make_request(). Avoid splitting the bio in this context and require that it is either read in full or not at all. This prevents recursion and avoids potential deadlocks during array suspension. Zhou Minqiang (3): md/raid1,raid10: fix deadlock in read error recovery path md/raid1,raid10: fix error-path detection with md_cloned_bio() md/raid1,raid10: fix bio accounting for split md cloned bios drivers/md/md.c | 40 ++++++++++++++++++++++++---------------- drivers/md/md.h | 5 +++++ drivers/md/raid1.c | 15 +++++++++++---- drivers/md/raid10.c | 28 ++++++++++++++++++---------- 4 files changed, 58 insertions(+), 30 deletions(-) -- 2.52.0
mainline inclusion from mainline-v7.2-rc1 commit 7b15c24f805339a585cfe7d72f446b7e88b9bcc0 category: bugfix bugzilla: https://atomgit.com/src-openeuler/kernel/issues/17987 Reference: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?i... -------------------------------- raid1d and raid10d may resubmit a split md cloned bio while handling a read error. In this case, resubmitting the bio can lead to a deadlock if the array is suspended before md_handle_request() acquires an active_io reference via percpu_ref_tryget_live(). Since the cloned bio already holds an active_io reference, trying to acquire another reference via percpu_ref_tryget_live() can lead to a deadlock while the array is suspended. Fix this by using percpu_ref_get() for md cloned bios. Fixes: bb2a9acefaf9 ("md/raid1: switch to use md_account_bio() for io accounting") Fixes: 820455238366 ("md/raid10: switch to use md_account_bio() for io accounting") Signed-off-by: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com> Reviewed-by: Xiao Ni <xiao@kernel.org> Reviewed-by: Yu Kuai <yukuai@fygo.io> Link: https://patch.msgid.link/20260501114652.590037-2-abd.masalkhi@gmail.com Signed-off-by: Yu Kuai <yukuai@fygo.io> Conflicts: drivers/md/md.c drivers/md/md.h [ctx conflicts] Signed-off-by: Zhou Minqiang <zhouminqiang2@huawei.com> --- drivers/md/md.c | 40 ++++++++++++++++++++++++---------------- drivers/md/md.h | 5 +++++ 2 files changed, 29 insertions(+), 16 deletions(-) diff --git a/drivers/md/md.c b/drivers/md/md.c index 5c81ca6f3955..30cd4c1b2941 100644 --- a/drivers/md/md.c +++ b/drivers/md/md.c @@ -378,24 +378,32 @@ static bool is_suspended(struct mddev *mddev, struct bio *bio) bool md_handle_request(struct mddev *mddev, struct bio *bio) { check_suspended: - if (is_suspended(mddev, bio)) { - DEFINE_WAIT(__wait); - /* Bail out if REQ_NOWAIT is set for the bio */ - if (bio->bi_opf & REQ_NOWAIT) { - bio_wouldblock_error(bio); - return true; - } - for (;;) { - prepare_to_wait(&mddev->sb_wait, &__wait, - TASK_UNINTERRUPTIBLE); - if (!is_suspended(mddev, bio)) - break; - schedule(); + if (unlikely(md_cloned_bio(mddev, bio))) { + /* + * This bio is an MD cloned bio and already holds an + * active_io reference, so percpu_ref_get() is safe here. + */ + percpu_ref_get(&mddev->active_io); + } else { + if (is_suspended(mddev, bio)) { + DEFINE_WAIT(__wait); + /* Bail out if REQ_NOWAIT is set for the bio */ + if (bio->bi_opf & REQ_NOWAIT) { + bio_wouldblock_error(bio); + return true; + } + for (;;) { + prepare_to_wait(&mddev->sb_wait, &__wait, + TASK_UNINTERRUPTIBLE); + if (!is_suspended(mddev, bio)) + break; + schedule(); + } + finish_wait(&mddev->sb_wait, &__wait); } - finish_wait(&mddev->sb_wait, &__wait); + if (!percpu_ref_tryget_live(&mddev->active_io)) + goto check_suspended; } - if (!percpu_ref_tryget_live(&mddev->active_io)) - goto check_suspended; if (!mddev->pers->make_request(mddev, bio)) { percpu_ref_put(&mddev->active_io); diff --git a/drivers/md/md.h b/drivers/md/md.h index 99c1151c5a3a..81636e2e0bde 100644 --- a/drivers/md/md.h +++ b/drivers/md/md.h @@ -1003,6 +1003,11 @@ int do_md_run(struct mddev *mddev); extern const struct block_device_operations md_fops; +static inline bool md_cloned_bio(struct mddev *mddev, struct bio *bio) +{ + return bio->bi_pool == &mddev->io_clone_set; +} + /* * MD devices can be used undeneath by DM, in which case ->gendisk is NULL. */ -- 2.52.0
mainline inclusion from mainline-v7.2-rc1 commit 811545e0926d02a6a0b1a1258bb5544777c164d4 category: bugfix bugzilla: https://atomgit.com/src-openeuler/kernel/issues/17694 Reference: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?i... -------------------------------- Detect the error path using md_cloned_bio() instead of relying on r1_bio in raid1 or r10_bio->read_slot in raid10, which may be NULL or -1 after splitting and resubmitting a failed bio. As a result, the error path may not be recognized and memory allocations can incorrectly use GFP_NOIO instead of (GFP_NOIO | __GFP_HIGH), which can lead to a deadlock under memory pressure. Fixes: 689389a06ce7 ("md/raid1: simplify handle_read_error().") Fixes: 545250f24809 ("md/raid10: simplify handle_read_error()") Signed-off-by: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com> Reviewed-by: Xiao Ni <xiao@kernel.org> Link: https://patch.msgid.link/20260501114652.590037-3-abd.masalkhi@gmail.com Signed-off-by: Yu Kuai <yukuai@fygo.io> Signed-off-by: Zhou Minqiang <zhouminqiang2@huawei.com> --- drivers/md/raid1.c | 13 ++++++++++--- drivers/md/raid10.c | 20 ++++++++++++++------ 2 files changed, 24 insertions(+), 9 deletions(-) diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c index e4bd1cee3866..ed18896e1fcc 100644 --- a/drivers/md/raid1.c +++ b/drivers/md/raid1.c @@ -1320,11 +1320,18 @@ static void raid1_read_request(struct mddev *mddev, struct bio *bio, char b[BDEVNAME_SIZE]; /* - * If r1_bio is set, we are blocking the raid1d thread - * so there is a tiny risk of deadlock. So ask for + * An md cloned bio indicates we are in the error path. + * This is more reliable than checking r1_bio, which might + * be NULL even in the error path if a failed bio was split. + */ + bool err_path = md_cloned_bio(mddev, bio); + + /* + * If we are in the error path, we are blocking the raid1d + * thread so there is a tiny risk of deadlock. So ask for * emergency memory if needed. */ - gfp_t gfp = r1_bio ? (GFP_NOIO | __GFP_HIGH) : GFP_NOIO; + gfp_t gfp = err_path ? (GFP_NOIO | __GFP_HIGH) : GFP_NOIO; if (r1bio_existed) { /* Need to get the block device name carefully */ diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c index 94a5bbc0dcfe..2d06e0cee0ac 100644 --- a/drivers/md/raid10.c +++ b/drivers/md/raid10.c @@ -1149,7 +1149,20 @@ static void raid10_read_request(struct mddev *mddev, struct bio *bio, char b[BDEVNAME_SIZE]; int slot = r10_bio->read_slot; struct md_rdev *err_rdev = NULL; - gfp_t gfp = GFP_NOIO; + + /* + * An md cloned bio indicates we are in the error path. + * This is more reliable than checking slot, which might + * be -1 even in the error path if a failed bio was split. + */ + bool err_path = md_cloned_bio(mddev, bio); + + /* + * If we are in the error path, we are blocking the raid10d + * thread so there is a tiny risk of deadlock. So ask for + * emergency memory if needed. + */ + gfp_t gfp = err_path ? (GFP_NOIO | __GFP_HIGH) : GFP_NOIO; if (slot >= 0 && r10_bio->devs[slot].rdev) { /* @@ -1160,11 +1173,6 @@ static void raid10_read_request(struct mddev *mddev, struct bio *bio, * we lose the device name in error messages. */ int disk; - /* - * As we are blocking raid10, it is a little safer to - * use __GFP_HIGH. - */ - gfp = GFP_NOIO | __GFP_HIGH; disk = r10_bio->devs[slot].devnum; err_rdev = conf->mirrors[disk].rdev; -- 2.52.0
mainline inclusion from mainline-v7.2-rc1 commit ba976e3501111d11c550848b3b7341a73035f582 category: bugfix bugzilla: https://atomgit.com/src-openeuler/kernel/issues/17965 Reference: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?i... -------------------------------- Use md_cloned_bio() to control bio accounting instead of relying on r1bio_existed in raid1 or the io_accounting flag in raid10. The previous logic does not reliably reflect whether a bio is an md cloned bio. When a failed bio is split and resubmitted via bio_submit_split_bioset() on the error path, this can lead to either double accounting for md cloned bios, or missing accounting for bios returned from bio_submit_split_bioset() Fix this by using md_cloned_bio() to detect md cloned bios and skip accounting accordingly. Fixes: bb2a9acefaf9 ("md/raid1: switch to use md_account_bio() for io accounting") Fixes: 820455238366 ("md/raid10: switch to use md_account_bio() for io accounting") Signed-off-by: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com> Reviewed-by: Xiao Ni <xiao@kernel.org> Link: https://patch.msgid.link/20260501114652.590037-4-abd.masalkhi@gmail.com Signed-off-by: Yu Kuai <yukuai@fygo.io> Signed-off-by: Zhou Minqiang <zhouminqiang2@huawei.com> --- drivers/md/raid1.c | 2 +- drivers/md/raid10.c | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c index ed18896e1fcc..4b2815bcc012 100644 --- a/drivers/md/raid1.c +++ b/drivers/md/raid1.c @@ -1412,7 +1412,7 @@ static void raid1_read_request(struct mddev *mddev, struct bio *bio, } r1_bio->read_disk = rdisk; - if (!r1bio_existed) { + if (likely(!md_cloned_bio(mddev, bio))) { md_account_bio(mddev, &bio); r1_bio->master_bio = bio; } diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c index 2d06e0cee0ac..38080906cb82 100644 --- a/drivers/md/raid10.c +++ b/drivers/md/raid10.c @@ -1140,7 +1140,7 @@ static bool regular_request_wait(struct mddev *mddev, struct r10conf *conf, } static void raid10_read_request(struct mddev *mddev, struct bio *bio, - struct r10bio *r10_bio, bool io_accounting) + struct r10bio *r10_bio) { struct r10conf *conf = mddev->private; struct bio *read_bio; @@ -1218,7 +1218,7 @@ static void raid10_read_request(struct mddev *mddev, struct bio *bio, } slot = r10_bio->read_slot; - if (io_accounting) { + if (likely(!md_cloned_bio(mddev, bio))) { md_account_bio(mddev, &bio); r10_bio->master_bio = bio; } @@ -1511,7 +1511,7 @@ static void __make_request(struct mddev *mddev, struct bio *bio, int sectors) conf->geo.raid_disks); if (bio_data_dir(bio) == READ) - raid10_read_request(mddev, bio, r10_bio, true); + raid10_read_request(mddev, bio, r10_bio); else raid10_write_request(mddev, bio, r10_bio); } @@ -2832,7 +2832,7 @@ static void handle_read_error(struct mddev *mddev, struct r10bio *r10_bio) rdev_dec_pending(rdev, mddev); r10_bio->state = 0; - raid10_read_request(mddev, r10_bio->master_bio, r10_bio, false); + raid10_read_request(mddev, r10_bio->master_bio, r10_bio); /* * allow_barrier after re-submit to ensure no sync io * can be issued while regular io pending. -- 2.52.0
反馈: 您发送到kernel@openeuler.org的补丁/补丁集，转换为PR失败！ 邮件列表地址：https://mailweb.openeuler.org/archives/list/kernel@openeuler.org/message/XAD... 失败原因：应用补丁/补丁集失败，Patch failed at 0001 md/raid1,raid10: fix deadlock in read error recovery path 建议解决方法：请查看失败原因， 确认补丁是否可以应用在当前期望分支的最新代码上 FeedBack: The patch(es) which you have sent to kernel@openeuler.org has been converted to PR failed! Mailing list address: https://mailweb.openeuler.org/archives/list/kernel@openeuler.org/message/XAD... Failed Reason: apply patch(es) failed, Patch failed at 0001 md/raid1,raid10: fix deadlock in read error recovery path Suggest Solution: please checkout if the failed patch(es) can work on the newest codes in expected branch
participants (2)
-
patchwork bot -
Zhou Minqiang