linux-kernel - Re: [RFC][PATCH] CPUSets: Move most calls to rebuild_sched

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] [day] [month] [year] [list]

Message-ID: <486523E6.4030201@qualcomm.com>
Date:	Fri, 27 Jun 2008 10:31:18 -0700
From:	Max Krasnyansky <maxk@...lcomm.com>
To:	Paul Menage <menage@...gle.com>
CC:	Vegard Nossum <vegard.nossum@...il.com>, Paul Jackson <pj@....com>,
	a.p.zijlstra@...llo.nl, linux-kernel@...r.kernel.org,
	Gautham shenoy <ego@...ibm.com>
Subject: Re: [RFC][PATCH] CPUSets: Move most calls to rebuild_sched_domains()
 to the workqueue

Paul Menage wrote:
> On Thu, Jun 26, 2008 at 10:10 PM, Max Krasnyansky <maxk@...lcomm.com> wrote:
>> Instead of changing cpu_hotplug locking should we maybe try to avoid using
>> cgroup_lock in rebuild_sched_domains() ?
> 
> Yes, that would be good too.
> 
>> There is a comment in cpuset.c that says
>>  * If a task is only holding callback_mutex, then it has read-only
>>  * access to cpusets.
>>
>> I'm not sure if it's still valid. rebuild_sched_domains() only needs read only
>> access, it does not really modify any cpuset structures.
> 
> The comment is still valid, if you interpret it narrowly enough.
> Holding callback_mutex gives you read-only access to structures that
> are under the control of cpusets. But rebuild_sched_domains() needs to
> traverse the hierarchy of cpusets, and that hierarchy is controlled by
> cgroups. 
Yes that's what I meant by "not sure if it's still valid" I looked at 
the code and it did not look like callback_mutex protected overall 
hierarchy. Thanx for confirming that.

> Currently the only synchronization primitives exposed by
> cgroups are:
> 
> - cgroup_lock()/cgroup_unlock() to prevent all cgroup modifications
> (also used as the main synchronization primitive by some subsystems,
> i.e. it's in danger of becoming the cgroups equivalent of the BKL).
> 
> - task_lock()/task_unlock() to prevent a specific task from changing cgroups
> 
> Possible options for richer locking support include:
> 
> - lock/unlock a hierarchy, to prevent creation/deletion of cgroups in
> that hierarchy
Sounds good.

> - lock/unlock a cgroup to prevent deletion of that cgroup
Can that be just an atomic refcount ?

> - lock/unlock a cgroup to prevent task movement in/out of that cgroup
Sounds good.

> For the case of rebuild_sched_domains, we need the first of these
> options. This lock would be taken in cgroup.c at the points where it
> attached and removed cgroups from a cgroup tree, and could be taken by
> something like cpusets that needed to keep the hierarchy stable while
> scanning it. I think it would be fine to make it a mutex rather than a
> spinlock.
Agree

> cpu_hotplug.lock has to nest outside this hierarchy lock due to it
> being taken at the root of the hotplug/unplug path. So as long as we
> can ensure that we can always nest the hierarchy lock inside any
> get_online_cpus()/put_online_cpus() pairs, we should be OK.
Yes. Although that basically means that we always have to take 
cpu_hotplug.lock before hierarchy lock.

I like the proposal in general. Specifically for the 
rebuild_sched_domain() I'm now thinking that maybe we can get away with 
not involving cpuset at all. I think that what Peter meant originally. 
I'll send more thoughts on this separately.

Max

--
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/