* [PATCH -next] md: synchronize flush io with array reconfiguration
@ 2023-11-08 18:02 Yu Kuai
2023-11-24 17:36 ` Song Liu
0 siblings, 1 reply; 3+ messages in thread
From: Yu Kuai @ 2023-11-08 18:02 UTC (permalink / raw)
To: xni, song, maan, neilb
Cc: linux-raid, linux-kernel, yukuai3, yukuai1, yi.zhang, yangerkun
From: Yu Kuai <yukuai3@huawei.com>
Currently rcu is used to protect iterating rdev from submit_flushes():
submit_flushes remove_and_add_spares
synchronize_rcu
pers->hot_remove_disk()
rcu_read_lock()
rdev_for_each_rcu
if (rdev->raid_disk >= 0)
rdev->radi_disk = -1;
atomic_inc(&rdev->nr_pending)
rcu_read_unlock()
bi = bio_alloc_bioset()
bi->bi_end_io = md_end_flush
bi->private = rdev
submit_bio
// issue io for removed rdev
Fix this problem by grabbing 'acive_io' before iterating rdev, make sure
that remove_and_add_spares() won't concurrent with submit_flushes().
Fixes: a2826aa92e2e ("md: support barrier requests on all personalities.")
Signed-off-by: Yu Kuai <yukuai3@huawei.com>
---
drivers/md/md.c | 21 +++++++++++++++------
1 file changed, 15 insertions(+), 6 deletions(-)
diff --git a/drivers/md/md.c b/drivers/md/md.c
index 4ee4593c874a..eb3e455bcbae 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -529,6 +529,9 @@ static void md_end_flush(struct bio *bio)
rdev_dec_pending(rdev, mddev);
if (atomic_dec_and_test(&mddev->flush_pending)) {
+ /* The pair is percpu_ref_tryget() from md_flush_request() */
+ percpu_ref_put(&mddev->active_io);
+
/* The pre-request flush has finished */
queue_work(md_wq, &mddev->flush_work);
}
@@ -548,12 +551,8 @@ static void submit_flushes(struct work_struct *ws)
rdev_for_each_rcu(rdev, mddev)
if (rdev->raid_disk >= 0 &&
!test_bit(Faulty, &rdev->flags)) {
- /* Take two references, one is dropped
- * when request finishes, one after
- * we reclaim rcu_read_lock
- */
struct bio *bi;
- atomic_inc(&rdev->nr_pending);
+
atomic_inc(&rdev->nr_pending);
rcu_read_unlock();
bi = bio_alloc_bioset(rdev->bdev, 0,
@@ -564,7 +563,6 @@ static void submit_flushes(struct work_struct *ws)
atomic_inc(&mddev->flush_pending);
submit_bio(bi);
rcu_read_lock();
- rdev_dec_pending(rdev, mddev);
}
rcu_read_unlock();
if (atomic_dec_and_test(&mddev->flush_pending))
@@ -617,6 +615,17 @@ bool md_flush_request(struct mddev *mddev, struct bio *bio)
/* new request after previous flush is completed */
if (ktime_after(req_start, mddev->prev_flush_start)) {
WARN_ON(mddev->flush_bio);
+ /*
+ * Grab a reference to make sure mddev_suspend() will wait for
+ * this flush to be done.
+ *
+ * md_flush_reqeust() is called under md_handle_request() and
+ * 'active_io' is already grabbed, hence percpu_ref_tryget()
+ * won't fail, percpu_ref_tryget_live() can't be used because
+ * percpu_ref_kill() can be called by mddev_suspend()
+ * concurrently.
+ */
+ percpu_ref_tryget(&mddev->active_io);
mddev->flush_bio = bio;
bio = NULL;
}
--
2.39.2
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH -next] md: synchronize flush io with array reconfiguration
2023-11-08 18:02 [PATCH -next] md: synchronize flush io with array reconfiguration Yu Kuai
@ 2023-11-24 17:36 ` Song Liu
2023-11-25 6:48 ` Yu Kuai
0 siblings, 1 reply; 3+ messages in thread
From: Song Liu @ 2023-11-24 17:36 UTC (permalink / raw)
To: Yu Kuai
Cc: xni, maan, neilb, linux-raid, linux-kernel, yukuai3, yi.zhang, yangerkun
On Wed, Nov 8, 2023 at 2:07 AM Yu Kuai <yukuai1@huaweicloud.com> wrote:
>
> From: Yu Kuai <yukuai3@huawei.com>
>
> Currently rcu is used to protect iterating rdev from submit_flushes():
>
> submit_flushes remove_and_add_spares
> synchronize_rcu
> pers->hot_remove_disk()
> rcu_read_lock()
> rdev_for_each_rcu
> if (rdev->raid_disk >= 0)
> rdev->radi_disk = -1;
> atomic_inc(&rdev->nr_pending)
> rcu_read_unlock()
> bi = bio_alloc_bioset()
> bi->bi_end_io = md_end_flush
> bi->private = rdev
> submit_bio
> // issue io for removed rdev
>
> Fix this problem by grabbing 'acive_io' before iterating rdev, make sure
> that remove_and_add_spares() won't concurrent with submit_flushes().
>
> Fixes: a2826aa92e2e ("md: support barrier requests on all personalities.")
> Signed-off-by: Yu Kuai <yukuai3@huawei.com>
LGTM.
> ---
> drivers/md/md.c | 21 +++++++++++++++------
> 1 file changed, 15 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index 4ee4593c874a..eb3e455bcbae 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -529,6 +529,9 @@ static void md_end_flush(struct bio *bio)
> rdev_dec_pending(rdev, mddev);
>
> if (atomic_dec_and_test(&mddev->flush_pending)) {
> + /* The pair is percpu_ref_tryget() from md_flush_request() */
> + percpu_ref_put(&mddev->active_io);
> +
> /* The pre-request flush has finished */
> queue_work(md_wq, &mddev->flush_work);
> }
> @@ -548,12 +551,8 @@ static void submit_flushes(struct work_struct *ws)
> rdev_for_each_rcu(rdev, mddev)
> if (rdev->raid_disk >= 0 &&
> !test_bit(Faulty, &rdev->flags)) {
> - /* Take two references, one is dropped
> - * when request finishes, one after
> - * we reclaim rcu_read_lock
> - */
> struct bio *bi;
> - atomic_inc(&rdev->nr_pending);
> +
> atomic_inc(&rdev->nr_pending);
> rcu_read_unlock();
> bi = bio_alloc_bioset(rdev->bdev, 0,
> @@ -564,7 +563,6 @@ static void submit_flushes(struct work_struct *ws)
> atomic_inc(&mddev->flush_pending);
> submit_bio(bi);
> rcu_read_lock();
> - rdev_dec_pending(rdev, mddev);
> }
> rcu_read_unlock();
> if (atomic_dec_and_test(&mddev->flush_pending))
> @@ -617,6 +615,17 @@ bool md_flush_request(struct mddev *mddev, struct bio *bio)
> /* new request after previous flush is completed */
> if (ktime_after(req_start, mddev->prev_flush_start)) {
> WARN_ON(mddev->flush_bio);
> + /*
> + * Grab a reference to make sure mddev_suspend() will wait for
> + * this flush to be done.
> + *
> + * md_flush_reqeust() is called under md_handle_request() and
> + * 'active_io' is already grabbed, hence percpu_ref_tryget()
> + * won't fail, percpu_ref_tryget_live() can't be used because
> + * percpu_ref_kill() can be called by mddev_suspend()
> + * concurrently.
> + */
> + percpu_ref_tryget(&mddev->active_io);
Probably add an warn_on here to catch any issues in the future.
Thanks,
Song
> mddev->flush_bio = bio;
> bio = NULL;
> }
> --
> 2.39.2
>
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH -next] md: synchronize flush io with array reconfiguration
2023-11-24 17:36 ` Song Liu
@ 2023-11-25 6:48 ` Yu Kuai
0 siblings, 0 replies; 3+ messages in thread
From: Yu Kuai @ 2023-11-25 6:48 UTC (permalink / raw)
To: Song Liu, Yu Kuai
Cc: xni, maan, neilb, linux-raid, linux-kernel, yi.zhang, yangerkun,
yukuai (C)
Hi,
在 2023/11/25 1:36, Song Liu 写道:
> On Wed, Nov 8, 2023 at 2:07 AM Yu Kuai <yukuai1@huaweicloud.com> wrote:
>>
>> From: Yu Kuai <yukuai3@huawei.com>
>>
>> Currently rcu is used to protect iterating rdev from submit_flushes():
>>
>> submit_flushes remove_and_add_spares
>> synchronize_rcu
>> pers->hot_remove_disk()
>> rcu_read_lock()
>> rdev_for_each_rcu
>> if (rdev->raid_disk >= 0)
>> rdev->radi_disk = -1;
>> atomic_inc(&rdev->nr_pending)
>> rcu_read_unlock()
>> bi = bio_alloc_bioset()
>> bi->bi_end_io = md_end_flush
>> bi->private = rdev
>> submit_bio
>> // issue io for removed rdev
>>
>> Fix this problem by grabbing 'acive_io' before iterating rdev, make sure
>> that remove_and_add_spares() won't concurrent with submit_flushes().
>>
>> Fixes: a2826aa92e2e ("md: support barrier requests on all personalities.")
>> Signed-off-by: Yu Kuai <yukuai3@huawei.com>
>
> LGTM.
>
>> ---
>> drivers/md/md.c | 21 +++++++++++++++------
>> 1 file changed, 15 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/md/md.c b/drivers/md/md.c
>> index 4ee4593c874a..eb3e455bcbae 100644
>> --- a/drivers/md/md.c
>> +++ b/drivers/md/md.c
>> @@ -529,6 +529,9 @@ static void md_end_flush(struct bio *bio)
>> rdev_dec_pending(rdev, mddev);
>>
>> if (atomic_dec_and_test(&mddev->flush_pending)) {
>> + /* The pair is percpu_ref_tryget() from md_flush_request() */
>> + percpu_ref_put(&mddev->active_io);
>> +
>> /* The pre-request flush has finished */
>> queue_work(md_wq, &mddev->flush_work);
>> }
>> @@ -548,12 +551,8 @@ static void submit_flushes(struct work_struct *ws)
>> rdev_for_each_rcu(rdev, mddev)
>> if (rdev->raid_disk >= 0 &&
>> !test_bit(Faulty, &rdev->flags)) {
>> - /* Take two references, one is dropped
>> - * when request finishes, one after
>> - * we reclaim rcu_read_lock
>> - */
>> struct bio *bi;
>> - atomic_inc(&rdev->nr_pending);
>> +
>> atomic_inc(&rdev->nr_pending);
>> rcu_read_unlock();
>> bi = bio_alloc_bioset(rdev->bdev, 0,
>> @@ -564,7 +563,6 @@ static void submit_flushes(struct work_struct *ws)
>> atomic_inc(&mddev->flush_pending);
>> submit_bio(bi);
>> rcu_read_lock();
>> - rdev_dec_pending(rdev, mddev);
>> }
>> rcu_read_unlock();
>> if (atomic_dec_and_test(&mddev->flush_pending))
>> @@ -617,6 +615,17 @@ bool md_flush_request(struct mddev *mddev, struct bio *bio)
>> /* new request after previous flush is completed */
>> if (ktime_after(req_start, mddev->prev_flush_start)) {
>> WARN_ON(mddev->flush_bio);
>> + /*
>> + * Grab a reference to make sure mddev_suspend() will wait for
>> + * this flush to be done.
>> + *
>> + * md_flush_reqeust() is called under md_handle_request() and
>> + * 'active_io' is already grabbed, hence percpu_ref_tryget()
>> + * won't fail, percpu_ref_tryget_live() can't be used because
>> + * percpu_ref_kill() can be called by mddev_suspend()
>> + * concurrently.
>> + */
>> + percpu_ref_tryget(&mddev->active_io);
>
> Probably add an warn_on here to catch any issues in the future.
Will do this in v2.
Thanks,
Kuai
>
> Thanks,
> Song
>
>> mddev->flush_bio = bio;
>> bio = NULL;
>> }
>> --
>> 2.39.2
>>
> .
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-11-25 6:48 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-11-08 18:02 [PATCH -next] md: synchronize flush io with array reconfiguration Yu Kuai
2023-11-24 17:36 ` Song Liu
2023-11-25 6:48 ` Yu Kuai
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®