[lkml]   [2009]   [Jul]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
    SubjectRe: [PATCH] cpusets: fix deadlock with cpu_down()->cpuset_lock()
    Oleg Nesterov wrote:
    > On 07/30, Lai Jiangshan wrote:
    >> Oleg Nesterov wrote:
    >>> On 07/29, Oleg Nesterov wrote:
    >>>> I strongly believe the bug does exist, but this patch needs the review
    >>>> from maintainers.
    >>> Yes...
    >>>> IOW, with this patch migration_call(CPU_DEAD) runs without callback_mutex,
    >>>> but kernel/cpuset.c always takes get_online_cpus() before callback_mutex.
    >>> Oh. I'm afraid this is not an option.
    >>> callback_mutex should nest under cgroup_mutex, but cpu hotplu pathes
    >>> take cgroup_mutex under cpu_hotplug->lock. Lockdep won't be happy.
    >>> Oleg.
    >> We have made great effort to remove get_online_cpus() from cgroup_mutex
    >> critical region.
    > Agreed.
    >> We can migrate the owner of callback_mutex in migration_call(CPU_DEAD)
    >> at first(and then take callback_mutex and migrate others).
    > Not sure I understand how can we do this. Even if we know the owner
    > of callback_mutex, if we can migrate it safely without callback_mutex
    > why we can't migrate other tasks without this lock?

    Since we have migrated the owner, we can take callback_mutex to
    migrate others ...

    > In any case this doesn't look like a clean solution,

    No, it's not a clean solution.

    > imho. But I hardly understand what cpuset is,

    > can't suggest something clever.

    We can add cpuset_lock()/cpuset_unlock() around __stop_machine()
    in _cpu_down().

    # It's OK, because we don't require any other lock in this
    # critical region. It's will not cause any kinds of deadlock.
    # It's OK too. because all work functions(chill(),stop_cpu())
    # of stop_machine_wq don't require any other lock.

    This fixes the bug in migrate_call(). Because there is no task which
    holds callback_mutex in dead cpu after we add
    cpuset_lock()/cpuset_unlock() around __stop_machine() in _cpu_down().

    And it helps for your "cpu_hotplug: don't play with current->cpus_allowed"
    Am I right?


     \ /
      Last update: 2009-07-31 04:25    [W:0.024 / U:1.204 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site