ARM architecture lacks support for pselect() and ppoll()

[Scott James Remnant (Canonical)]

The ARM architecture in the upstream kernel lacks support for the pselect() and ppoll() system calls.

pselect is defined in POSIX.1g, and in POSIX.1-2001, while ppoll is Linux specific but matches the intent of pselect.

The difference between select() and pselect() & poll() and ppoll() is that the latter system calls accept an additional sigmask parameter. Effectively they are equivalent to:

  sigprocmask (SIG_SETMASK, &mask, &oldmask);
  select (...);
  sigprocmask (SIG_SETMASK, &oldmask, NULL);

*EXCEPT* that this is performed atomically!

When these syscalls do not exist, glibc emulates them with the above code - which defeats the entire object of their existance.

Consider when the mask is empty, and the normal process mark blocks SIGCHLD. With pselect()/ppoll() you are guaranteed to only have the SIGCHLD delivered while in the syscall, and thus guaranteed that any child death will terminate the syscall with -EINTR.

With the non-atomic code, the SIGCHLD signal may be delivered between the sigprocmask() and select() system calls, in which case the select will NOT terminate. Software will randomly hang in select and not reap its children.

In some cases, the software may be self-imposing a limit on the number of its children. If the children die at the wrong point, the daemon may hang indefinitely - since it will not process the death.

One such process that uses ppoll() and limits its children is udev!

[Michael Casadevall (mcasadevall)]

Taking a look at the kernel source, the syscall id numbers for both ppoll and pselect are properly reserved for ARM. The pselect function (sys_pselect7) is defined in fs/select.c; it is a C function with no specific arch dependent bits, but does require the sigmask_set_restore function to be available (threading is partially arch specific). This is a simple stub which sets the necessary flag (it lives in arch/*arch*/include/thread_info.h on architectures where it is defined, and sets the necessary flag in that architectures flag control block).

A couple greps of the source say that this flag is only used in a small bit of the thread block control code, used to push or pop the saved sigmask from the control block; implementing it shouldn't been too difficult, although it is possible that there is more to implementation than my greps can pick up.

glibc does not autodetect the presence of this call (possibly because depending on the architecture, the number of arguments change (since some arches can't hand 7 argument syscalls), so it will also have be modified to use the kernel's ppoll/psystem mechanisms once they are implemented in the kernel.

[Scott James Remnant (Canonical) (canonical-scott)]

On Fri, 2009-01-23 at 21:52 +0000, Michael Casadevall wrote:

> glibc does not autodetect the presence of this call (possibly because
> depending on the architecture, the number of arguments change (since
> some arches can't hand 7 argument syscalls), so it will also have be
> modified to use the kernel's ppoll/psystem mechanisms once they are
> implemented in the kernel.
>
This bit's not true.

glibc first tries the ppoll() syscall, and if that fails with ENOSYS it
will fall back to a generic implementation (that has all the race
conditions that ppoll() was designed to avoid!)

The only change to glibc would be to remove the #undef __ASSUME_PPOLL
line from ports/sysdeps/unix/sysv/linux/arm/kernel-features.h, this
disables the fallback code (you'd version guard it as in generic).

Scott
--
Scott James Remnant
<email address hidden>

[Amit Kucheria (amitk)]

Could we please have a sample program that will trigger this bug? That carries more weight when posting to the kernel lists than just asking why it isn't implemented.

/Amit

[Amit Kucheria (amitk)]

Ohh, and I haven't received any reply to my query on the ARM lists yet.

[Scott James Remnant (Canonical) (canonical-scott)]

On Thu, 2009-02-19 at 12:54 +0000, Amit Kucheria wrote:

> Could we please have a sample program that will trigger this bug? That
> carries more weight when posting to the kernel lists than just asking
> why it isn't implemented.
>
It's a race condition. It's difficult to write a program guaranteed to
trigger it every time.

Scott
--
Scott James Remnant
<email address hidden>

[Amit Kucheria (amitk)]

Sample program from Scott to trigger the problem

< Keybuk> http://people.ubuntu.com/~scott/childspin.c
< Keybuk> This will demonstrate the race
< Keybuk> on a machine with a properly implemented pselect(), you will just get continuous .s
< Keybuk> on a machine with a race condition, it will hang
< Keybuk> on a machine with a proper pselect(), you can emulate the hang by compiling -DUSE_SELECT

[Scott James Remnant (Canonical) (canonical-scott)]

As the program demonstrates, the biggest problem is missed SIGCHLDs.

I'm actually quite surprised by how fast the program demonstrates the problem, I was actually expecting you to need to run it for a minute or two before a hang happens. For me, it hangs within mere hundreds of iterations.

The worst result of this hang would be that udev did not get woken up because of an important SIGCHLD such as one of the tests it runs on the root filesystem. This is actually the most likely scenario since the root filesystem device is often one of the very last that udev processes, and there would be no further children dying after to wake it up harder.

The failure would then be that udev would timeout the handler for the root filesystem device, likely causing /dev/sda1 to not be created.

Given the speed at which this can be replicated with the test program, and the tendancy for the root filesystem to be the last device, I'd guess you're looking at such a failure in the order of every thousand boots.

(Or if you booted a thousand netbooks, one of them would likely fail to boot and drop to a "root filesystem not found" prompt)

[Pete Graner (pgraner)]

Decided to release as an update to Jaunty once the syscalls are implemented.

[Pete Graner (pgraner)]

Sample release note text:

The ARM port currently does not support the ppoll() & pselect() system syscalls. This will exhibit itself by failing to mount the root filesystem. This is a race condition and retrying the boot will usually clear the problem. The work to add the syscalls is currently underway and will be delivered as an update to the Jaunty kernel sometime after release.

[Loïc Minier (lool)]

Joseph S. Myers kindly pointed me at http://www.spinics.net/lists/arm-kernel/msg38114.html which just one of the implementations which were proposed for these syscalls. Can we consider its technical quality and whether it helps passing the test case?

[Dave Martin (dave-martin-arm)]

Note that epoll_pwait is also affected (fairly easy to spot in asm/unistd.h)

I may have now experienced an occurrence of this race bug: namely, init stopped respawning terminal logins for no obvious reason. init was definitely waiting in select, but unfortunately I accidentally rebooted the system as a side-effect of waking init up. I'll try to get a better understanding of the symptoms if this happens to me again...

[Steve Langasek (vorlon)]

Added to the release notes:

Occasional hangs possible on ARM architecture

The ARM port currently does not support the ppoll() & pselect() system syscalls, which will sometimes cause a failure to mount the root filesystem on boot. This is a race condition, and retrying the boot will usually clear the problem. The work to add the syscalls is currently underway and will be delivered as an update to the Ubuntu 9.04 kernel some time after release.

[Loïc Minier (lool)]

I can't start an installed versatile image in qemu; switching to sysvinit fixes the problem for me.

[Dave Martin (dave-martin-arm)]

A clarification in relation to my historical comments on this thread (https://bugs.launchpad.net/ubuntu/+source/linux/+bug/319729/comments/11), the problem I observed where init sometimes stops respawning terminal logins is _not_ caused by a pselect-style race. It's a separate issue to do with hot upgrading of libc and upstart: https://bugs.launchpad.net/ubuntu/+source/glibc/+bug/348346

But the other problems described on the thread still sound genuine.

[Amit Kucheria (amitk)]

On 09 Jun 16, Dave Martin wrote:
> A clarification in relation to my historical comments on this thread
> (https://bugs.launchpad.net/ubuntu/+source/linux/+bug/319729/comments/11),
> the problem I observed where init sometimes stops respawning terminal
> logins is _not_ caused by a pselect-style race. It's a separate issue
> to do with hot upgrading of libc and upstart:
> https://bugs.launchpad.net/ubuntu/+source/glibc/+bug/348346
>
> But the other problems described on the thread still sound genuine.

Thanks for the info. I finally have time to work on this bug again now.
It is my number one priority.

[Amit Kucheria (amitk)]

Number one priority obviously got reprioritized :-/

Anyways, I finally have a patch that I would appreciate some testing on. Please apply the attached patch to a recent kernel (2.6.30+) and try the test program (childspin.c) attached to this bug on an arm board. You ought to be able to test this on any arm board (imx51, dove, beagle, etc.)

There is a precompiled kernel for imx51 at http://people.canonical.com/~amitk/mx51/ppoll-linux-image-2.6.31-100-imx51_2.6.31-100.8_armel.deb.

[Amit Kucheria (amitk)]

The bug does not exist in the Karmic 'master' branch since arm support is in soc-specific topic branches in Karmic.

[Amit Kucheria (amitk)]

Jaunty does not have support for the Marvell Dove platform

[Jeremy Kerr (jk-ozlabs)]

Just tried the attached build, and the childspin example still hangs. However, it looks like it's waiting somewhere in do_exit, rather than the previous deadlock, which seems to be waiting in poll_schedule_timeout

Will see if I can find more about what's happening here.

[Jeremy Kerr (jk-ozlabs)]

Looks like the glibc syscall wrapper is getting in the way here; the test run in my previous comment was still using select() even though pselect is available. Once I get hold of some hardware, will do some hacking with the wrapper components.

[Amit Kucheria (amitk)]

We've survived this far w/o the patch. Now Jeremy is looking at the glibc side as well. Marking down the importance.

[Scott James Remnant (Canonical) (canonical-scott)]

On Tue, 2009-10-13 at 13:27 +0000, Amit Kucheria wrote:

> We've survived this far w/o the patch. Now Jeremy is looking at the
> glibc side as well. Marking down the importance.
>
udev is a lot less sensitive to this now, the side effect won't be a
stuck boot, just a bunch of zombie worker processes ;-)

Scott
--
Scott James Remnant
<email address hidden>

[Jeremy Kerr (jk-ozlabs)]

Attached a new testcase. Rather than use the libc pselect() wrapper, the new testcase has its own wrapper for pselect.

Compile with:

 gcc -o childspin -DUSE_PSELECT_WRAPPER -Wall childspin.c

Running this on a patched kernel (from Amit's comment 16), the program does not show the blocking behaviour. So, looks like the patch works as expected.

Since the eglibc sources will Do The Right Thing when __NR_pselect6 is #defined, a rebuild of the eglibc package against the new kernel headers should provide the correct libc wrapper.

[Andy Whitcroft (apw)]

Lots of status updates but the overall upshot is this is now targetted to karmic-updates across the board, and to lucid-alpha1 across the board.

[Amit Kucheria (amitk)]

Marking won't fix for Jaunty. This will be in Lucid from a different patch from upstream.

[Paul Larson (pwlars)]

Are there any plans for an SRU here since Karmic should still benefit from this?

[Amit Kucheria (amitk)]

Scott mentioned that upstart is not as dependent on ppoll/pselect as it once was so it shouldn't be a problem anymore.

The ppoll/pselect patch is now upstream in 2.6.32 (369842658a36bcea28ecb643ba4bdb53919330dd). If we feel the need, it can be cherry-picked into the Karmic ARM kernels (Marvell and Freescale).

For Lucid, Marvell is already on 2.6.32, meaning they already have ppoll/pselect support. Freescale is still on 2.6.31 though, so they will need the patch to be cherry-picked from 2.6.32.

[Amit Kucheria (amitk)]

Re-assigning it to Bryan

[Launchpad Janitor (janitor)]

This bug was fixed in the package linux-fsl-imx51 - 2.6.31-605.9

---------------
linux-fsl-imx51 (2.6.31-605.9) lucid; urgency=low

  [ Andy Whitcroft ]

  * Revert "include modules.builtin in the binary debs"
  * include modules.builtin in the binary debs

  [ Bryan Wu ]

  * SAUCE: IMX51: skip ATA PASS THROUGH command for USB storage disk from
    JMicron
    - LP: #499881

  [ Upstream Kernel Changes ]

  * ARM: 5677/1: ARM support for
    TIF_RESTORE_SIGMASK/pselect6/ppoll/epoll_pwait
    - LP: #319729
  * leds/mc13892: Use workqueue for setting LED brightness
    - LP: #531696
  * ARM: 5882/1: ARM: Fix uncompress code compile for different defines of
    flush(void)
    - LP: #319240
  * ARM: 5884/1: arm: Fix DCC console for v7
    - LP: #319240
  * ARM: 5885/1: arm: Flush TLB entries in setup_mm_for_reboot()
    - LP: #319240
  * ARM: 5886/1: arm: Fix cpu_proc_fin() for proc-v7.S and make kexec work
    - LP: #319240
  * ARM: 5888/1: arm: Update comments in cacheflush.h and remove
    unnecessary V6 and V7 comments
    - LP: #319240
  * arm: disable L2 cache in the v7 finish function
  * arm: invalidate TLBs when enabling mmu
 -- Andy Whitcroft <email address hidden> Tue, 09 Mar 2010 13:52:16 +0000

[Rolf Leggewie (r0lf)]

karmic has seen the end of its life and is no longer receiving any updates. Marking the karmic task for this ticket as "Won't Fix".