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 for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20150413144311.GF5029@twins.programming.kicks-ass.net>
Date:	Mon, 13 Apr 2015 16:43:11 +0200
From:	Peter Zijlstra <peterz@...radead.org>
To:	Preeti U Murthy <preeti@...ux.vnet.ibm.com>
Cc:	tj@...nel.org, svaidy@...ux.vnet.ibm.com, nacc@...ux.vnet.ibm.com,
	rjw@...ysocki.net, linux-kernel@...r.kernel.org,
	lizefan@...wei.com, anton@...ba.org, bharata@...ux.vnet.ibm.com,
	cgroups@...r.kernel.org, paulmck@...ux.vnet.ibm.com,
	mingo@...nel.org, serge@...lyn.com
Subject: Re: [PATCH V2] cpuset: Add knob to make allowed masks hotplug
 invariant on legacy hierarchy

On Mon, Apr 13, 2015 at 05:46:37PM +0530, Preeti U Murthy wrote:
> On 04/13/2015 12:31 PM, Peter Zijlstra wrote:

> > Would it not make sense to make that a mount option and limit the amount
> > of semantic variants of cpusets?
> 
> I spent some time analyzing if this would be a better option than the
> sysfs knob and I think not for the following reasons:
> 
> 1. Mount options tend to be generic across the controllers of a cgroup.
> But use case addressed by this patch is specific to the cpuset controller.

Surely we can get around that somehow.

> 2. The behavior that this patch is trying to bring about is not a
> drastic one to call for a mount option equivalent to the __SANE_BEHAVIOR
> one that existed earlier. This option was used to switch the legacy
> design to the default one.
> 
> However this patch is not *wholly* mimicking the default hierarchy
> behavior. The behavior when cpusets become empty is left untouched for
> instance. The patch borrows one of the behaviors from the default
> hierarchy only and hence just not justify the use of a mount flag.

So the 'problem' I have is that you introduce a 3rd semantic for the
cpuset thing.

You also do not answer if you can live with the default hierarchy
behaviour, only that your patch mimicks a subset of it.

Why not all of it?

> 3. cpuset controller already has flags that allow a runtime change in
> the semantics of scheduling, memory allocation.
> Eg: CS_SCHED_LOAD_BALANCE, CS_SPREAD_PAGE. This page intends to bring
> about a runtime change in the semantics of maintaining the allowed cpus
> and mem masks in the face of hotplug. So adding a flag to take care of
> this looks better and the design falls in place.

s/page/patch/

I disagree, you change global semantics, the ONLY_ON_ROOT is a hint
here. If such a file really is the only option, then at least take the
entire semantics from default hierarchy, not only part of it.

> 4. The worry was around possible race conditions when the user tries to
> change this sysfs knob during runtime after mounting a cgroup hierarchy.
> But the sites are protected by the cpuset_lock. Moreover all of the
> cpuset behavior enforced by its flags can be changed after mounting the
> cgroup. So cpuset controller already expects twiddling of these knobs at
> will by the user.

That just doesn't parse. cpuset_lock doesn't protect anything userspace.

You cannot atomically read this flag and do something else. It being
ONLY_ON_ROOT might mean you don't even have access to the flag.
--
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/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ