linux-kernel - Re: [rfc] [patch] mm: Slab - Eliminate lock_cpu

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20061030102206.GA26669@in.ibm.com>
Date:	Mon, 30 Oct 2006 15:52:06 +0530
From:	Srivatsa Vaddagiri <vatsa@...ibm.com>
To:	Ravikiran G Thirumalai <kiran@...lex86.org>
Cc:	Andrew Morton <akpm@...l.org>, linux-kernel@...r.kernel.org,
	Christoph Lameter <clameter@...r.sgi.com>,
	Pekka J Enberg <penberg@...Helsinki.FI>, ego@...ibm.com,
	"Benzi Galili (Benzi@...leMP.com)" <benzi@...lemp.com>,
	Alok Kataria <alok.kataria@...softinc.com>, shai@...lex86.org
Subject: Re: [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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/