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

[tang.junhui at zte.com.cn]

Hi, Michael Wang

This patch looks well for me, cleanup_lock() should be called without
affected by the return value of h->fn().

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


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/dm-devel/attachments/20161012/94bde498/attachment.htm>