Re: [PATCH 1/2] md/raid10: wake pending freeze in raid10d()

[NeilBrown <neilb@suse.de>]

[-- Attachment #1: Type: text/plain, Size: 2113 bytes --]

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@gmail.com>
> ---
>  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 #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 853 bytes --]