[dm-devel] 答复: [RFC] unreleased lock after handler failure

[Michael Wang]

Hi, Tang

On 10/12/2016 10:33 AM, tang.junhui at zte.com.cn wrote:
> Hi, Michael Wang
> 
> This patch looks well for me, cleanup_lock() should be called without
> affectedby the return value of h->fn().

Thanks for the quick respond, I'll submit a formal patch then :-)

Regards,
Michael Wang

> 
> Regards,
> Tang
> 
> 
> 
> 发件人:         Michael Wang <yun.wang at profitbricks.com>
> 收件人:         christophe.varoqui at opensvc.com, dm-devel at redhat.com,
> 日期:         2016/10/12 16:11
> 主题:        [dm-devel] [RFC] unreleased lock after handler failure
> 发件人:        dm-devel-bounces at redhat.com
> ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
> 
> 
> 
> Hi, folks
> 
> While we are testing with the latest multipathd, we encounter issue that once
> 'del map' failed, all the following operation will show 'error -1 receiving packet'
> whatever how long the timeout has been set (we have applied the latest fix which
> make sure the poll will last for 10 seconds).
> 
> After some debugging we found that inside parse_cmd(), the pthread_cleanup_pop()
> rely on '!r' as indicator for locked or not, while this will be overwritten if
> the handler return failed (1 in our case), and then the unlock will be missed.
> 
> After applied below fix the problem got solved, although the del map failed, the
> other operation can still works.
> 
> Could you please help to review whether this is really the problem to be fixed
> like that?
> 
> Regards,
> Michael Wang
> 
> 
> 
> diff --git a/multipathd/cli.c b/multipathd/cli.c
> index e8a9384..50161be 100644
> --- a/multipathd/cli.c
> +++ b/multipathd/cli.c
> @@ -481,6 +481,7 @@ parse_cmd (char * cmd, char ** reply, int * len, void * data, int timeout )
>                tmo.tv_sec = 0;
>        }
>        if (h->locked) {
> +               int locked = 0;
>                struct vectors * vecs = (struct vectors *)data;
> 
>                pthread_cleanup_push(cleanup_lock, &vecs->lock);
> @@ -491,10 +492,11 @@ parse_cmd (char * cmd, char ** reply, int * len, void * data, int timeout )
>                        r = 0;
>                }
>                if (r == 0) {
> +                       locked = 1;
>                        pthread_testcancel();
>                        r = h->fn(cmdvec, reply, len, data);
>                }
> -               pthread_cleanup_pop(!r);
> +               pthread_cleanup_pop(locked);
>        } else
>                r = h->fn(cmdvec, reply, len, data);
>        free_keys(cmdvec);
> 
> --
> dm-devel mailing list
> dm-devel at redhat.com
> https://www.redhat.com/mailman/listinfo/dm-devel
>