[dm-devel] multipathd:checkerloop() - why not check path stat e before calling pathinfo(), which calls path priority callouts?

[egoggin at emc.com]

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).
 
> 
> 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);
>             }