On Thu, Dec 31 2020, Kevin Vigor wrote:
> The freeze_array() function waits for changes to nr_queued,
> but the raid10d() function alters it without sending a wake.
>
> This can lead to freeze_array() never being woken.
No it cannot.
freeze_array() is only called from handle_read_error() which is only
called from raid10d(). So it cannot possibly be sleeping at the point
where you added the wake_up().
handle_read_error() passes '1' as the 'extra' arg to freeze_array()
which exactly compensates for the recent decrement of ->nr_queued.
NAK.
Thanks,
NeilBrown
>
> Signed-off-by: Kevin Vigor <kvigor@xxxxxxxxx>
> ---
> drivers/md/raid10.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
> index c5d88ef6a45c..f98df9f084c2 100644
> --- a/drivers/md/raid10.c
> +++ b/drivers/md/raid10.c
> @@ -2698,6 +2698,7 @@ static void raid10d(struct md_thread *thread)
> struct r10conf *conf = mddev->private;
> struct list_head *head = &conf->retry_list;
> struct blk_plug plug;
> + int dequeued = 0;
>
> md_check_recovery(mddev);
>
> @@ -2709,6 +2710,7 @@ static void raid10d(struct md_thread *thread)
> while (!list_empty(&conf->bio_end_io_list)) {
> list_move(conf->bio_end_io_list.prev, &tmp);
> conf->nr_queued--;
> + dequeued++;
> }
> }
> spin_unlock_irqrestore(&conf->device_lock, flags);
> @@ -2739,6 +2741,7 @@ static void raid10d(struct md_thread *thread)
> r10_bio = list_entry(head->prev, struct r10bio, retry_list);
> list_del(head->prev);
> conf->nr_queued--;
> + dequeued++;
> spin_unlock_irqrestore(&conf->device_lock, flags);
>
> mddev = r10_bio->mddev;
> @@ -2762,6 +2765,10 @@ static void raid10d(struct md_thread *thread)
> md_check_recovery(mddev);
> }
> blk_finish_plug(&plug);
> +
> + /* in case freeze_array is waiting for changes to nr_queued. */
> + if (conf->array_freeze_pending && dequeued)
> + wake_up(&conf->wait_barrier);
> }
>
> static int init_resync(struct r10conf *conf)
> --
> 2.26.2
Attachment:
signature.asc
Description: PGP signature
