lkml.org 
[lkml]   [2006]   [Oct]   [30]   [last100]   RSS Feed
Views: [more markup]  [less markup]  [headers]  [forward] 
 
Messages in this thread
    /
    DateMon, 30 Oct 2006 15:52:06 +0530
    FromSrivatsa Vaddagiri <>
    SubjectRe: [rfc] [patch] mm: Slab - Eliminate lock_cpu_hotplug from slab
    On Fri, Oct 27, 2006 at 06:19:19PM -0700, Ravikiran G Thirumalai wrote:
    > This patch also takes the cache_chain_sem at kmem_cache_shrink to
    > protect sanity of cpu_online_map at __cache_shrink, as viewed by slab.
    > (kmem_cache_shrink->__cache_shrink->drain_cpu_caches).  But, really,
    
    drain_cpu_caches uses on_each_cpu() ..which does a preempt_disable()
    before using the cpu_online_map. That should be a safe enough access to the
    bitmap?
    
    > kmem_cache_shrink is used at just one place in the acpi subsystem!
    > Do we really need to keep kmem_cache_shrink at all?
    > 
    > Another note.  Looks like a cpu hotplug event can send  CPU_UP_CANCELED to
    > a registered subsystem even if the subsystem did not receive CPU_UP_PREPARE.
    > This could be due to a subsystem registered for notification earlier than
    > the current subsystem crapping out with NOTIFY_BAD. Badness can occur with
    > in the CPU_UP_CANCELED code path at slab if this happens (The same would
    > apply for workqueue.c as well).  To overcome this, we might have to use either
    > a) a per subsystem flag and avoid handling of CPU_UP_CANCELED, or
    > b) Use a special notifier events like LOCK_ACQUIRE/RELEASE as Gautham was
    >    using in his experiments, or
    > c) Do not send CPU_UP_CANCELED to a subsystem which did not receive
    >    CPU_UP_PREPARE.
    > 
    > I would prefer c).
    
    I think we need both b) and c).
    
    Let me explain.
    
    The need for c) is straightforward.
    
    The need for b) comes from the fact that _cpu_down messes with the
    tsk->cpus_allowed mask (to possibly jump off the dying CPU). This would cause 
    sched_getaffinity() to potentially return a false value back to the user and 
    hence it was modified to take lock_cpu_hotplug() before reading 
    tsk->cpus_allowed.
    
    If we are discarding this whole lock_cpu_hotplug(), then IMO, we should
    use LOCK_ACQUIRE/RELEASE, where ACQUIRE notification is sent *before*
    messing with tsk->cpus_allowed and RELEASE notification sent *after*
    restoring tsk->cpus_allowed (something like below):
    
    @@ -186,13 +186,14 @@ int cpu_down(unsigned int cpu)
     {
            int err = 0;
    -       mutex_lock(&cpu_add_remove_lock);
    +       blocking_notifier_call_chain(&cpu_chain, CPU_LOCK_ACQUIRE,
    +                                               (void *)(long)cpu);
            if (cpu_hotplug_disabled)
                    err = -EBUSY;
            else
                    err = _cpu_down(cpu);
    -
    -       mutex_unlock(&cpu_add_remove_lock);
    +       blocking_notifier_call_chain(&cpu_chain, CPU_LOCK_RELEASE,
    +                                               (void *)(long)cpu);
            return err;
     }
    
    -- 
    Regards,
    vatsa
    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at  http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at  http://www.tux.org/lkml/
    
    
    \
     
     \ /
      Last update: 2006-10-30 11:19    [from the cache]
    ©2003-2008