diff mbox series

[11/13] multipathd: don't call update_map() from missing_uev_wait_tick()

Message ID 20241206233617.382200-12-mwilck@suse.com (mailing list archive)
State Not Applicable, archived
Delegated to: Benjamin Marzinski
Headers show
Series multipathd: More map reload handling, and checkerloop work | expand

Commit Message

Martin Wilck Dec. 6, 2024, 11:36 p.m. UTC
Instead, check for missing uevents in the existing mpvec loop.
Note that if the uevent tick expires, we need to call update_map() rather than
reload_and_sync_map(), because the paths have not been added to the multipath
(see wait_for_udev handling ev_add_path()).

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 multipathd/main.c | 40 +++++++++++++++++++---------------------
 1 file changed, 19 insertions(+), 21 deletions(-)

Comments

Benjamin Marzinski Dec. 10, 2024, 11:13 p.m. UTC | #1
On Sat, Dec 07, 2024 at 12:36:15AM +0100, Martin Wilck wrote:
> Instead, check for missing uevents in the existing mpvec loop.
> Note that if the uevent tick expires, we need to call update_map() rather than
> reload_and_sync_map(), because the paths have not been added to the multipath
> (see wait_for_udev handling ev_add_path()).
> 
> Signed-off-by: Martin Wilck <mwilck@suse.com>
> ---
>  multipathd/main.c | 40 +++++++++++++++++++---------------------
>  1 file changed, 19 insertions(+), 21 deletions(-)
> 
> diff --git a/multipathd/main.c b/multipathd/main.c
> index 4cf5493..4478cc9 100644
> --- a/multipathd/main.c
> +++ b/multipathd/main.c
> @@ -2011,29 +2011,19 @@ followover_should_failback(struct multipath *mpp)
>  	return 0;
>  }
>  
> -static void
> -missing_uev_wait_tick(struct vectors *vecs)
> +/* Returns true if update_map() needs to be called */
> +static bool
> +missing_uev_wait_tick(struct multipath *mpp, bool *timed_out)
>  {
> -	struct multipath * mpp;
> -	int i;
> -	int timed_out = 0;
> +	if (mpp->wait_for_udev && --mpp->uev_wait_tick <= 0) {
> +		int wait = mpp->wait_for_udev;
>  
> -	vector_foreach_slot (vecs->mpvec, mpp, i) {
> -		if (mpp->wait_for_udev && --mpp->uev_wait_tick <= 0) {
> -			timed_out = 1;
> -			condlog(0, "%s: timeout waiting on creation uevent. enabling reloads", mpp->alias);
> -			if (mpp->wait_for_udev > 1 &&
> -			    update_map(mpp, vecs, 0)) {
> -				/* update_map removed map */
> -				i--;
> -				continue;
> -			}
> -			mpp->wait_for_udev = 0;
> -		}
> +		mpp->wait_for_udev = 0;
> +		*timed_out = true;
> +		condlog(0, "%s: timeout waiting on creation uevent. enabling reloads", mpp->alias);
> +		return wait > 1;
>  	}
> -
> -	if (timed_out && !need_to_delay_reconfig(vecs))
> -		unblock_reconfigure();
> +	return false;
>  }
>  
>  static void
> @@ -2947,9 +2937,16 @@ update_paths(struct vectors *vecs, int *num_paths_p, time_t start_secs)
>  static void checker_finished(struct vectors *vecs)
>  {
>  	struct multipath *mpp;
> +	bool uev_timed_out = false;
>  	int i;
>  
>  	vector_foreach_slot(vecs->mpvec, mpp, i) {
> +		if (missing_uev_wait_tick(mpp, &uev_timed_out) &&
> +		    update_map(mpp, vecs, 0)) {
> +			/* multipath device deleted */
> +			i--;
> +			continue;
> +		}

Looking at this made me think we should probably be adding a check
in reload_and_sync_map() for mpp->wait_for_udev. If it's no-zero,
we should set it to 2 (and we might want to make it symoblic too)
and skip the reload. Otherwise we'll be reloading when we shouldn't be.

>  		if ((update_mpp_prio(mpp) ||
>  		     (mpp->need_reload && mpp->synced_count > 0) ||
>  		     deferred_failback_tick(mpp)) &&
> @@ -2959,7 +2956,8 @@ static void checker_finished(struct vectors *vecs)
>  		else
>  			retry_count_tick(mpp);
>  	}
> -	missing_uev_wait_tick(vecs);
> +	if (uev_timed_out && !need_to_delay_reconfig(vecs))
> +		unblock_reconfigure();
>  	ghost_delay_tick(vecs);
>  	partial_retrigger_tick(vecs->pathvec);
>  }
> -- 
> 2.47.0
diff mbox series

Patch

diff --git a/multipathd/main.c b/multipathd/main.c
index 4cf5493..4478cc9 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -2011,29 +2011,19 @@  followover_should_failback(struct multipath *mpp)
 	return 0;
 }
 
-static void
-missing_uev_wait_tick(struct vectors *vecs)
+/* Returns true if update_map() needs to be called */
+static bool
+missing_uev_wait_tick(struct multipath *mpp, bool *timed_out)
 {
-	struct multipath * mpp;
-	int i;
-	int timed_out = 0;
+	if (mpp->wait_for_udev && --mpp->uev_wait_tick <= 0) {
+		int wait = mpp->wait_for_udev;
 
-	vector_foreach_slot (vecs->mpvec, mpp, i) {
-		if (mpp->wait_for_udev && --mpp->uev_wait_tick <= 0) {
-			timed_out = 1;
-			condlog(0, "%s: timeout waiting on creation uevent. enabling reloads", mpp->alias);
-			if (mpp->wait_for_udev > 1 &&
-			    update_map(mpp, vecs, 0)) {
-				/* update_map removed map */
-				i--;
-				continue;
-			}
-			mpp->wait_for_udev = 0;
-		}
+		mpp->wait_for_udev = 0;
+		*timed_out = true;
+		condlog(0, "%s: timeout waiting on creation uevent. enabling reloads", mpp->alias);
+		return wait > 1;
 	}
-
-	if (timed_out && !need_to_delay_reconfig(vecs))
-		unblock_reconfigure();
+	return false;
 }
 
 static void
@@ -2947,9 +2937,16 @@  update_paths(struct vectors *vecs, int *num_paths_p, time_t start_secs)
 static void checker_finished(struct vectors *vecs)
 {
 	struct multipath *mpp;
+	bool uev_timed_out = false;
 	int i;
 
 	vector_foreach_slot(vecs->mpvec, mpp, i) {
+		if (missing_uev_wait_tick(mpp, &uev_timed_out) &&
+		    update_map(mpp, vecs, 0)) {
+			/* multipath device deleted */
+			i--;
+			continue;
+		}
 		if ((update_mpp_prio(mpp) ||
 		     (mpp->need_reload && mpp->synced_count > 0) ||
 		     deferred_failback_tick(mpp)) &&
@@ -2959,7 +2956,8 @@  static void checker_finished(struct vectors *vecs)
 		else
 			retry_count_tick(mpp);
 	}
-	missing_uev_wait_tick(vecs);
+	if (uev_timed_out && !need_to_delay_reconfig(vecs))
+		unblock_reconfigure();
 	ghost_delay_tick(vecs);
 	partial_retrigger_tick(vecs->pathvec);
 }