| Message ID | 20241211225909.298770-1-mwilck@suse.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | multipathd: More map reload handling, and checkerloop work | expand |
On Wed, Dec 11, 2024 at 11:58:55PM +0100, Martin Wilck wrote: For all the patches except 3, 4, and 8: Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com> > This patch set goes on top of Ben's set [1] for github issue 105 [2]. > > The first patch implements the remark I had on patch 2 on Ben's set, the 2nd > is a minor cleanup. > > Patch 3 moves the sync_mpp() call from the beginning to the end of the > checkerloop, as suggested by Ben in [3]. If an inconsitency is found > (mpp->need_reload), we reload the map and schedule another sync for the > next tick (patch 4). > > Patch 5 ff. reshuffle the code in checkerloop(). There is now one function, > checker_finished(), that takes all actions that need to be done with the vecs > lock taken after the checkers have finished. checkerloop() enters this > function immediately when the checkers have finished, without dropping and > re-acquiring the vecs lock. The map reload logic is completely handled in this > function. > > The various _tick() functions don't loop over mpvec any more; they are now > just called for a single mpp, and they simply return true if a map reload is > required. The actual reload action differs: if missing_uev_wait_tick() > requests a reload, it needs to be a full update_map() (which calls > adopt_paths()), whereas in the other cases, reload_and_sync_map() is sufficient. > Patch 12 changes the reload action for the ghost delay tick. > > Patch 13 removes maps if that are not found in the kernel any more. This > obsoletes the map garbage collector. Unlike the logic in v1, we won't remove > maps on arbitrary error conditions any more. > > Changes v1 -> v2: > > Removed patch 3 and 4 of v1 and replaced them by an alternative approach. > Instead of allowing map and path removal in the checker loop, the kernel sync > is moved towards the end. > > Patch 5 ff. in v1 contained a bug in the new checker_finished() function. > If one "tick" function returned true, the other might not be executed any > more. In v2, all tick functions are executed, and the action to be taken is > selected according to the combined results. Also, we won't call > reload_and_sync_map() when we've already called update_map(). > > Patch 13 is new in v2. Patch 8 from v1 has been moved after patch 13. > > (In the thread following the review of v1, I mistakenly wrote about an > upcoming "v4" of this patch set. That was wrong, I meant this v2 here). > > Reviews & comments welcome. > > Regards > Martin > > [1] https://lore.kernel.org/dm-devel/20241205035638.1218953-1-bmarzins@redhat.com/ > [2] https://github.com/opensvc/multipath-tools/issues/105 > [3] https://lore.kernel.org/dm-devel/Z1iUekRg8sai8HLT@redhat.com/ > > > Martin Wilck (14): > multipathd: don't reload map in update_mpp_prio() > multipathd: remove dm_get_info() call from refresh_multipath() > multipathd: sync maps at end of checkerloop > multipathd: quickly re-sync if a map is inconsistent > multipathd: move yielding for waiters to start of checkerloop > multipathd: add checker_finished() > multipathd: move "tick" calls into checker_finished() > multipathd: don't call reload_and_sync_map() from > deferred_failback_tick() > multipathd: move retry_count_tick() into existing mpvec loop > multipathd: don't call update_map() from missing_uev_wait_tick() > multipathd: don't call udpate_map() from ghost_delay_tick() > multipathd: only call reload_and_sync_map() when ghost delay expires > multipathd: remove non-existent maps in checkerloop > multipathd: remove mpvec_garbage_collector() > > libmultipath/structs.h | 2 +- > libmultipath/structs_vec.c | 1 - > multipathd/main.c | 287 +++++++++++++++---------------------- > 3 files changed, 118 insertions(+), 172 deletions(-) > > -- > 2.47.0
This patch set goes on top of Ben's set [1] for github issue 105 [2]. The first patch implements the remark I had on patch 2 on Ben's set, the 2nd is a minor cleanup. Patch 3 moves the sync_mpp() call from the beginning to the end of the checkerloop, as suggested by Ben in [3]. If an inconsitency is found (mpp->need_reload), we reload the map and schedule another sync for the next tick (patch 4). Patch 5 ff. reshuffle the code in checkerloop(). There is now one function, checker_finished(), that takes all actions that need to be done with the vecs lock taken after the checkers have finished. checkerloop() enters this function immediately when the checkers have finished, without dropping and re-acquiring the vecs lock. The map reload logic is completely handled in this function. The various _tick() functions don't loop over mpvec any more; they are now just called for a single mpp, and they simply return true if a map reload is required. The actual reload action differs: if missing_uev_wait_tick() requests a reload, it needs to be a full update_map() (which calls adopt_paths()), whereas in the other cases, reload_and_sync_map() is sufficient. Patch 12 changes the reload action for the ghost delay tick. Patch 13 removes maps if that are not found in the kernel any more. This obsoletes the map garbage collector. Unlike the logic in v1, we won't remove maps on arbitrary error conditions any more. Changes v1 -> v2: Removed patch 3 and 4 of v1 and replaced them by an alternative approach. Instead of allowing map and path removal in the checker loop, the kernel sync is moved towards the end. Patch 5 ff. in v1 contained a bug in the new checker_finished() function. If one "tick" function returned true, the other might not be executed any more. In v2, all tick functions are executed, and the action to be taken is selected according to the combined results. Also, we won't call reload_and_sync_map() when we've already called update_map(). Patch 13 is new in v2. Patch 8 from v1 has been moved after patch 13. (In the thread following the review of v1, I mistakenly wrote about an upcoming "v4" of this patch set. That was wrong, I meant this v2 here). Reviews & comments welcome. Regards Martin [1] https://lore.kernel.org/dm-devel/20241205035638.1218953-1-bmarzins@redhat.com/ [2] https://github.com/opensvc/multipath-tools/issues/105 [3] https://lore.kernel.org/dm-devel/Z1iUekRg8sai8HLT@redhat.com/ Martin Wilck (14): multipathd: don't reload map in update_mpp_prio() multipathd: remove dm_get_info() call from refresh_multipath() multipathd: sync maps at end of checkerloop multipathd: quickly re-sync if a map is inconsistent multipathd: move yielding for waiters to start of checkerloop multipathd: add checker_finished() multipathd: move "tick" calls into checker_finished() multipathd: don't call reload_and_sync_map() from deferred_failback_tick() multipathd: move retry_count_tick() into existing mpvec loop multipathd: don't call update_map() from missing_uev_wait_tick() multipathd: don't call udpate_map() from ghost_delay_tick() multipathd: only call reload_and_sync_map() when ghost delay expires multipathd: remove non-existent maps in checkerloop multipathd: remove mpvec_garbage_collector() libmultipath/structs.h | 2 +- libmultipath/structs_vec.c | 1 - multipathd/main.c | 287 +++++++++++++++---------------------- 3 files changed, 118 insertions(+), 172 deletions(-)