* FreeBSD: sleeper using ppoll does not sleep forever @ 2015-03-09 15:18 Mykola Golub 2015-03-09 20:35 ` Eric Wong 0 siblings, 1 reply; 5+ messages in thread From: Mykola Golub @ 2015-03-09 15:18 UTC (permalink / raw) To: cmogstored-public Hi, I have been observing the following assertion failure when starting cmogstored on recent versions of FreeBSD: Assertion failed: (0 && "init_once did not get cancelled"), function init_once, file mnt.c, line 139. On these versions ppoll is available and the crush is observed only when built with HAVE_PPOLL. According to ktrace, ppoll (called by sleeper) returns immediately with EINTR: 86352 cmogstored CALL ppoll(0,0,0,0x63dc28) 86352 cmogstored CALL thr_kill(0x18f47,SIG 32) 86352 cmogstored RET thr_kill 0 86352 cmogstored RET ppoll -1 errno 4 Interrupted system call 86352 cmogstored PSIG SIG 32 caught handler=0x800867f30 mask=0x7ffefeff code=SI_LWP 86352 cmogstored CALL _umtx_op(0x801406800,UMTX_OP_WAIT,0x18f47,0,0) 86352 cmogstored CALL sigreturn(0x7fffdfffdb40) 86352 cmogstored RET sigreturn JUSTRETURN 86352 cmogstored CALL write(0x2,0x7fffdfffd900,0x66) 86352 cmogstored GIO fd 2 wrote 102 bytes "Assertion failed: (0 && "init_once did not get cancelled"), function init_once, file mnt.c, line 13\ 9. " I have workarounded this by building without HAVE_PPOLL. -- Mykola Golub ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: FreeBSD: sleeper using ppoll does not sleep forever 2015-03-09 15:18 FreeBSD: sleeper using ppoll does not sleep forever Mykola Golub @ 2015-03-09 20:35 ` Eric Wong 2015-03-09 20:49 ` [PATCH 1/2] Fix assertion failure during startup Eric Wong 2015-03-10 19:23 ` FreeBSD: sleeper using ppoll does not sleep forever Mykola Golub 0 siblings, 2 replies; 5+ messages in thread From: Eric Wong @ 2015-03-09 20:35 UTC (permalink / raw) To: Mykola Golub; +Cc: cmogstored-public Thanks, there's at least two problems in cmogstored here: 1) The assertion could trigger on any OS due to the lack of retry on EINTR. 2) ppoll is non-POSIX, and is a cancellation point in GNU libc, but perhaps not the FreeBSD libc based on your bug report. We'll switch to pselect which POSIX as a cancellation point. I suggest FreeBSD developers make ppoll a thread cancellation point for GNU/Linux compatibility. So two patches coming in reply to this email... ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/2] Fix assertion failure during startup 2015-03-09 20:35 ` Eric Wong @ 2015-03-09 20:49 ` Eric Wong 2015-03-09 20:49 ` [PATCH 2/2] avoid relying on ppoll as a cancellation point Eric Wong 2015-03-10 19:23 ` FreeBSD: sleeper using ppoll does not sleep forever Mykola Golub 1 sibling, 1 reply; 5+ messages in thread From: Eric Wong @ 2015-03-09 20:49 UTC (permalink / raw) To: cmogstored-public During the initial device scan, it is possible for the waiter to be interrupted while awaiting cancellation. We must account for this on all platforms regardless of whether pselect or ppoll is used. Reported-by: Mykola Golub <trociny@FreeBSD.org> ref: <20150309151851.GC2195@gmail.com> --- cmogstored.h | 2 +- mnt.c | 6 +++++- sig.c | 26 ++++++++++++++++++-------- 3 files changed, 24 insertions(+), 10 deletions(-) diff --git a/cmogstored.h b/cmogstored.h index c6c5291..a7309b5 100644 --- a/cmogstored.h +++ b/cmogstored.h @@ -291,7 +291,7 @@ struct mog_file { extern sigset_t mog_emptyset; void mog_intr_disable(void); void mog_intr_enable(void); -void mog_sleep(long seconds); +int mog_sleep(long seconds); #include "selfwake.h" enum mog_fd_type { diff --git a/mnt.c b/mnt.c index 0de0bb9..ca4bdf8 100644 --- a/mnt.c +++ b/mnt.c @@ -122,6 +122,7 @@ skip: static void * init_once(void *ptr) { struct init_args *ia = ptr; + int err; CHECK(int, 0, pthread_mutex_lock(&by_dev_lock) ); assert(by_dev == NULL && @@ -135,7 +136,10 @@ static void * init_once(void *ptr) CHECK(int, 0, pthread_cond_signal(&ia->cond)); CHECK(int, 0, pthread_mutex_unlock(&ia->cond_lock)); - mog_sleep(-1); /* wait for cancellation */ + /* wait for cancellation, mog_sleep may return ENOMEM or EINTR */ + do { + err = mog_sleep(-1); + } while (err == EINTR || err == ENOMEM); assert(0 && "init_once did not get cancelled"); return NULL; } diff --git a/sig.c b/sig.c index c04117a..cfbffc2 100644 --- a/sig.c +++ b/sig.c @@ -32,23 +32,33 @@ void mog_intr_enable(void) * would increase the size of the executable */ #ifdef HAVE_PPOLL -static void sleeper(struct timespec *tsp, const sigset_t *sigmask) +static int sleeper(struct timespec *tsp, const sigset_t *sigmask) { - if (ppoll(NULL, 0, tsp, sigmask) < 0) - assert((errno == EINTR || errno == ENOMEM) && + int err = 0; + + if (ppoll(NULL, 0, tsp, sigmask) < 0) { + err = errno; + assert((err == EINTR || err == ENOMEM) && "BUG in ppoll usage"); + } + return err; } #else /* PSELECT */ -static void sleeper(struct timespec *tsp, const sigset_t *sigmask) +static int sleeper(struct timespec *tsp, const sigset_t *sigmask) { - if (pselect(0, NULL, NULL, NULL, tsp, sigmask) < 0) - assert((errno == EINTR || errno == ENOMEM) && + int err = 0; + + if (pselect(0, NULL, NULL, NULL, tsp, sigmask) < 0) { + err = errno; + assert((err == EINTR || err == ENOMEM) && "BUG in pselect usage"); + } + return err; } #endif /* PSELECT */ /* thread-safe, interruptible sleep, negative seconds -> sleep forever */ -void mog_sleep(long seconds) +int mog_sleep(long seconds) { struct timespec ts; struct timespec *tsp; @@ -61,5 +71,5 @@ void mog_sleep(long seconds) tsp = &ts; } - sleeper(tsp, &mog_emptyset); + return sleeper(tsp, &mog_emptyset); } -- EW ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/2] avoid relying on ppoll as a cancellation point 2015-03-09 20:49 ` [PATCH 1/2] Fix assertion failure during startup Eric Wong @ 2015-03-09 20:49 ` Eric Wong 0 siblings, 0 replies; 5+ messages in thread From: Eric Wong @ 2015-03-09 20:49 UTC (permalink / raw) To: cmogstored-public While glibc supports ppoll, ppoll is not standardized and apparently is not a cancellation point in some versions FreeBSD based on Mykola Golub's bug report. Reported-by: Mykola Golub <trociny@FreeBSD.org> ref: <20150309151851.GC2195@gmail.com> --- sig.c | 22 ++++------------------ 1 file changed, 4 insertions(+), 18 deletions(-) diff --git a/sig.c b/sig.c index cfbffc2..7e1c200 100644 --- a/sig.c +++ b/sig.c @@ -27,27 +27,14 @@ void mog_intr_enable(void) CHECK(int, 0, pthread_sigmask(SIG_SETMASK, &mog_emptyset, NULL)); } -/* - * favor ppoll if available since this is our only pselect user and - * would increase the size of the executable - */ -#ifdef HAVE_PPOLL -static int sleeper(struct timespec *tsp, const sigset_t *sigmask) -{ - int err = 0; - - if (ppoll(NULL, 0, tsp, sigmask) < 0) { - err = errno; - assert((err == EINTR || err == ENOMEM) && - "BUG in ppoll usage"); - } - return err; -} -#else /* PSELECT */ static int sleeper(struct timespec *tsp, const sigset_t *sigmask) { int err = 0; + /* + * pselect is a cancellation point, + * ppoll is not POSIX and is only a cancellation point on glibc. + */ if (pselect(0, NULL, NULL, NULL, tsp, sigmask) < 0) { err = errno; assert((err == EINTR || err == ENOMEM) && @@ -55,7 +42,6 @@ static int sleeper(struct timespec *tsp, const sigset_t *sigmask) } return err; } -#endif /* PSELECT */ /* thread-safe, interruptible sleep, negative seconds -> sleep forever */ int mog_sleep(long seconds) -- EW ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: FreeBSD: sleeper using ppoll does not sleep forever 2015-03-09 20:35 ` Eric Wong 2015-03-09 20:49 ` [PATCH 1/2] Fix assertion failure during startup Eric Wong @ 2015-03-10 19:23 ` Mykola Golub 1 sibling, 0 replies; 5+ messages in thread From: Mykola Golub @ 2015-03-10 19:23 UTC (permalink / raw) To: Eric Wong; +Cc: cmogstored-public On Mon, Mar 09, 2015 at 08:35:43PM +0000, Eric Wong wrote: > Thanks, there's at least two problems in cmogstored here: > > 1) The assertion could trigger on any OS due to the lack of retry > on EINTR. > > 2) ppoll is non-POSIX, and is a cancellation point in GNU libc, > but perhaps not the FreeBSD libc based on your bug report. > We'll switch to pselect which POSIX as a cancellation point. > > I suggest FreeBSD developers make ppoll a thread cancellation > point for GNU/Linux compatibility. > > So two patches coming in reply to this email... Cool! Works here. Thanks. -- Mykola Golub ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2015-03-10 19:23 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-03-09 15:18 FreeBSD: sleeper using ppoll does not sleep forever Mykola Golub 2015-03-09 20:35 ` Eric Wong 2015-03-09 20:49 ` [PATCH 1/2] Fix assertion failure during startup Eric Wong 2015-03-09 20:49 ` [PATCH 2/2] avoid relying on ppoll as a cancellation point Eric Wong 2015-03-10 19:23 ` FreeBSD: sleeper using ppoll does not sleep forever Mykola Golub
Code repositories for project(s) associated with this public inbox https://yhbt.net/cmogstored.git/ This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).