Re: [PATCH 04/13] libmultipath: Add bio handling

[Nilay Shroff <nilay@linux.ibm.com>]

On 3/2/26 9:22 PM, John Garry wrote:
> On 02/03/2026 12:39, Nilay Shroff wrote:
>>> static struct mpath_device *mpath_find_path(struct mpath_head 
>>> *mpath_head)
>>>   {
>>>       enum mpath_iopolicy_e iopolicy =
>>> @@ -243,6 +243,66 @@ static struct mpath_device 
>>> *mpath_find_path(struct mpath_head *mpath_head)
>>>       }
>>>   }
>>> +static bool mpath_available_path(struct mpath_head *mpath_head)
>>> +{
>>> +    struct mpath_device *mpath_device;
>>> +
>>> +    if (!test_bit(MPATH_HEAD_DISK_LIVE, &mpath_head->flags))
>>> +        return false;
>>> +
>>> +    list_for_each_entry_srcu(mpath_device, &mpath_head->dev_list, 
>>> siblings,
>>> +                 srcu_read_lock_held(&mpath_head->srcu)) {
>>> +        bool available = false;
>>> +
>>> +        if (!mpath_head->mpdt->available_path(mpath_device,
>>> +                &available))
>>> +            continue;
>>> +        if (available)
>>> +            return true;
>>> +    }
>>> +
>>> +    return false;
>>> +}
>>
>> IMO, we may further simplify the callback ->available_path() to return 
>> true or false instead of passing the result in a separate @available 
>> argument.
> 
> I have to admit that I am not keen on this abstraction at all, as it is 
> purely generated to fit the current code.
> 
> Anyway, from checking mainline nvme_available_path(), we skip checking 
> the ctrl state if the ctrl failfast flag is set (which means mpath_head- 
>  >mpdt->available_path returns false). But I suppose the callback could 
> check both the ctrl flags and state (and just return a single boolean), 
> like:
> 
> if (failfast flag set)
>      return false;
> if (ctrl live, resetting, connecting)
>      return true;
> return false;
> 
Yes I think, as now the ->dev_list (or ns sibling) iterator is handled 
within libmultipath code, the above logic makes sense. We should plan to 
simplify nvme_available_path() as per the above pseudo code.

Thanks,
--Nilay