On Mon, 2024-07-01 at 14:41 -0400, Benjamin Marzinski wrote:
>
>
>
> My purpose in writing 0f850db wasn't to cut down on ioctls. It was to
> keep set_no_path_retry() from incorrectly handling queueing in cases
> where no_path_retry was set to a number, and the device had no usable
> paths. My purpose in this patch is also to make sure
> set_no_path_retry() does the right thing. Obviously, this one is more
> of
> a corner case, and if instead we were occasionally resyncing with the
> kernel even when a multipath device had no paths, that would solve
> this
> issue, and I'd be fine with dropping this patch.
That sounds good.
> So unless it conflicts with work you're already doing, I'll send a
> new
> patchset that changes checkerloop() to pull the multipath device
> updating work out of check_path(). I'm not sure that it makes a lot
> of
> sense to change how we loop through the paths, since check_path()
> does
> work on some paths that aren't assigned to a multipath device.
> Plus
> different paths in a multipath device will need checking at different
> times depending on their states anyway, so you can't just go through
> all
> the paths at once.
While I agree that syncing the map state just once per checkerloop and
map is the right thing, I believe we should do it as soon as we have
checked all paths that need to be checked for the given map. And that's
most easily done by looping over the mpath vector, like this:
for each mpp
for each path in mpp
if path needs to be checked:
check path
sync mpp
for each path
if (!pp->mpp and path needs to be checked)
check path
Of course we could do this in multiple steps and move the mpp sync out
of the path loop first, like you suggested. But if we do this, there
may be a considerable time delay between checking the paths of a given
map and syncing the related mpp state, especially on systems with a
large number of devices, and that might be problematic.
Path status changes are asynchronous by nature, and thus whatever we do
will not be perfect. IMO it makes sense to do all checks related to a
given map in as short a time interval as possible. This way we are most
likely to get a consistent picture of the current state of the map in
question in the kernel.
In the long run, I think what we should do is use asynchronous checkers
exclusively. And instead of waiting a millisecond for them to complete
synchronously, we should trigger the path check on all paths (that need
to be checked) in one go, then wait for all checkers to complete (this
is the difficult part, I know), and then go over all the results, again
without waiting for anything.
Another project I've been wanting to do for a long time :-/
Regards
Martin
