On Thu, 1 May 2025, Benjamin Marzinski wrote:

> __switch_pg() sets m->current_pg before initializing the pathgroup. If
> probe_active_paths() is called and pathgroup has not been fully
> initialized, IO to the paths may fail, even though they will be usable
> when the pathgroup has finished initializing. Instead, skip sending IO
> in this situation and just return the number of usable paths. The next
> request will wait for the pathgroup to be fully initialized.

Thanks.

I folded this change into the existing patch.

Mikulas


> Fixes: d5d16fabe431 ("dm mpath: Interface for explicit probing of active 
> paths")
> Signed-off-by: Benjamin Marzinski <bmarz...@redhat.com>
> ---
>  drivers/md/dm-mpath.c | 15 ++++++++++++---
>  1 file changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/md/dm-mpath.c b/drivers/md/dm-mpath.c
> index af17a35c6457..53861ad5dd1d 100644
> --- a/drivers/md/dm-mpath.c
> +++ b/drivers/md/dm-mpath.c
> @@ -2068,19 +2068,28 @@ static int probe_path(struct pgpath *pgpath)
>   *
>   * Return -ENOTCONN if no valid path is left (even outside of current_pg). We
>   * cannot probe paths in other pgs without switching current_pg, so if valid
> - * paths are only in different pgs, they may or may not work. Userspace can
> - * submit a request and we'll switch. If the request fails, it may need to
> + * paths are only in different pgs, they may or may not work. Additionally
> + * we should not probe paths in a pathgroup that is in the process of
> + * Initializing. Userspace can submit a request and we'll switch and wait
> + * for the pathgroup to be initialized. If the request fails, it may need to
>   * probe again.
>   */
>  static int probe_active_paths(struct multipath *m)
>  {
>       struct pgpath *pgpath;
>       struct priority_group *pg;
> +     unsigned long flags;
>       int r = 0;
>  
>       mutex_lock(&m->work_mutex);
>  
> -     pg = READ_ONCE(m->current_pg);
> +     spin_lock_irqsave(&m->lock, flags);
> +     if (test_bit(MPATHF_QUEUE_IO, &m->flags))
> +             pg = NULL;
> +     else
> +             pg = m->current_pg;
> +     spin_unlock_irqrestore(&m->lock, flags);
> +
>       if (pg) {
>               list_for_each_entry(pgpath, &pg->pgpaths, list) {
>                       if (!pgpath->is_active)
> -- 
> 2.48.1
> 


Reply via email to