* [PATCH 0/2] Two multipath-tools bug fixes
@ 2016-10-14 15:33 Bart Van Assche
2016-10-14 15:34 ` [PATCH 1/2] multipathd: Avoid "socket operation on non-socket" errors Bart Van Assche
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Bart Van Assche @ 2016-10-14 15:33 UTC (permalink / raw)
To: Christophe Varoqui; +Cc: device-mapper development
Hello Christophe,
Please consider the two patches in this series for inclusion in the
multipath-tools repository. The first patch fixes a recently introduced
bug and the second patch fixes a longstanding bug.
Thanks,
Bart.
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH 1/2] multipathd: Avoid "socket operation on non-socket" errors 2016-10-14 15:33 [PATCH 0/2] Two multipath-tools bug fixes Bart Van Assche @ 2016-10-14 15:34 ` Bart Van Assche 2016-10-14 15:35 ` [PATCH 2/2] multipathd: Avoid that a deadlock is triggered sporadically during shutdown Bart Van Assche 2016-10-16 7:52 ` [PATCH 0/2] Two multipath-tools bug fixes Christophe Varoqui 2 siblings, 0 replies; 6+ messages in thread From: Bart Van Assche @ 2016-10-14 15:34 UTC (permalink / raw) To: Christophe Varoqui; +Cc: device-mapper development Changing write_all() into write() is safe for files since the POSIX standard guarantees that write() writes the entire buffer except if the disk is full, a resource limit is encountered or if interrupted by a signal. See also http://pubs.opengroup.org/onlinepubs/9699919799/functions/write.html. Fixes: commit 810082e7a8cf ("libmultipath, multipathd: Rework SIGPIPE handling") Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com> --- libmultipath/alias.c | 2 +- libmultipath/file.c | 2 +- libmultipath/wwids.c | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/libmultipath/alias.c b/libmultipath/alias.c index b86843a..12afef8 100644 --- a/libmultipath/alias.c +++ b/libmultipath/alias.c @@ -219,7 +219,7 @@ allocate_binding(int fd, char *wwid, int id, char *prefix) strerror(errno)); return NULL; } - if (write_all(fd, buf, strlen(buf)) != strlen(buf)){ + if (write(fd, buf, strlen(buf)) != strlen(buf)){ condlog(0, "Cannot write binding to bindings file : %s", strerror(errno)); /* clear partial write */ diff --git a/libmultipath/file.c b/libmultipath/file.c index 74cde64..e4951c9 100644 --- a/libmultipath/file.c +++ b/libmultipath/file.c @@ -158,7 +158,7 @@ open_file(char *file, int *can_write, char *header) goto fail; /* If file is empty, write the header */ size_t len = strlen(header); - if (write_all(fd, header, len) != len) { + if (write(fd, header, len) != len) { condlog(0, "Cannot write header to file %s : %s", file, strerror(errno)); diff --git a/libmultipath/wwids.c b/libmultipath/wwids.c index babf149..bc70a27 100644 --- a/libmultipath/wwids.c +++ b/libmultipath/wwids.c @@ -71,7 +71,7 @@ write_out_wwid(int fd, char *wwid) { strerror(errno)); return -1; } - if (write_all(fd, buf, strlen(buf)) != strlen(buf)) { + if (write(fd, buf, strlen(buf)) != strlen(buf)) { condlog(0, "cannot write wwid to wwids file : %s", strerror(errno)); if (ftruncate(fd, offset)) @@ -110,7 +110,7 @@ replace_wwids(vector mp) goto out_file; } len = strlen(WWIDS_FILE_HEADER); - if (write_all(fd, WWIDS_FILE_HEADER, len) != len) { + if (write(fd, WWIDS_FILE_HEADER, len) != len) { condlog(0, "Can't write wwid file header : %s", strerror(errno)); /* cleanup partially written header */ -- 2.10.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] multipathd: Avoid that a deadlock is triggered sporadically during shutdown 2016-10-14 15:33 [PATCH 0/2] Two multipath-tools bug fixes Bart Van Assche 2016-10-14 15:34 ` [PATCH 1/2] multipathd: Avoid "socket operation on non-socket" errors Bart Van Assche @ 2016-10-14 15:35 ` Bart Van Assche 2016-10-20 22:28 ` Xose Vazquez Perez 2016-10-16 7:52 ` [PATCH 0/2] Two multipath-tools bug fixes Christophe Varoqui 2 siblings, 1 reply; 6+ messages in thread From: Bart Van Assche @ 2016-10-14 15:35 UTC (permalink / raw) To: Christophe Varoqui; +Cc: device-mapper development 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; -- 2.10.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] multipathd: Avoid that a deadlock is triggered sporadically during shutdown 2016-10-14 15:35 ` [PATCH 2/2] multipathd: Avoid that a deadlock is triggered sporadically during shutdown Bart Van Assche @ 2016-10-20 22:28 ` Xose Vazquez Perez 2016-10-20 22:47 ` Bart Van Assche 0 siblings, 1 reply; 6+ messages in thread From: Xose Vazquez Perez @ 2016-10-20 22:28 UTC (permalink / raw) To: Bart Van Assche; +Cc: device-mapper development 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] multipathd: Avoid that a deadlock is triggered sporadically during shutdown 2016-10-20 22:28 ` Xose Vazquez Perez @ 2016-10-20 22:47 ` Bart Van Assche 0 siblings, 0 replies; 6+ messages in thread From: Bart Van Assche @ 2016-10-20 22:47 UTC (permalink / raw) To: Xose Vazquez Perez; +Cc: device-mapper development 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 0/2] Two multipath-tools bug fixes 2016-10-14 15:33 [PATCH 0/2] Two multipath-tools bug fixes Bart Van Assche 2016-10-14 15:34 ` [PATCH 1/2] multipathd: Avoid "socket operation on non-socket" errors Bart Van Assche 2016-10-14 15:35 ` [PATCH 2/2] multipathd: Avoid that a deadlock is triggered sporadically during shutdown Bart Van Assche @ 2016-10-16 7:52 ` Christophe Varoqui 2 siblings, 0 replies; 6+ messages in thread From: Christophe Varoqui @ 2016-10-16 7:52 UTC (permalink / raw) To: Bart Van Assche; +Cc: device-mapper development [-- Attachment #1.1: Type: text/plain, Size: 347 bytes --] Merged. Thanks. On Fri, Oct 14, 2016 at 5:33 PM, Bart Van Assche <bart.vanassche@sandisk.com > wrote: > Hello Christophe, > > Please consider the two patches in this series for inclusion in the > multipath-tools repository. The first patch fixes a recently introduced bug > and the second patch fixes a longstanding bug. > > Thanks, > > Bart. > [-- Attachment #1.2: Type: text/html, Size: 675 bytes --] [-- Attachment #2: Type: text/plain, Size: 0 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-10-20 22:47 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-10-14 15:33 [PATCH 0/2] Two multipath-tools bug fixes Bart Van Assche 2016-10-14 15:34 ` [PATCH 1/2] multipathd: Avoid "socket operation on non-socket" errors Bart Van Assche 2016-10-14 15:35 ` [PATCH 2/2] multipathd: Avoid that a deadlock is triggered sporadically during shutdown Bart Van Assche 2016-10-20 22:28 ` Xose Vazquez Perez 2016-10-20 22:47 ` Bart Van Assche 2016-10-16 7:52 ` [PATCH 0/2] Two multipath-tools bug fixes Christophe Varoqui
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.