| Message ID | 20240423222536.2096486-4-bmarzins@redhat.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Delegated to: | Benjamin Marzinski |
| Headers | show |
| Series | multipath: fix hang in flush_map_nopaths | expand |
On Tue, 2024-04-23 at 18:25 -0400, Benjamin Marzinski wrote: > Now that flush_map_nopaths(), its only caller, needs to deal with > whether or not a remove is deferred, dm_flush_map_nopaths() > doesn't need to worry if LIBDM_API_DEFERRED is defined. It can just > trust that the caller took care of making sure deferred_remove is > correct. > > Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com> > --- > libmultipath/devmapper.c | 19 ------------------- > libmultipath/devmapper.h | 3 ++- > libmultipath/libmultipath.version | 3 +-- > 3 files changed, 3 insertions(+), 22 deletions(-) > > diff --git a/libmultipath/devmapper.c b/libmultipath/devmapper.c > index 24d03e83..dc8c4d17 100644 > --- a/libmultipath/devmapper.c > +++ b/libmultipath/devmapper.c > @@ -1140,25 +1140,6 @@ int _dm_flush_map (const char * mapname, int > need_sync, int deferred_remove, > return DM_FLUSH_FAIL; > } > > -#ifdef LIBDM_API_DEFERRED > - > -int > -dm_flush_map_nopaths(const char * mapname, int deferred_remove) > -{ > - return _dm_flush_map(mapname, 1, deferred_remove, 0, 0); > -} > - > -#else > - > -int > -dm_flush_map_nopaths(const char * mapname, > - int deferred_remove __attribute__((unused))) > -{ > - return _dm_flush_map(mapname, 1, 0, 0, 0); > -} > - > -#endif > - > int dm_flush_maps (int retries) > { > int r = DM_FLUSH_FAIL; > diff --git a/libmultipath/devmapper.h b/libmultipath/devmapper.h > index 2987a1b5..8599ee98 100644 > --- a/libmultipath/devmapper.h > +++ b/libmultipath/devmapper.h > @@ -61,7 +61,8 @@ enum { > > int partmap_in_use(const char *name, void *data); > int _dm_flush_map (const char *, int, int, int, int); > -int dm_flush_map_nopaths(const char * mapname, int deferred_remove); > +#define dm_flush_map_nopaths(mapname, deferred) \ > + _dm_flush_map(mapname, 1, deferred, 0, 0) Perhaps we can do away with dm_flush_map_nopaths entirely. > #define dm_flush_map(mapname) _dm_flush_map(mapname, 1, 0, 0, 0) > #define dm_flush_map_nosync(mapname) _dm_flush_map(mapname, 0, 0, 0, > 0) > #define dm_suspend_and_flush_map(mapname, retries) \ > diff --git a/libmultipath/libmultipath.version > b/libmultipath/libmultipath.version > index e070f296..806676bf 100644 > --- a/libmultipath/libmultipath.version > +++ b/libmultipath/libmultipath.version > @@ -43,7 +43,7 @@ LIBMPATHCOMMON_1.0.0 { > put_multipath_config; > }; > > -LIBMULTIPATH_23.0.0 { > +LIBMULTIPATH_24.0.0 { > global: > /* symbols referenced by multipath and multipathd */ > add_foreign; > @@ -74,7 +74,6 @@ global: > dm_enablegroup; > dm_fail_path; > _dm_flush_map; > - dm_flush_map_nopaths; > dm_flush_maps; > dm_geteventnr; > dm_get_info;
On Wed, Apr 24, 2024 at 06:40:36PM +0200, Martin Wilck wrote: > On Tue, 2024-04-23 at 18:25 -0400, Benjamin Marzinski wrote: > > Now that flush_map_nopaths(), its only caller, needs to deal with > > whether or not a remove is deferred, dm_flush_map_nopaths() > > doesn't need to worry if LIBDM_API_DEFERRED is defined. It can just > > trust that the caller took care of making sure deferred_remove is > > correct. > > > > Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com> > > --- > > libmultipath/devmapper.c | 19 ------------------- > > libmultipath/devmapper.h | 3 ++- > > libmultipath/libmultipath.version | 3 +-- > > 3 files changed, 3 insertions(+), 22 deletions(-) > > > > diff --git a/libmultipath/devmapper.c b/libmultipath/devmapper.c > > index 24d03e83..dc8c4d17 100644 > > --- a/libmultipath/devmapper.c > > +++ b/libmultipath/devmapper.c > > @@ -1140,25 +1140,6 @@ int _dm_flush_map (const char * mapname, int > > need_sync, int deferred_remove, > > return DM_FLUSH_FAIL; > > } > > > > -#ifdef LIBDM_API_DEFERRED > > - > > -int > > -dm_flush_map_nopaths(const char * mapname, int deferred_remove) > > -{ > > - return _dm_flush_map(mapname, 1, deferred_remove, 0, 0); > > -} > > - > > -#else > > - > > -int > > -dm_flush_map_nopaths(const char * mapname, > > - int deferred_remove __attribute__((unused))) > > -{ > > - return _dm_flush_map(mapname, 1, 0, 0, 0); > > -} > > - > > -#endif > > - > > int dm_flush_maps (int retries) > > { > > int r = DM_FLUSH_FAIL; > > diff --git a/libmultipath/devmapper.h b/libmultipath/devmapper.h > > index 2987a1b5..8599ee98 100644 > > --- a/libmultipath/devmapper.h > > +++ b/libmultipath/devmapper.h > > @@ -61,7 +61,8 @@ enum { > > > > int partmap_in_use(const char *name, void *data); > > int _dm_flush_map (const char *, int, int, int, int); > > -int dm_flush_map_nopaths(const char * mapname, int deferred_remove); > > +#define dm_flush_map_nopaths(mapname, deferred) \ > > + _dm_flush_map(mapname, 1, deferred, 0, 0) > > Perhaps we can do away with dm_flush_map_nopaths entirely. You mean by directly calling _dm_flush_map instead, since there's only one caller of dm_flush_map_nopaths()? Sure. -Ben > > > #define dm_flush_map(mapname) _dm_flush_map(mapname, 1, 0, 0, 0) > > #define dm_flush_map_nosync(mapname) _dm_flush_map(mapname, 0, 0, 0, > > 0) > > #define dm_suspend_and_flush_map(mapname, retries) \ > > diff --git a/libmultipath/libmultipath.version > > b/libmultipath/libmultipath.version > > index e070f296..806676bf 100644 > > --- a/libmultipath/libmultipath.version > > +++ b/libmultipath/libmultipath.version > > @@ -43,7 +43,7 @@ LIBMPATHCOMMON_1.0.0 { > > put_multipath_config; > > }; > > > > -LIBMULTIPATH_23.0.0 { > > +LIBMULTIPATH_24.0.0 { > > global: > > /* symbols referenced by multipath and multipathd */ > > add_foreign; > > @@ -74,7 +74,6 @@ global: > > dm_enablegroup; > > dm_fail_path; > > _dm_flush_map; > > - dm_flush_map_nopaths; > > dm_flush_maps; > > dm_geteventnr; > > dm_get_info;
diff --git a/libmultipath/devmapper.c b/libmultipath/devmapper.c index 24d03e83..dc8c4d17 100644 --- a/libmultipath/devmapper.c +++ b/libmultipath/devmapper.c @@ -1140,25 +1140,6 @@ int _dm_flush_map (const char * mapname, int need_sync, int deferred_remove, return DM_FLUSH_FAIL; } -#ifdef LIBDM_API_DEFERRED - -int -dm_flush_map_nopaths(const char * mapname, int deferred_remove) -{ - return _dm_flush_map(mapname, 1, deferred_remove, 0, 0); -} - -#else - -int -dm_flush_map_nopaths(const char * mapname, - int deferred_remove __attribute__((unused))) -{ - return _dm_flush_map(mapname, 1, 0, 0, 0); -} - -#endif - int dm_flush_maps (int retries) { int r = DM_FLUSH_FAIL; diff --git a/libmultipath/devmapper.h b/libmultipath/devmapper.h index 2987a1b5..8599ee98 100644 --- a/libmultipath/devmapper.h +++ b/libmultipath/devmapper.h @@ -61,7 +61,8 @@ enum { int partmap_in_use(const char *name, void *data); int _dm_flush_map (const char *, int, int, int, int); -int dm_flush_map_nopaths(const char * mapname, int deferred_remove); +#define dm_flush_map_nopaths(mapname, deferred) \ + _dm_flush_map(mapname, 1, deferred, 0, 0) #define dm_flush_map(mapname) _dm_flush_map(mapname, 1, 0, 0, 0) #define dm_flush_map_nosync(mapname) _dm_flush_map(mapname, 0, 0, 0, 0) #define dm_suspend_and_flush_map(mapname, retries) \ diff --git a/libmultipath/libmultipath.version b/libmultipath/libmultipath.version index e070f296..806676bf 100644 --- a/libmultipath/libmultipath.version +++ b/libmultipath/libmultipath.version @@ -43,7 +43,7 @@ LIBMPATHCOMMON_1.0.0 { put_multipath_config; }; -LIBMULTIPATH_23.0.0 { +LIBMULTIPATH_24.0.0 { global: /* symbols referenced by multipath and multipathd */ add_foreign; @@ -74,7 +74,6 @@ global: dm_enablegroup; dm_fail_path; _dm_flush_map; - dm_flush_map_nopaths; dm_flush_maps; dm_geteventnr; dm_get_info;
Now that flush_map_nopaths(), its only caller, needs to deal with whether or not a remove is deferred, dm_flush_map_nopaths() doesn't need to worry if LIBDM_API_DEFERRED is defined. It can just trust that the caller took care of making sure deferred_remove is correct. Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com> --- libmultipath/devmapper.c | 19 ------------------- libmultipath/devmapper.h | 3 ++- libmultipath/libmultipath.version | 3 +-- 3 files changed, 3 insertions(+), 22 deletions(-)