Re: Bug in pth_sched_eventmanager() -- can't yield to a thread waiting on an fd

Thanks for reporting a bug and providing fix for it. Your fix will solve
problem for an uniprocessor
system, but may impact on SMP performance. Here is what I think a right way
to fix for both cases:

============================================================================
                    /* Filedescriptor I/O */
-                   if (ev->ev_type == PTH_EVENT_FD && !dopoll && (ds ==
descr)) {
+                   if (ev->ev_type == PTH_EVENT_FD && (ds == descr)) {
                        /* filedescriptors are checked later all at once.
                           Here we only assemble them in the fd sets */
                        if (ev->ev_goal & PTH_UNTIL_FD_READABLE)
@@ -1034,10 +1034,6 @@
     /* let select() wait for the read-part of the pipe */
     FD_SET(descr->sigpipe[0], &rfds);

     /* If dopoll is set, handle the thread from ready queue... */
-    if (dopoll)
+    if (m_to_n && dopoll)
       return;


     if (fdmax < descr->sigpipe[0])
         fdmax = descr->sigpipe[0];

- - - - -
Saurabh Desai
POSIX Threading for Linux
IBM  Linux Technology Center
e-mail: [email protected] OR [email protected]
phone: 512-838-2655, T/L: 678-2655
http://oss.software.ibm.com/developerworks/opensource/pthreads



Michael Beach <[email protected]>@www-124.southbury.usf.ibm.com on
08/15/2002 04:35:21 AM

Sent by:    [email protected]


To:    [email protected]
cc:
Subject:    [pthreads-devel] Bug in pth_sched_eventmanager() -- can't yield
       to a thread waiting on an fd


The good news or the bad news first? The bad news you say? Well...

The bad news is that I've just recently been bitten by what I believe to be
a
bug in pth_sched_eventmanager().

The good news is that I think I've fixed it!

Ok, some more details. I'm using NGPT 2.0.1 on a more or less stock SuSE
7.2
installation, but with an appropriately patched 2.4.19 kernel.

The problem relates to the way that the scheduler selects a new thread to
run.

At the end of the scheduler main loop, at around line 700 of pth_sched.c,
pth_scheduler() calls pth_sched_eventmanager() in order to process any
events
which have occurred and to move those threads which are waiting on those
threads from the WAITING queue to the READY queue. If the READY queue is
not
empty, then pth_sched_eventmanager() is called with a flag which indicates
that it should only poll for any events which may have occurred, rather
than
waiting for one to occur. So far so good.

The problem lies in the way that pth_sched_eventmanager() uses that
"dopoll"
flag. Ultimately, what it's used for is to decide whether or not to call
select() with a zero or a non-zero timeout. Presumably if we already have
threads in the READY queue, we want to poll (ie call select() with a zero
timeout) so that if nothing turns up from the select() call we can then go
straight on and schedule a thread that is already in the READY queue.
However
at several points in pth_sched_eventmanager() the "dopoll" flag is tested
and
used to short-circuit parts of the event checking altogether. This I think
is
the heart of the problem. It means that if we have threads which are
currently on the READY queue, then some events will not be checked for,
potentially resulting in starvation problems.

The circumstances in which I experienced this problem involved a CPU bound
thread (call it thread A) which would occasionally call pthread_yield_np()
to
allow other threads to run, and a thread performing a recv() from a socket
(call it thread B). Despite the socket being ready to read (ie data was
arriving from the network), and thread A yielding quite regularly, thread B

would not return from the recv() call. Eventually I tracked this down to
the
problem in the scheduler. When thread A yields, it goes back on the READY
queue, so when it comes time for the scheduler to service events it sets
the
"dopoll" flag since the READY queue is non-empty. However the "dopoll" flag

was causing checking of file descriptor related events to be
short-circuited,
so the socket event which should have woken thread B was never being
checked.
Thus the scheduler just ran thread A again. And again. And again.

I have a simple test case to illustrate this problem. This program creates
a
CPU bound thread (in this case just a tight loop) which regularly yields,
and
a thread which reads from stdin and in reponse writes stuff to stdout. With

the stock standard NGPT 2.0.1, the thread which reads from stdin never
seems
to run, as no output is ever generated.

==================================================================

#include <pthread.h>


void *cpu_bound_thread_entry(void *arg)
{
    while (1) pthread_yield_np();
}


void *stdin_reading_thread_entry(void *arg)
{
    while (1)
    {
        ssize_t n;
        const size_t bufLen = 20;
        char buf[bufLen];
        n = read(0, buf, bufLen);
        if (n > 0)
        {
            write(1, "Read some data ==> ", 19);
            write(1, buf, n);
            write(1, "\n", 1);
        }
        else if (n == 0)
        {
            write(1, "*EOF*\n", 6);
            break;
        }
        else
        {
            write(1, "*ERROR*\n", 8);
            break;
        }
    }
}


int main(int argc, char *argv[])
{
    pthread_t cpu_bound_thread;
    pthread_t stdin_reading_thread;

    pthread_create(&stdin_reading_thread, 0, stdin_reading_thread_entry,
0);
    pthread_create(&cpu_bound_thread, 0, cpu_bound_thread_entry, 0);

    pthread_join(stdin_reading_thread, 0);
    pthread_join(cpu_bound_thread, 0);
}

==================================================================

However if the NGPT library is patched to remove the short-circuiting of
the
event tests based on "dopoll" this program echos the data read as expected.

The patch I applied is as follows...

==================================================================

--- ngpt-2.0.1/pth_sched.c      Sat Aug 10 05:31:32 2002
+++ ngpt-2.0.1-hacks/pth_sched.c        Thu Aug 15 18:17:32 2002
@@ -840,7 +840,7 @@
                    this_occurred = FALSE;

                    /* Filedescriptor I/O */
-                   if (ev->ev_type == PTH_EVENT_FD && !dopoll && (ds ==
descr)) {
+                   if (ev->ev_type == PTH_EVENT_FD) {
                        /* filedescriptors are checked later all at once.
                           Here we only assemble them in the fd sets */
                        if (ev->ev_goal & PTH_UNTIL_FD_READABLE)
@@ -1034,10 +1034,6 @@
     /* let select() wait for the read-part of the pipe */
     FD_SET(descr->sigpipe[0], &rfds);

-    /* If dopoll is set, handle the thread from ready queue... */
-    if (dopoll)
-       return;
-
     if (fdmax < descr->sigpipe[0])
         fdmax = descr->sigpipe[0];

==================================================================

I doubt that this patch is the last word in solving this problem, largely
because I've got no idea as to the details of how the M:N threading model
is
handled, and how all of that relates to bound threads etc etc. In
particular,
in the first change in the patch I've removed the (ds == dsecr) part of the

expression too, but I've no idea if this is correct or not. Also, I've been

working solely on a uniprocessor machine, so I've not been able to verify
its
correct operation in an SMP environment.

However, it does get the test program (and my actual software project!)
running correctly, so I'm sure it is a start!

Regards
M.Beach
_______________________________________________
pthreads-devel mailing list
[email protected]
http://www-124.ibm.com/developerworks/oss/mailman/listinfo/pthreads-devel