lkml.org 
[lkml]   [2012]   [Oct]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [PATCH] CPU hotplug, debug: Detect imbalance between get_online_cpus() and put_online_cpus()
    On Wed, 03 Oct 2012 18:23:09 +0530
    "Srivatsa S. Bhat" <srivatsa.bhat@linux.vnet.ibm.com> wrote:

    > The synchronization between CPU hotplug readers and writers is achieved by
    > means of refcounting, safe-guarded by the cpu_hotplug.lock.
    >
    > get_online_cpus() increments the refcount, whereas put_online_cpus() decrements
    > it. If we ever hit an imbalance between the two, we end up compromising the
    > guarantees of the hotplug synchronization i.e, for example, an extra call to
    > put_online_cpus() can end up allowing a hotplug reader to execute concurrently with
    > a hotplug writer. So, add a BUG_ON() in put_online_cpus() to detect such cases
    > where the refcount can go negative.
    >
    > Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
    > ---
    >
    > kernel/cpu.c | 1 +
    > 1 file changed, 1 insertion(+)
    >
    > diff --git a/kernel/cpu.c b/kernel/cpu.c
    > index f560598..00d29bc 100644
    > --- a/kernel/cpu.c
    > +++ b/kernel/cpu.c
    > @@ -80,6 +80,7 @@ void put_online_cpus(void)
    > if (cpu_hotplug.active_writer == current)
    > return;
    > mutex_lock(&cpu_hotplug.lock);
    > + BUG_ON(cpu_hotplug.refcount == 0);
    > if (!--cpu_hotplug.refcount && unlikely(cpu_hotplug.active_writer))
    > wake_up_process(cpu_hotplug.active_writer);
    > mutex_unlock(&cpu_hotplug.lock);

    I think calling BUG() here is a bit harsh. We should only do that if
    there's a risk to proceeding: a risk of data loss, a reduced ability to
    analyse the underlying bug, etc.

    But a cpu-hotplug locking imbalance is a really really really minor
    problem! So how about we emit a warning then try to fix things up?
    This should increase the chance that the machine will keep running and
    so will increase the chance that a user will be able to report the bug
    to us.


    --- a/kernel/cpu.c~cpu-hotplug-debug-detect-imbalance-between-get_online_cpus-and-put_online_cpus-fix
    +++ a/kernel/cpu.c
    @@ -80,9 +80,12 @@ void put_online_cpus(void)
    if (cpu_hotplug.active_writer == current)
    return;
    mutex_lock(&cpu_hotplug.lock);
    - BUG_ON(cpu_hotplug.refcount == 0);
    - if (!--cpu_hotplug.refcount && unlikely(cpu_hotplug.active_writer))
    - wake_up_process(cpu_hotplug.active_writer);
    + if (!--cpu_hotplug.refcount) {
    + if (WARN_ON(cpu_hotplug.refcount == -1))
    + cpu_hotplug.refcount++; /* try to fix things up */
    + if (unlikely(cpu_hotplug.active_writer))
    + wake_up_process(cpu_hotplug.active_writer);
    + }
    mutex_unlock(&cpu_hotplug.lock);

    }
    _


    \
     
     \ /
      Last update: 2012-10-04 00:01    [W:4.507 / U:0.012 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site