20 Jul, 2007

1 commit

  • This patch completes Linus's wish that the fault return codes be made into
    bit flags, which I agree makes everything nicer. This requires requires
    all handle_mm_fault callers to be modified (possibly the modifications
    should go further and do things like fault accounting in handle_mm_fault --
    however that would be for another patch).

    [akpm@linux-foundation.org: fix alpha build]
    [akpm@linux-foundation.org: fix s390 build]
    [akpm@linux-foundation.org: fix sparc build]
    [akpm@linux-foundation.org: fix sparc64 build]
    [akpm@linux-foundation.org: fix ia64 build]
    Signed-off-by: Nick Piggin
    Cc: Richard Henderson
    Cc: Ivan Kokshaysky
    Cc: Russell King
    Cc: Ian Molton
    Cc: Bryan Wu
    Cc: Mikael Starvik
    Cc: David Howells
    Cc: Yoshinori Sato
    Cc: "Luck, Tony"
    Cc: Hirokazu Takata
    Cc: Geert Uytterhoeven
    Cc: Roman Zippel
    Cc: Greg Ungerer
    Cc: Matthew Wilcox
    Cc: Paul Mackerras
    Cc: Benjamin Herrenschmidt
    Cc: Heiko Carstens
    Cc: Martin Schwidefsky
    Cc: Paul Mundt
    Cc: Kazumoto Kojima
    Cc: Richard Curnow
    Cc: William Lee Irwin III
    Cc: "David S. Miller"
    Cc: Jeff Dike
    Cc: Paolo 'Blaisorblade' Giarrusso
    Cc: Miles Bader
    Cc: Chris Zankel
    Acked-by: Kyle McMartin
    Acked-by: Haavard Skinnemoen
    Acked-by: Ralf Baechle
    Acked-by: Andi Kleen
    Signed-off-by: Andrew Morton
    [ Still apparently needs some ARM and PPC loving - Linus ]
    Signed-off-by: Linus Torvalds

    Nick Piggin
     

17 Jul, 2007

1 commit

  • The recent PRIVATE and REQUEUE_PI changes to the futex code made it hard to
    read. Tidy it up.

    Signed-off-by: Thomas Gleixner
    Cc: Ingo Molnar
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Thomas Gleixner
     

25 Jun, 2007

1 commit

  • The return value of futex_find_get_task() needs to be -ESRCH in case
    that the search fails. This was part of the original futex fixes and
    got accidentally dropped, when the futex-tidy-up patch was split out.

    Signed-off-by: Thomas Gleixner
    Cc: Ingo Molnar
    Cc: Stable Team
    Signed-off-by: Linus Torvalds

    Thomas Gleixner
     

19 Jun, 2007

1 commit

  • This reverts commit d0aa7a70bf03b9de9e995ab272293be1f7937822.

    It not only introduced user space visible changes to the futex syscall,
    it is also non-functional and there is no way to fix it proper before
    the 2.6.22 release.

    The breakage report ( http://lkml.org/lkml/2007/5/12/17 ) went
    unanswered, and unfortunately it turned out that the concept is not
    feasible at all. It violates the rtmutex semantics badly by introducing
    a virtual owner, which hacks around the coupling of the user-space
    pi_futex and the kernel internal rt_mutex representation.

    At the moment the only safe option is to remove it fully as it contains
    user-space visible changes to broken kernel code, which we do not want
    to expose in the 2.6.22 release.

    The patch reverts the original patch mostly 1:1, but contains a couple
    of trivial manual cleanups which were necessary due to patches, which
    touched the same area of code later.

    Verified against the glibc tests and my own PI futex tests.

    Signed-off-by: Thomas Gleixner
    Acked-by: Ingo Molnar
    Acked-by: Ulrich Drepper
    Cc: Pierre Peiffer
    Signed-off-by: Linus Torvalds

    Thomas Gleixner
     

09 Jun, 2007

1 commit

  • 1. New entries can be added to tsk->pi_state_list after task completed
    exit_pi_state_list(). The result is memory leakage and deadlocks.

    2. handle_mm_fault() is called under spinlock. The result is obvious.

    3. results in self-inflicted deadlock inside glibc.
    Sometimes futex_lock_pi returns -ESRCH, when it is not expected
    and glibc enters to for(;;) sleep() to simulate deadlock. This problem
    is quite obvious and I think the patch is right. Though it looks like
    each "if" in futex_lock_pi() got some stupid special case "else if". :-)

    4. sometimes futex_lock_pi() returns -EDEADLK,
    when nobody has the lock. The reason is also obvious (see comment
    in the patch), but correct fix is far beyond my comprehension.
    I guess someone already saw this, the chunk:

    if (rt_mutex_trylock(&q.pi_state->pi_mutex))
    ret = 0;

    is obviously from the same opera. But it does not work, because the
    rtmutex is really taken at this point: wake_futex_pi() of previous
    owner reassigned it to us. My fix works. But it looks very stupid.
    I would think about removal of shift of ownership in wake_futex_pi()
    and making all the work in context of process taking lock.

    From: Thomas Gleixner

    Fix 1) Avoid the tasklist lock variant of the exit race fix by adding
    an additional state transition to the exit code.

    This fixes also the issue, when a task with recursive segfaults
    is not able to release the futexes.

    Fix 2) Cleanup the lookup_pi_state() failure path and solve the -ESRCH
    problem finally.

    Fix 3) Solve the fixup_pi_state_owner() problem which needs to do the fixup
    in the lock protected section by using the in_atomic userspace access
    functions.

    This removes also the ugly lock drop / unqueue inside of fixup_pi_state()

    Fix 4) Fix a stale lock in the error path of futex_wake_pi()

    Added some error checks for verification.

    The -EDEADLK problem is solved by the rtmutex fixups.

    Signed-off-by: Thomas Gleixner
    Acked-by: Ingo Molnar
    Cc: Steven Rostedt
    Cc: Ulrich Drepper
    Cc: Eric Dumazet
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Alexey Kuznetsov
     

10 May, 2007

4 commits

  • Analysis of current linux futex code :
    --------------------------------------

    A central hash table futex_queues[] holds all contexts (futex_q) of waiting
    threads.

    Each futex_wait()/futex_wait() has to obtain a spinlock on a hash slot to
    perform lookups or insert/deletion of a futex_q.

    When a futex_wait() is done, calling thread has to :

    1) - Obtain a read lock on mmap_sem to be able to validate the user pointer
    (calling find_vma()). This validation tells us if the futex uses
    an inode based store (mapped file), or mm based store (anonymous mem)

    2) - compute a hash key

    3) - Atomic increment of reference counter on an inode or a mm_struct

    4) - lock part of futex_queues[] hash table

    5) - perform the test on value of futex.
    (rollback is value != expected_value, returns EWOULDBLOCK)
    (various loops if test triggers mm faults)

    6) queue the context into hash table, release the lock got in 4)

    7) - release the read_lock on mmap_sem

    8) Eventually unqueue the context (but rarely, as this part  may be done
    by the futex_wake())

    Futexes were designed to improve scalability but current implementation has
    various problems :

    - Central hashtable :

    This means scalability problems if many processes/threads want to use
    futexes at the same time.
    This means NUMA unbalance because this hashtable is located on one node.

    - Using mmap_sem on every futex() syscall :

    Even if mmap_sem is a rw_semaphore, up_read()/down_read() are doing atomic
    ops on mmap_sem, dirtying cache line :
    - lot of cache line ping pongs on SMP configurations.

    mmap_sem is also extensively used by mm code (page faults, mmap()/munmap())
    Highly threaded processes might suffer from mmap_sem contention.

    mmap_sem is also used by oprofile code. Enabling oprofile hurts threaded
    programs because of contention on the mmap_sem cache line.

    - Using an atomic_inc()/atomic_dec() on inode ref counter or mm ref counter:
    It's also a cache line ping pong on SMP. It also increases mmap_sem hold time
    because of cache misses.

    Most of these scalability problems come from the fact that futexes are in
    one global namespace. As we use a central hash table, we must make sure
    they are all using the same reference (given by the mm subsystem). We
    chose to force all futexes be 'shared'. This has a cost.

    But fact is POSIX defined PRIVATE and SHARED, allowing clear separation,
    and optimal performance if carefuly implemented. Time has come for linux
    to have better threading performance.

    The goal is to permit new futex commands to avoid :
    - Taking the mmap_sem semaphore, conflicting with other subsystems.
    - Modifying a ref_count on mm or an inode, still conflicting with mm or fs.

    This is possible because, for one process using PTHREAD_PROCESS_PRIVATE
    futexes, we only need to distinguish futexes by their virtual address, no
    matter the underlying mm storage is.

    If glibc wants to exploit this new infrastructure, it should use new
    _PRIVATE futex subcommands for PTHREAD_PROCESS_PRIVATE futexes. And be
    prepared to fallback on old subcommands for old kernels. Using one global
    variable with the FUTEX_PRIVATE_FLAG or 0 value should be OK.

    PTHREAD_PROCESS_SHARED futexes should still use the old subcommands.

    Compatibility with old applications is preserved, they still hit the
    scalability problems, but new applications can fly :)

    Note : the same SHARED futex (mapped on a file) can be used by old binaries
    *and* new binaries, because both binaries will use the old subcommands.

    Note : Vast majority of futexes should be using PROCESS_PRIVATE semantic,
    as this is the default semantic. Almost all applications should benefit
    of this changes (new kernel and updated libc)

    Some bench results on a Pentium M 1.6 GHz (SMP kernel on a UP machine)

    /* calling futex_wait(addr, value) with value != *addr */
    433 cycles per futex(FUTEX_WAIT) call (mixing 2 futexes)
    424 cycles per futex(FUTEX_WAIT) call (using one futex)
    334 cycles per futex(FUTEX_WAIT_PRIVATE) call (mixing 2 futexes)
    334 cycles per futex(FUTEX_WAIT_PRIVATE) call (using one futex)
    For reference :
    187 cycles per getppid() call
    188 cycles per umask() call
    181 cycles per ni_syscall() call

    Signed-off-by: Eric Dumazet
    Pierre Peiffer
    Cc: "Ulrich Drepper"
    Cc: "Nick Piggin"
    Cc: "Ingo Molnar"
    Cc: Rusty Russell
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Eric Dumazet
     
  • This patch provides the futex_requeue_pi functionality, which allows some
    threads waiting on a normal futex to be requeued on the wait-queue of a
    PI-futex.

    This provides an optimization, already used for (normal) futexes, to be used
    with the PI-futexes.

    This optimization is currently used by the glibc in pthread_broadcast, when
    using "normal" mutexes. With futex_requeue_pi, it can be used with
    PRIO_INHERIT mutexes too.

    Signed-off-by: Pierre Peiffer
    Cc: Ingo Molnar
    Cc: Ulrich Drepper
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Pierre Peiffer
     
  • This patch modifies futex_wait() to use an hrtimer + schedule() in place of
    schedule_timeout().

    schedule_timeout() is tick based, therefore the timeout granularity is the
    tick (1 ms, 4 ms or 10 ms depending on HZ). By using a high resolution timer
    for timeout wakeup, we can attain a much finer timeout granularity (in the
    microsecond range). This parallels what is already done for futex_lock_pi().

    The timeout passed to the syscall is no longer converted to jiffies and is
    therefore passed to do_futex() and futex_wait() as an absolute ktime_t
    therefore keeping nanosecond resolution.

    Also this removes the need to pass the nanoseconds timeout part to
    futex_lock_pi() in val2.

    In futex_wait(), if there is no timeout then a regular schedule() is
    performed. Otherwise, an hrtimer is fired before schedule() is called.

    [akpm@linux-foundation.org: fix `make headers_check']
    Signed-off-by: Sebastien Dugue
    Signed-off-by: Pierre Peiffer
    Cc: Ingo Molnar
    Cc: Ulrich Drepper
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Pierre Peiffer
     
  • Today, all threads waiting for a given futex are woken in FIFO order (first
    waiter woken first) instead of priority order.

    This patch makes use of plist (pirotity ordered lists) instead of simple list
    in futex_hash_bucket.

    All non-RT threads are stored with priority MAX_RT_PRIO, causing them to be
    woken last, in FIFO order (RT-threads are woken first, in priority order).

    Signed-off-by: Sebastien Dugue
    Signed-off-by: Pierre Peiffer
    Cc: Ingo Molnar
    Cc: Ulrich Drepper
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Pierre Peiffer
     

09 May, 2007

2 commits

  • LTP test sigaction_16_24 fails, because it expects sem_wait to be restarted
    if SA_RESTART is set. sem_wait is implemented with futex_wait, that
    currently doesn't support being restarted. Ulrich confirms that the call
    should be restartable.

    Implement a restart_block method to handle the relative timeout, and allow
    restarts.

    Signed-off-by: Nick Piggin
    Cc: Ulrich Drepper
    Cc: Rusty Russell
    Cc: Roland McGrath
    Cc: Oleg Nesterov
    Acked-by: Ingo Molnar
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Nick Piggin
     
  • lguest uses the convenient futex infrastructure for inter-domain I/O, so
    expose get_futex_key, get_key_refs (renamed get_futex_key_refs) and
    drop_key_refs (renamed drop_futex_key_refs). Also means we need to expose the
    union that these use.

    No code changes.

    Signed-off-by: Rusty Russell
    Cc: Andi Kleen
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Rusty Russell
     

17 Mar, 2007

1 commit

  • Testing of -rt by IBM uncovered a locking bug in wake_futex_pi(): the PI
    state needs to be locked before we access it.

    Signed-off-by: Ingo Molnar
    Acked-by: Thomas Gleixner
    Cc: Chuck Ebbert
    Cc:
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Ingo Molnar
     

17 Feb, 2007

1 commit

  • - hrtimers did not use the hrtimer_restart enum and relied on the implict
    int representation. Fix the prototypes and the functions using the enums.
    - Use seperate name spaces for the enumerations
    - Convert hrtimer_restart macro to inline function
    - Add comments

    No functional changes.

    [akpm@osdl.org: fix input driver]
    Signed-off-by: Thomas Gleixner
    Signed-off-by: Ingo Molnar
    Cc: john stultz
    Cc: Roman Zippel
    Cc: Dmitry Torokhov
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Thomas Gleixner
     

09 Dec, 2006

1 commit


08 Dec, 2006

5 commits

  • - move some file_operations structs into the .rodata section

    - move static strings from policy_types[] array into the .rodata section

    - fix generic seq_operations usages, so that those structs may be defined
    as "const" as well

    [akpm@osdl.org: couple of fixes]
    Signed-off-by: Helge Deller
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Helge Deller
     
  • When disassembling a kernel I found around over 90 sync Instructions from
    mb, rmb and wmb calls in the kernel and only few of those make any sense to
    me. So here's the first one - I think the wmb() in kernel/futex.c is not
    needed on uniprocessors so should become an smb_wmb().

    Signed-off-by: Ralf Baechle
    Acked-by: Ingo Molnar
    Cc: Thomas Gleixner
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Ralf Baechle
     
  • Check register_filesystem() and kern_mount() return values.

    Cc: Ingo Molnar
    Signed-off-by: Akinobu Mita
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Akinobu Mita
     
  • Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Burman Yan
     
  • Introduce pagefault_{disable,enable}() and use these where previously we did
    manual preempt increments/decrements to make the pagefault handler do the
    atomic thing.

    Currently they still rely on the increased preempt count, but do not rely on
    the disabled preemption, this might go away in the future.

    (NOTE: the extra barrier() in pagefault_disable might fix some holes on
    machines which have too many registers for their own good)

    [heiko.carstens@de.ibm.com: s390 fix]
    Signed-off-by: Peter Zijlstra
    Acked-by: Nick Piggin
    Cc: Martin Schwidefsky
    Signed-off-by: Heiko Carstens
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Peter Zijlstra
     

04 Nov, 2006

1 commit

  • Apparently FUTEX_FD is unfixably racy and nothing uses it (or if it does, it
    shouldn't).

    Add a warning printk, give any remaining users six months to migrate off it.

    Cc: Ulrich Drepper
    Cc: Ingo Molnar
    Acked-by: Thomas Gleixner
    Cc: Rusty Russell
    Cc: Alan Cox
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Andrew Morton
     

11 Oct, 2006

1 commit


02 Oct, 2006

1 commit


30 Sep, 2006

2 commits


09 Sep, 2006

1 commit

  • The current implementation of futex_lock_pi returns -ERESTART_RESTARTBLOCK
    in case that the lock operation has been interrupted by a signal. This
    results in a return of -EINTR to userspace in case there is an handler for
    the signal. This is wrong, because userspace expects that the lock
    function does not return in any case of signal delivery.

    This was not caught by my insufficient test case, but triggered a nasty
    userspace problem in an high load application scenario. Unfortunately also
    glibc does not check for this invalid return value.

    Using -ERSTARTNOINTR makes sure, that the interrupted syscall is restarted.
    The restart block related code can be safely removed, as the possible
    timeout argument is an absolute time value.

    Signed-off-by: Thomas Gleixner
    Acked-by: Ingo Molnar
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Thomas Gleixner
     

28 Aug, 2006

1 commit

  • futex_find_get_task:

    if (p->state == EXIT_ZOMBIE || p->exit_state == EXIT_ZOMBIE)
    return NULL;

    I can't understand this. First, p->state can't be EXIT_ZOMBIE. The
    ->exit_state check looks strange too. Sub-threads or tasks whose ->parent
    ignores SIGCHLD go directly to EXIT_DEAD state (I am ignoring a ptrace
    case). Why EXIT_DEAD tasks should be ok? Yes, EXIT_ZOMBIE is more
    important (a task may stay zombie for a long time), but this doesn't mean
    we should explicitely ignore other EXIT_XXX states.

    Signed-off-by: Oleg Nesterov
    Acked-by: Ingo Molnar
    Cc: Thomas Gleixner
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Oleg Nesterov
     

15 Aug, 2006

1 commit

  • We found this issue last week w/ the -RT kernel, but it seems the same
    issue is in mainline as well.

    Basically it is possible for futex_unlock_pi to return without actually
    freeing the lock. This is due to buggy logic in the use of
    futex_handle_fault() and its attempt argument in a failure case.

    Looking at futex.c the logic is as follows:

    1) In futex_unlock_pi() we start w/ ret=0 and we go down to the first
    futex_atomic_cmpxchg_inatomic(), where we find uval==-EFAULT. We then
    jump to the pi_faulted label.

    2) From pi_faulted: We increment attempt, unlock the sem and hit the
    retry label.

    3) From the retry label, with ret still zero, we again hit EFAULT on the
    first futex_atomic_cmpxchg_inatomic(), and again goto the pi_faulted
    label.

    4) Again from pi_faulted: we increment attempt and enter the
    conditional, where we call futex_handle_fault.

    5) futex_handle_fault fails, and we goto the out_unlock_release_sem
    label.

    6) From out_unlock_release_sem we return, and since ret is still zero,
    we return without error, while never actually unlocking the lock.

    Issue #1: at the first futex_atomic_cmpxchg_inatomic() we should probably
    be setting ret=-EFAULT before jumping to pi_faulted: However in our case
    this doesn't really affect anything, as the glibc we're using ignores the
    error value from futex_unlock_pi().

    Issue #2: Look at futex_handle_fault(), its first conditional will return
    -EFAULT if attempt is >= 2. However, from the "if(attempt++)
    futex_handle_fault(attempt)" logic above, we'll *never* call
    futex_handle_fault when attempt is less then two. So we never get a chance
    to even try to fault the page in.

    The following patch addresses these two issues by 1) Always setting ret to
    -EFAULT if futex_handle_fault fails, and 2) Removing the = in
    futex_handle_fault's (attempt >= 2) check.

    I'm really not sure this is the right fix, but wanted to bring it up so
    folks knew the issue is alive and well in the current -git tree. From
    looking at the git logs the logic was first introduced (then later copied
    to other places) in the following commit almost a year ago:

    http://www.kernel.org/git/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=4732efbeb997189d9f9b04708dc26bf8613ed721;hp=5b039e681b8c5f30aac9cc04385cc94be45d0823

    Cc: Rusty Russell
    Cc: Ingo Molnar
    Acked-by: Thomas Gleixner
    Signed-off-by: Andrew Morton
    Signed-off-by: Greg Kroah-Hartman

    john stultz
     

06 Aug, 2006

1 commit

  • This patch adds a barrier() in futex unqueue_me to avoid aliasing of two
    pointers.

    On my s390x system I saw the following oops:

    Unable to handle kernel pointer dereference at virtual kernel address
    0000000000000000
    Oops: 0004 [#1]
    CPU: 0 Not tainted
    Process mytool (pid: 13613, task: 000000003ecb6ac0, ksp: 00000000366bdbd8)
    Krnl PSW : 0704d00180000000 00000000003c9ac2 (_spin_lock+0xe/0x30)
    Krnl GPRS: 00000000ffffffff 000000003ecb6ac0 0000000000000000 0700000000000000
    0000000000000000 0000000000000000 000001fe00002028 00000000000c091f
    000001fe00002054 000001fe00002054 0000000000000000 00000000366bddc0
    00000000005ef8c0 00000000003d00e8 0000000000144f91 00000000366bdcb8
    Krnl Code: ba 4e 20 00 12 44 b9 16 00 3e a7 84 00 08 e3 e0 f0 88 00 04
    Call Trace:
    ([] unqueue_me+0x40/0xe4)
    [] do_futex+0x33c/0xc40
    [] sys_futex+0x12e/0x144
    [] sysc_noemu+0x10/0x16
    [] 0x2000003741c

    The code in question is:

    static int unqueue_me(struct futex_q *q)
    {
    int ret = 0;
    spinlock_t *lock_ptr;

    /* In the common case we don't take the spinlock, which is nice. */
    retry:
    lock_ptr = q->lock_ptr;
    if (lock_ptr != 0) {
    spin_lock(lock_ptr);
    /*
    * q->lock_ptr can change between reading it and
    * spin_lock(), causing us to take the wrong lock. This
    * corrects the race condition.
    [...]

    and my compiler (gcc 4.1.0) makes the following out of it:

    00000000000003c8 :
    3c8: eb bf f0 70 00 24 stmg %r11,%r15,112(%r15)
    3ce: c0 d0 00 00 00 00 larl %r13,3ce
    3d0: R_390_PC32DBL .rodata+0x2a
    3d4: a7 f1 1e 00 tml %r15,7680
    3d8: a7 84 00 01 je 3da
    3dc: b9 04 00 ef lgr %r14,%r15
    3e0: a7 fb ff d0 aghi %r15,-48
    3e4: b9 04 00 b2 lgr %r11,%r2
    3e8: e3 e0 f0 98 00 24 stg %r14,152(%r15)
    3ee: e3 c0 b0 28 00 04 lg %r12,40(%r11)
    /* write q->lock_ptr in r12 */
    3f4: b9 02 00 cc ltgr %r12,%r12
    3f8: a7 84 00 4b je 48e
    /* if r12 is zero then jump over the code.... */
    3fc: e3 20 b0 28 00 04 lg %r2,40(%r11)
    /* write q->lock_ptr in r2 */
    402: c0 e5 00 00 00 00 brasl %r14,402
    404: R_390_PC32DBL _spin_lock+0x2
    /* use r2 as parameter for spin_lock */

    So the code becomes more or less:
    if (q->lock_ptr != 0) spin_lock(q->lock_ptr)
    instead of
    if (lock_ptr != 0) spin_lock(lock_ptr)

    Which caused the oops from above.
    After adding a barrier gcc creates code without this problem:
    [...] (the same)
    3ee: e3 c0 b0 28 00 04 lg %r12,40(%r11)
    3f4: b9 02 00 cc ltgr %r12,%r12
    3f8: b9 04 00 2c lgr %r2,%r12
    3fc: a7 84 00 48 je 48c
    400: c0 e5 00 00 00 00 brasl %r14,400
    402: R_390_PC32DBL _spin_lock+0x2

    As a general note, this code of unqueue_me seems a bit fishy. The retry logic
    of unqueue_me only works if we can guarantee, that the original value of
    q->lock_ptr is always a spinlock (Otherwise we overwrite kernel memory). We
    know that q->lock_ptr can change. I dont know what happens with the original
    spinlock, as I am not an expert with the futex code.

    Cc: Martin Schwidefsky
    Cc: Rusty Russell
    Acked-by: Ingo Molnar
    Cc: Thomas Gleixner
    Signed-off-by: Christian Borntraeger
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Christian Borntraeger
     

29 Jul, 2006

2 commits

  • Fix robust PI-futexes to be properly unlocked on unexpected exit.

    For this to work the kernel has to know whether a futex is a PI or a
    non-PI one, because the semantics are different. Since the space in
    relevant glibc data structures is extremely scarce, the best solution is
    to encode the 'PI' information in bit 0 of the robust list pointer.
    Existing (non-PI) glibc robust futexes have this bit always zero, so the
    ABI is kept. New glibc with PI-robust-futexes will set this bit.

    Further fixes from Thomas Gleixner

    Signed-off-by: Ingo Molnar
    Signed-off-by: Ulrich Drepper
    Signed-off-by: Thomas Gleixner
    Signed-off-by: Linus Torvalds

    Ingo Molnar
     
  • Fix pi_state->list handling bugs: list handling mishap, locking error.
    Plus add more debug checks and fix a few style issues i noticed while
    debugging this.

    (reported by Ulrich Drepper and Jakub Jelinek.)

    Signed-off-by: Ingo Molnar
    Signed-off-by: Linus Torvalds

    Ingo Molnar
     

11 Jul, 2006

1 commit

  • Calling futex_lock_pi is called with a reference to a non PI futex and
    waiters exist already, lookup_pi_state() oopses due to pi_state == NULL.
    Check this condition and return -EINVAL to userspace.

    Signed-off-by: Thomas Gleixner
    Cc: Ingo Molnar
    Cc: Jakub Jelinek
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Thomas Gleixner
     

04 Jul, 2006

1 commit

  • Teach special (recursive) locking code to the lock validator. Introduces
    double_lock_hb() to unify double- hash-bucket-lock taking.

    Signed-off-by: Ingo Molnar
    Signed-off-by: Arjan van de Ven
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Ingo Molnar
     

02 Jul, 2006

2 commits

  • Fix futex_wake() exit condition bug when handling the robust-list with PI
    futexes on them.

    (reported by Ulrich Drepper, debugged by the lock validator.)

    Signed-off-by: Ingo Molnar
    Cc: Ulrich Drepper
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Ingo Molnar
     
  • lock_queue was getting called essentially twice in a row and was
    continually incrementing the mm_count ref count, thus causing a memory
    leak.

    Dinakar Guniguntala provided a proper fix for the problem that simply grabs
    the spinlock for the hash bucket queue rather than calling lock_queue.

    The second time we do a queue_lock in futex_lock_pi, we really only need to
    take the hash bucket lock.

    Signed-off-by: Dinakar Guniguntala
    Signed-off-by: Vernon Mauery
    Acked-by: Paul E. McKenney
    Signed-off-by: Ingo Molnar
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Vernon Mauery
     

28 Jun, 2006

3 commits

  • In futex_requeue(), when the 2 futexes keys hash to the same bucket, there
    is no need to move the futex_q to the end of the bucket list.

    Signed-off-by: Sebastien Dugue
    Cc: Ingo Molnar
    Cc: Thomas Gleixner
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Sebastien Dugue
     
  • This adds the actual pi-futex implementation, based on rt-mutexes.

    [dino@in.ibm.com: fix an oops-causing race]
    Signed-off-by: Ingo Molnar
    Signed-off-by: Thomas Gleixner
    Signed-off-by: Arjan van de Ven
    Signed-off-by: Dinakar Guniguntala
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Ingo Molnar
     
  • We are pleased to announce "lightweight userspace priority inheritance" (PI)
    support for futexes. The following patchset and glibc patch implements it,
    ontop of the robust-futexes patchset which is included in 2.6.16-mm1.

    We are calling it lightweight for 3 reasons:

    - in the user-space fastpath a PI-enabled futex involves no kernel work
    (or any other PI complexity) at all. No registration, no extra kernel
    calls - just pure fast atomic ops in userspace.

    - in the slowpath (in the lock-contention case), the system call and
    scheduling pattern is in fact better than that of normal futexes, due to
    the 'integrated' nature of FUTEX_LOCK_PI. [more about that further down]

    - the in-kernel PI implementation is streamlined around the mutex
    abstraction, with strict rules that keep the implementation relatively
    simple: only a single owner may own a lock (i.e. no read-write lock
    support), only the owner may unlock a lock, no recursive locking, etc.

    Priority Inheritance - why, oh why???
    -------------------------------------

    Many of you heard the horror stories about the evil PI code circling Linux for
    years, which makes no real sense at all and is only used by buggy applications
    and which has horrible overhead. Some of you have dreaded this very moment,
    when someone actually submits working PI code ;-)

    So why would we like to see PI support for futexes?

    We'd like to see it done purely for technological reasons. We dont think it's
    a buggy concept, we think it's useful functionality to offer to applications,
    which functionality cannot be achieved in other ways. We also think it's the
    right thing to do, and we think we've got the right arguments and the right
    numbers to prove that. We also believe that we can address all the
    counter-arguments as well. For these reasons (and the reasons outlined below)
    we are submitting this patch-set for upstream kernel inclusion.

    What are the benefits of PI?

    The short reply:
    ----------------

    User-space PI helps achieving/improving determinism for user-space
    applications. In the best-case, it can help achieve determinism and
    well-bound latencies. Even in the worst-case, PI will improve the statistical
    distribution of locking related application delays.

    The longer reply:
    -----------------

    Firstly, sharing locks between multiple tasks is a common programming
    technique that often cannot be replaced with lockless algorithms. As we can
    see it in the kernel [which is a quite complex program in itself], lockless
    structures are rather the exception than the norm - the current ratio of
    lockless vs. locky code for shared data structures is somewhere between 1:10
    and 1:100. Lockless is hard, and the complexity of lockless algorithms often
    endangers to ability to do robust reviews of said code. I.e. critical RT
    apps often choose lock structures to protect critical data structures, instead
    of lockless algorithms. Furthermore, there are cases (like shared hardware,
    or other resource limits) where lockless access is mathematically impossible.

    Media players (such as Jack) are an example of reasonable application design
    with multiple tasks (with multiple priority levels) sharing short-held locks:
    for example, a highprio audio playback thread is combined with medium-prio
    construct-audio-data threads and low-prio display-colory-stuff threads. Add
    video and decoding to the mix and we've got even more priority levels.

    So once we accept that synchronization objects (locks) are an unavoidable fact
    of life, and once we accept that multi-task userspace apps have a very fair
    expectation of being able to use locks, we've got to think about how to offer
    the option of a deterministic locking implementation to user-space.

    Most of the technical counter-arguments against doing priority inheritance
    only apply to kernel-space locks. But user-space locks are different, there
    we cannot disable interrupts or make the task non-preemptible in a critical
    section, so the 'use spinlocks' argument does not apply (user-space spinlocks
    have the same priority inversion problems as other user-space locking
    constructs). Fact is, pretty much the only technique that currently enables
    good determinism for userspace locks (such as futex-based pthread mutexes) is
    priority inheritance:

    Currently (without PI), if a high-prio and a low-prio task shares a lock [this
    is a quite common scenario for most non-trivial RT applications], even if all
    critical sections are coded carefully to be deterministic (i.e. all critical
    sections are short in duration and only execute a limited number of
    instructions), the kernel cannot guarantee any deterministic execution of the
    high-prio task: any medium-priority task could preempt the low-prio task while
    it holds the shared lock and executes the critical section, and could delay it
    indefinitely.

    Implementation:
    ---------------

    As mentioned before, the userspace fastpath of PI-enabled pthread mutexes
    involves no kernel work at all - they behave quite similarly to normal
    futex-based locks: a 0 value means unlocked, and a value==TID means locked.
    (This is the same method as used by list-based robust futexes.) Userspace uses
    atomic ops to lock/unlock these mutexes without entering the kernel.

    To handle the slowpath, we have added two new futex ops:

    FUTEX_LOCK_PI
    FUTEX_UNLOCK_PI

    If the lock-acquire fastpath fails, [i.e. an atomic transition from 0 to TID
    fails], then FUTEX_LOCK_PI is called. The kernel does all the remaining work:
    if there is no futex-queue attached to the futex address yet then the code
    looks up the task that owns the futex [it has put its own TID into the futex
    value], and attaches a 'PI state' structure to the futex-queue. The pi_state
    includes an rt-mutex, which is a PI-aware, kernel-based synchronization
    object. The 'other' task is made the owner of the rt-mutex, and the
    FUTEX_WAITERS bit is atomically set in the futex value. Then this task tries
    to lock the rt-mutex, on which it blocks. Once it returns, it has the mutex
    acquired, and it sets the futex value to its own TID and returns. Userspace
    has no other work to perform - it now owns the lock, and futex value contains
    FUTEX_WAITERS|TID.

    If the unlock side fastpath succeeds, [i.e. userspace manages to do a TID ->
    0 atomic transition of the futex value], then no kernel work is triggered.

    If the unlock fastpath fails (because the FUTEX_WAITERS bit is set), then
    FUTEX_UNLOCK_PI is called, and the kernel unlocks the futex on the behalf of
    userspace - and it also unlocks the attached pi_state->rt_mutex and thus wakes
    up any potential waiters.

    Note that under this approach, contrary to other PI-futex approaches, there is
    no prior 'registration' of a PI-futex. [which is not quite possible anyway,
    due to existing ABI properties of pthread mutexes.]

    Also, under this scheme, 'robustness' and 'PI' are two orthogonal properties
    of futexes, and all four combinations are possible: futex, robust-futex,
    PI-futex, robust+PI-futex.

    glibc support:
    --------------

    Ulrich Drepper and Jakub Jelinek have written glibc support for PI-futexes
    (and robust futexes), enabling robust and PI (PTHREAD_PRIO_INHERIT) POSIX
    mutexes. (PTHREAD_PRIO_PROTECT support will be added later on too, no
    additional kernel changes are needed for that). [NOTE: The glibc patch is
    obviously inofficial and unsupported without matching upstream kernel
    functionality.]

    the patch-queue and the glibc patch can also be downloaded from:

    http://redhat.com/~mingo/PI-futex-patches/

    Many thanks go to the people who helped us create this kernel feature: Steven
    Rostedt, Esben Nielsen, Benedikt Spranger, Daniel Walker, John Cooper, Arjan
    van de Ven, Oleg Nesterov and others. Credits for related prior projects goes
    to Dirk Grambow, Inaky Perez-Gonzalez, Bill Huey and many others.

    Clean up the futex code, before adding more features to it:

    - use u32 as the futex field type - that's the ABI
    - use __user and pointers to u32 instead of unsigned long
    - code style / comment style cleanups
    - rename hash-bucket name from 'bh' to 'hb'.

    I checked the pre and post futex.o object files to make sure this
    patch has no code effects.

    Signed-off-by: Ingo Molnar
    Signed-off-by: Thomas Gleixner
    Signed-off-by: Arjan van de Ven
    Cc: Ulrich Drepper
    Cc: Jakub Jelinek
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Ingo Molnar
     

23 Jun, 2006

1 commit

  • Extend the get_sb() filesystem operation to take an extra argument that
    permits the VFS to pass in the target vfsmount that defines the mountpoint.

    The filesystem is then required to manually set the superblock and root dentry
    pointers. For most filesystems, this should be done with simple_set_mnt()
    which will set the superblock pointer and then set the root dentry to the
    superblock's s_root (as per the old default behaviour).

    The get_sb() op now returns an integer as there's now no need to return the
    superblock pointer.

    This patch permits a superblock to be implicitly shared amongst several mount
    points, such as can be done with NFS to avoid potential inode aliasing. In
    such a case, simple_set_mnt() would not be called, and instead the mnt_root
    and mnt_sb would be set directly.

    The patch also makes the following changes:

    (*) the get_sb_*() convenience functions in the core kernel now take a vfsmount
    pointer argument and return an integer, so most filesystems have to change
    very little.

    (*) If one of the convenience function is not used, then get_sb() should
    normally call simple_set_mnt() to instantiate the vfsmount. This will
    always return 0, and so can be tail-called from get_sb().

    (*) generic_shutdown_super() now calls shrink_dcache_sb() to clean up the
    dcache upon superblock destruction rather than shrink_dcache_anon().

    This is required because the superblock may now have multiple trees that
    aren't actually bound to s_root, but that still need to be cleaned up. The
    currently called functions assume that the whole tree is rooted at s_root,
    and that anonymous dentries are not the roots of trees which results in
    dentries being left unculled.

    However, with the way NFS superblock sharing are currently set to be
    implemented, these assumptions are violated: the root of the filesystem is
    simply a dummy dentry and inode (the real inode for '/' may well be
    inaccessible), and all the vfsmounts are rooted on anonymous[*] dentries
    with child trees.

    [*] Anonymous until discovered from another tree.

    (*) The documentation has been adjusted, including the additional bit of
    changing ext2_* into foo_* in the documentation.

    [akpm@osdl.org: convert ipath_fs, do other stuff]
    Signed-off-by: David Howells
    Acked-by: Al Viro
    Cc: Nathan Scott
    Cc: Roland Dreier
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    David Howells
     

01 Apr, 2006

1 commit

  • The futex timeval is not checked for correctness. The change does not
    break existing applications as the timeval is supplied by glibc (and glibc
    always passes a correct value), but the glibc-internal tests for this
    functionality fail.

    Signed-off-by: Thomas Gleixner
    Signed-off-by: Ingo Molnar
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Thomas Gleixner
     

28 Mar, 2006

1 commit

  • - fix: initialize the robust list(s) to NULL in copy_process.

    - doc update

    - cleanup: rename _inuser to _inatomic

    - __user cleanups and other small cleanups

    Signed-off-by: Ingo Molnar
    Cc: Thomas Gleixner
    Cc: Arjan van de Ven
    Cc: Ulrich Drepper
    Cc: Andi Kleen
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Ingo Molnar