[PATCH] endless loop in __libc_fork

[Martin Schwidefsky]

Greetings,
Christian Borntraeger found a bug in __libc_fork with a KVM stress on s390:

pid_t
__libc_fork (void)
{
  ...

  /* Run all the registered preparation handlers.  In reverse order.
     While doing this we build up a list of all the entries.  */
  struct fork_handler *runp;
  while ((runp = __fork_handlers) != NULL)
    {
      unsigned int oldval = runp->refcntr;

      if (oldval == 0)
        /* This means some other thread removed the list just after
           the pointer has been loaded.  Try again.  Either the list
           is empty or we can retry it.  */
        continue;
   ...
}

The (oldval == 0) check with the continue is translated to an endless
loop on s390-64:

   16:   e3 60 10 00 00 04       lg      %r6,0(%r1)
   1c:   b9 02 00 66             ltgr    %r6,%r6	# runp != NULL check
   20:   b9 04 00 bf             lgr     %r11,%r15
   24:   41 40 60 28             la      %r4,40(%r6)
   28:   a7 74 00 9f             jne     166 <__libc_fork+0x166>
  ...
  166:   58 30 60 28             l       %r3,40(%r6)
> 16a:   12 33                   ltr     %r3,%r3	# oldval == 0 check
> 16c:   a7 84 ff ff             je      16a <__libc_fork+0x16a> # endless loop

Once you stumbled over the problem it is obvious that a memory barrier
is missing here. See patch below.

-- 
blue skies,
  Martin.

"Reality continues to ruin my life." - Calvin.
--

2008-09-11  Martin Schwidefsky  <schwidefsky@de.ibm.com>

	* sysdeps/unix/sysv/linux/fork.c (__libc_fork): Add memory barrier
	to force runp->refcntr to be read from memory.

diff -urpN libc/nptl/sysdeps/unix/sysv/linux/fork.c libc-s390/nptl/sysdeps/unix/sysv/linux/fork.c
--- libc/nptl/sysdeps/unix/sysv/linux/fork.c	2007-08-07 13:09:01.000000000 +0200
+++ libc-s390/nptl/sysdeps/unix/sysv/linux/fork.c	2008-09-08 13:20:13.000000000 +0200
@@ -64,7 +64,11 @@ __libc_fork (void)
   struct fork_handler *runp;
   while ((runp = __fork_handlers) != NULL)
     {
-      unsigned int oldval = runp->refcntr;
+      unsigned int oldval;
+
+      atomic_full_barrier();
+
+      oldval = runp->refcntr;
 
       if (oldval == 0)
 	/* This means some other thread removed the list just after