On Tue, 2024-07-02 at 14:10 -0400, Benjamin Marzinski wrote: > On Tue, Jul 02, 2024 at 09:34:17AM +0200, Martin Wilck wrote: > > 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 > > Huh? Are you suggesting syncing the mpp state every second? That > would > be an increase in the number of ioctls we're doing in many cases. That was not my intention. My pseudo-code was oversimplified. We could of carry a boolean in the inner loop that would check whether any of the path checks suggest doing a map sync, iow, if any of the check_path() invocations had proceeded down to the call to update_multipath_strings(). That way we'd necessarily do less ioctls. No?
