[lkml]   [2007]   [May]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
    SubjectRe: [PATCH] make cancel_rearming_delayed_work() reliable
    Hello Tejun,

    On 05/16, Tejun Heo wrote:
    > >> lock is read arrier, unlock is write barrier.
    > Let's say there's a shared data structure protected by a spinlock and
    > two threads are accessing it.
    > 1. thr1 locks spin
    > 2. thr1 updates data structure
    > 3. thr1 unlocks spin
    > 4. thr2 locks spin
    > 5. thr2 accesses data structure
    > 6. thr2 unlocks spin
    > If spin_unlock is not a write barrier and spin_lock is not a read
    > barrier, nothing guarantees memory accesses from step#5 will see the
    > changes made in step#2. Memory fetch can occur during updates in step#2
    > or even before that.

    Ah, but this is something different. Both lock/unlock are full barriers,
    but they protect only one direction. A memory op must not leak out of the
    critical section, but it may leak in.

    A = B; // 1
    lock(); // 2
    C = D; // 3

    this can be re-ordered to

    lock(); // 2
    C = D; // 3
    A = B; // 1

    but 2 and 3 must not be re-ordered.

    To be sure, I contacted Paul E. McKenney privately, and his reply is

    > No. See for example IA64 in file include/asm-ia64/spinlock.h,
    > line 34 for spin_lock() and line 92 for spin_unlock(). The
    > spin_lock() case uses a ,acq completer, which will allow preceding
    > reads to be reordered into the critical section. The spin_unlock()
    > uses the ,rel completer, which will allow subsequent writes to be
    > reordered into the critical section. The locking primitives are
    > guaranteed to keep accesses bound within the critical section, but
    > are free to let outside accesses be reordered into the critical
    > section.
    > Download the Itanium Volume 2 manual:
    > Table 2.3 on page 2:489 (physical page 509) shows an example of how
    > the rel and acq completers work.

    > > Could you also look at
    > >
    > >
    > > and, in particular,
    > >
    > This is because spin_lock() isn't a write barrier, right? I totally
    > agree with you there.

    Yes, but in fact I think wake_up() needs a full mb() semantics (which we
    don't have _in theory_), because try_to_wake_up() first checks task->state
    and does nothing if it is TASK_RUNNING.

    That is why I think that smp_mb__before_spinlock() may be useful not only
    for workqueue.c


    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to
    More majordomo info at
    Please read the FAQ at

     \ /
      Last update: 2007-05-16 20:57    [W:0.023 / U:8.124 seconds]
    ©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site