lib: remove select(), allow pthreads to poke poll() #555
Conversation
Continous Integration Result: SUCCESSFULCongratulations, this patch passed basic tests Tested-by: NetDEF / OpenSourceRouting.org CI System CI System Testrun URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-727/ This is a comment from an EXPERIMENTAL automated CI system. |
lib/thread.c
Outdated
| @@ -378,13 +384,14 @@ thread_master_create (void) | |||
| rv->timer->update = rv->background->update = thread_timer_update; | |||
| rv->spin = true; | |||
| rv->handle_signals = true; | |||
| pipe (rv->io_pipe); | |||
| fcntl (rv->io_pipe[0], F_SETFL, fcntl(rv->io_pipe[0], F_GETFL, 0) | O_NONBLOCK); | |||
There was a problem hiding this comment.
There's a function in lib/network.c called set_nonblocking for that, maybe you want to use it.
When scheduling a task onto a thread master owned by another pthread, we need to lock the thread master's mutex. However, if the pthread which owns that thread master is in poll(), we could be stuck waiting for a very long time. To solve this, we copy all data poll() needs and unlock during poll(). To break the target pthread out of poll(), thread_master has gained a pipe whose reading end is passed into poll(). After an event that requires immediate action by the target pthread, a byte is written into the pipe in order to wake it up. This commit also removes select(), as poll() is present on every supported platform and does not have an upper limit on file descriptors. Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
Continous Integration Result: SUCCESSFULCongratulations, this patch passed basic tests Tested-by: NetDEF / OpenSourceRouting.org CI System CI System Testrun URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-737/ This is a comment from an EXPERIMENTAL automated CI system. |
| @@ -40,6 +41,12 @@ DEFINE_MTYPE_STATIC(LIB, THREAD_STATS, "Thread stats") | |||
| #include <mach/mach_time.h> | |||
| #endif | |||
|
|
|||
| #define AWAKEN(m) \ | |||
There was a problem hiding this comment.
This isn't used anywhere ... is this patch actually complete?
(Would've been better to put the select() removal in a separate patch, very confusing to review like this...)
There was a problem hiding this comment.
It is used in multiple places, but looks like it snuck out of funcname_thread_add_read_write in a force push somewhere...and yes, but the pipe poke related changes are actually quite small, just the bits in fd_poll and thread_master_create.
@@ -878,6 +779,8 @@ funcname_thread_add_timer_timeval (struct thread_master *m,
}
}
pthread_mutex_unlock (&thread->mtx);
+
+ AWAKEN (m);
}
pthread_mutex_unlock (&m->mtx);
and funcname_thread_add_event as well, one moment and I'll add it back to rw
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
Continous Integration Result: SUCCESSFULCongratulations, this patch passed basic tests Tested-by: NetDEF / OpenSourceRouting.org CI System CI System Testrun URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-766/ This is a comment from an EXPERIMENTAL automated CI system. CLANG Static Analyzer Summary
Bug comparison statistics:
143 Static Analyzer issues remaining.See details at |
|
@eqvinox needs to relook |
|
replaced by #633 |
When scheduling a task onto a thread master owned by another pthread, we
need to lock the thread master's mutex. However, if the pthread which
owns that thread master is in poll(), we could be stuck waiting for a
very long time. To solve this, we copy all data poll() needs and unlock
during poll(). To break the target pthread out of poll(), thread_master
has gained a pipe whose reading end is passed into poll(). After an event
that requires immediate action by the target pthread, a byte is written
into the pipe in order to wake it up.
This commit also removes select(), as poll() is present on every supported
platform and does not have an upper limit on file descriptors.
Signed-off-by: Quentin Young qlyoung@cumulusnetworks.com