Re: [PATCH 2/2] multipathd: Avoid that a deadlock is triggered sporadically during shutdown

From: Bart Van Assche <bart.vanassche@sandisk.com>
To: Xose Vazquez Perez <xose.vazquez@gmail.com>
Cc: device-mapper development <dm-devel@redhat.com>
Subject: Re: [PATCH 2/2] multipathd: Avoid that a deadlock is triggered sporadically during shutdown
Date: Thu, 20 Oct 2016 15:47:53 -0700	[thread overview]
Message-ID: <5816570e-9c96-4845-6f51-1c19e63681a7@sandisk.com> (raw)
In-Reply-To: <4b86f060-4232-1483-8312-a61fcc5b3e95@gmail.com>

On 10/20/2016 03:28 PM, Xose Vazquez Perez wrote:
> On 10/14/2016 05:35 PM, Bart Van Assche wrote:
>
>> pthread_cond_wait() is a thread cancellation point. If a thread that
>> is waiting in pthread_cond_wait() is canceled it is possible that the
>> mutex is re-acquired before the first cancellation cleanup handler
>> is called. In this case the cleanup handler is uevq_stop() and that
>> function locks uevq_lock. Avoid that calling uevq_stop() results in
>> a deadlock due to an attempt to lock a non-recursive mutex recursively.
>>
>> Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
>> ---
>>  libmultipath/uevent.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/libmultipath/uevent.c b/libmultipath/uevent.c
>> index 6247898..85fd2fb 100644
>> --- a/libmultipath/uevent.c
>> +++ b/libmultipath/uevent.c
>> @@ -52,7 +52,7 @@ typedef int (uev_trigger)(struct uevent *, void * trigger_data);
>>
>>  pthread_t uevq_thr;
>>  LIST_HEAD(uevq);
>> -pthread_mutex_t uevq_lock = PTHREAD_MUTEX_INITIALIZER;
>> +pthread_mutex_t uevq_lock = PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP;
>>  pthread_mutex_t *uevq_lockp = &uevq_lock;
>>  pthread_cond_t uev_cond = PTHREAD_COND_INITIALIZER;
>>  pthread_cond_t *uev_condp = &uev_cond;
>>
>
> PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP is not defined in POSIX.
>
> This is going to be problematic in musl-libc based distributions:
> http://wiki.musl-libc.org/wiki/Projects_using_musl#Distributions
>
>
> ~/musl-gcc -O2 -g -pipe -Wall -Wextra -Wformat=2 -Wno-sign-compare -Wno-unused-parameter -Wno-clobbered -Wp,-D_FORTIFY_SOURCE=2 -fstack-protector --param=ssp-buffer-size=4 -fPIC -DLIB_STRING=\"lib64\"
> -DRUN_DIR=\"run\" -I../libmpathcmd -DUSE_SYSTEMD=229 -DLIBDM_API_FLUSH -D_GNU_SOURCE -DLIBDM_API_COOKIE -DLIBUDEV_API_RECVBUF -DLIBDM_API_DEFERRED -c -o uevent.o uevent.c
> uevent.c:55:29: error: â€˜PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NPâ€™ undeclared here (not in a function)
>  pthread_mutex_t uevq_lock = PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP;
>                              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> ../Makefile.inc:74: recipe for target 'uevent.o' failed
> make[1]: [uevent.o] Error 1 (ignored)

Hello Xose,

Thanks for reporting this. I will rework that patch.

Bart.

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel