| Message ID | 5832949e-aa89-ebf2-11e1-67bbb4d050bf@sandisk.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Delegated to: | Mike Snitzer |
| Headers | show |
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) -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
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
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_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(-)