Re: multipathd:checkerloop() - why not check path state before calling pathinfo(), which calls path priority callouts?

[David Wysochanski <davidw@netapp.com>]

[-- Attachment #1: Type: text/plain, Size: 2510 bytes --]

egoggin@emc.com wrote:
>
> On Wednesday, March 01, 2006 2:55 PM David Wysochans wrote
>
> >
> > Looking through the mulitpathd code, in particular
> > checkerloop() and the
> > below snippit.  I'm wondering why the path state isn't
> > checked and then the
> > pathinfo() not called if the path state is PATH_DOWN (or
> > maybe !PATH_UP).
> > Seems like we shouldn't be trying to refresh the priority if
> > we know the
> > path is bad anyway, since the callouts all issue scsi
> > passthru commands,
> > which will always fail.  Is this right or am I missing something?
>
> I think you are definitely right, but ...
>
> Keep in mind that some of the multipath path checkers set the
> multipath user state to PATH_DOWN even though the scsi pass
> through io succeeded.  So some scsi pass through commands may
> succeed, including path priority checking, path health testing,
> uid generation, and serial number retrieval even though the
> path's multipath state is PATH_DOWN.  So I think another reason
> to not issue the path priority update for a path in the PATH_DOWN
> state is that the updated priority value isn't even used.
>
> The calculation of a path group's priority does not include paths
> in a PATH_DOWN state.  See select_path_group() in
> libmultipath/switchgroup.c.  So there is no need to check if the
> priority of a path in a PATH_DOWN state has changed.
>
> But, I would instead patch pathinfo() in libmultipath/discovery.c
> to not call get_prio() if the path's state is PATH_DOWN.  Doing
> so will catch all other cases of calls to pathinfo() with
> DI_PRIO (like in need_switch_pathgroup() in multipathd/main.c).
>  
>
Ok, attached is the 1-line patch against libmultipath/discovery.c
Should apply cleanly against latest source.


> >
> > Thanks.
> >
> >
> >
> >             /*
> >              * path prio refreshing
> >              */
> >             condlog(4, "path prio refresh");
> >             pathinfo(pp, conf->hwtable, DI_PRIO);
> >
> >             if (need_switch_pathgroup(pp->mpp, 0)) {
> >                 if (pp->mpp->pgfailback > 0 &&
> >                     pp->mpp->failback_tick <= 0)
> >                     pp->mpp->failback_tick =
> >                         pp->mpp->pgfailback + 1;
> >                 else if (pp->mpp->pgfailback ==
> >                         -FAILBACK_IMMEDIATE)
> >                     switch_pathgroup(pp->mpp);
> >             }
>
> -- 
> dm-devel mailing list
> dm-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/dm-devel
>


[-- Attachment #2: dont-call-get-prio-if-path-down.patch --]
[-- Type: text/x-patch, Size: 347 bytes --]

--- libmultipath/discovery.c.orig	2006-03-01 18:29:38.000000000 -0500
+++ libmultipath/discovery.c	2006-03-01 18:33:10.000000000 -0500
@@ -652,7 +652,7 @@ pathinfo (struct path *pp, vector hwtabl
 	/*
 	 * get path prio
 	 */
-	if (mask & DI_PRIO) {
+	if (mask & DI_PRIO && pp->state != PATH_DOWN) {
 		if (!pp->getprio)
 			select_getprio(pp);
 

[-- Attachment #3: Type: text/plain, Size: 0 bytes --]