[lkml]   [2011]   [Dec]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
    Patch in this message
    SubjectRe: [PATCH] VFS: br_write_lock locks on possible CPUs other than online CPUs


    I feel the following patch is a better fix for 2 reasons:

    1. As Al Viro pointed out, if we do for_each_possible_cpus() then we might
    encounter unnecessary performance hit in some scenarios. So working with
    only online cpus, safely(a.k.a race-free), if possible, would be a good
    solution (which this patch implements).

    2. *_global_lock_online() and *_global_unlock_online() needs fixing as well
    because, the names suggest that they lock/unlock per-CPU locks of only the
    currently online CPUs, but unfortunately they do not have any synchronization
    to prevent offlining those CPUs in between, if it happens to race with a CPU
    hotplug operation.

    And if we solve issue 2 above "carefully" (as mentioned in the changelog below),
    it solves this whole thing!

    From: Srivatsa S. Bhat <>
    Subject: [PATCH] VFS: Fix race between CPU hotplug and *_global_[un]lock_online()

    The *_global_[un]lock_online() macros defined in include/linux/lglock.h
    can race with CPU hotplug operations. Fix this race by using the pair
    get_online_cpus() and put_online_cpus() around them, so as to prevent CPU
    hotplug happening at the same time.

    But be careful to note the semantics here: if we enable CPU hotplug in-between
    a lock_online() and the corresponding unlock_online(), the lock states can
    get messed up, since we might end up for example, in a situation such as taking
    a lock on an online CPU but not releasing it because that CPU was offline when
    unlock_online() was invoked (thanks to Cong Meng for debugging this issue).
    [Soft-lockups detected as a consequence of this issue was reported earlier in]

    So, we are careful to allow CPU hotplug only after the lock-unlock sequence
    is complete.

    Debugged-by: Cong Meng <>
    Signed-off-by: Srivatsa S. Bhat <>

    include/linux/lglock.h | 3 +++
    1 files changed, 3 insertions(+), 0 deletions(-)

    diff --git a/include/linux/lglock.h b/include/linux/lglock.h
    index f549056..583d1a8 100644
    --- a/include/linux/lglock.h
    +++ b/include/linux/lglock.h
    @@ -22,6 +22,7 @@
    #include <linux/spinlock.h>
    #include <linux/lockdep.h>
    #include <linux/percpu.h>
    +#include <linux/cpu.h>

    /* can make br locks by using local lock for read side, global lock for write */
    #define br_lock_init(name) name##_lock_init()
    @@ -126,6 +127,7 @@
    int i; \
    preempt_disable(); \
    rwlock_acquire(&name##_lock_dep_map, 0, 0, _RET_IP_); \
    + get_online_cpus(); \
    for_each_online_cpu(i) { \
    arch_spinlock_t *lock; \
    lock = &per_cpu(name##_lock, i); \
    @@ -142,6 +144,7 @@
    lock = &per_cpu(name##_lock, i); \
    arch_spin_unlock(lock); \
    } \
    + put_online_cpus(); \
    preempt_enable(); \
    } \
    EXPORT_SYMBOL(name##_global_unlock_online); \

     \ /
      Last update: 2011-12-19 08:35    [W:0.022 / U:62.812 seconds]
    ©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site