linux-kernel - Re: cpusets vs cpu-hotplug interaction is broken?

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: <20070829105204.GA190@tv-sign.ru>
Date:	Wed, 29 Aug 2007 14:52:04 +0400
From:	Oleg Nesterov <oleg@...sign.ru>
To:	Gautham R Shenoy <ego@...ibm.com>
Cc:	Cliff Wickman <cpw@....com>, Paul Jackson <pj@....com>,
	Paul Menage <menage@...gle.com>, linux-kernel@...r.kernel.org,
	Andrew Morton <akpm@...ux-foundation.org>,
	Srivatsa Vaddagiri <vatsa@...ibm.com>
Subject: Re: cpusets vs cpu-hotplug interaction is broken?

On 08/29, Gautham R Shenoy wrote:
>
> On Tue, Aug 28, 2007 at 05:48:53PM +0400, Oleg Nesterov wrote:
> > (cpu-hotplug experts cc'ed)
> > 
> > On 08/25, Oleg Nesterov wrote:
> > >
> > > After the brief look at kernel/cpuset.c, it seems that attach_task() should
> > > guarantee that the task can't use CPUs outside of cpuset->cpus_allowed.
> > > 
> > > But this looks racy wrt sched_setaffinity() which does
> > > 
> > > 	cpus_allowed = cpuset_cpus_allowed(p);
> > > 	// callback_mutex is free
> > > 	set_cpus_allowed(p);
> > > 
> > > What if attach_task()->set_cpus_allowed() happens in between?
> > 
> > Actually, I think there is another problem, and cpuset_cpus_allowed() is
> > just broken wrt CONFIG_HOTPLUG_CPU.
> > 
> > Suppose that CONFIG_CPUSETS is true, but we don't use cpusets. In that
> > case all tasks in system belong to the top_cpuset (btw, why cpuset_init()
> > sets init_task.cpuset? this was already done by cpuset_init_early()), and
> > we should have the same behaviour as without CONFIG_CPUSETS.
> > 
> > By default, all tasks have ->cpus_allowed = CPU_MASK_ALL inherited from
> > kernel_init(). This means that the task can use the new CPU right after
> > cpu_up().
> > 
> > Now let's suppose that some task does sched_setaffinity(0, CPU_MASK_ALL).
> > In that case, cpuset_cpus_allowed() sets ->cpus_allowed = cpu_online_map,
> > and I think this is just wrong. Now that task doesn't see the new CPUs.
> > 
> 
> Good point! 
> 
> A task's cpu_allowed mask can contain cpus which are offline.
> And if those cpus exist in the intersection of the task's requested mask
> and cpuset's cpu mask, why should we unset the offlined cpus from that 
> intersection? Either way the task is not going to run on the cpus while
> they are in the offlined state.  And on cpu_up, if the cpu is present in
> the task's allowed mask, it can run on that cpu, which is a good thing.
> 
> The two users of cpuset_cpus_allowed - sched_setaffinity and pdflush
> don't seem to require the online cpu information.
> 
> Paul, is there any particular reason why we need guarentee_online_cpus
> to be called in cpuset_cpus_allowed ? 

Note also that cpuset_cpus_allowed()->guarentee_online_cpus() easily allows
the task to escape its ->cpuset, sched_setaffinity(cpumask_of_cpu(OFFLINE_CPU))
is enough.

Another note, http://marc.info/?t=118823976000002 really needs something
like guarentee_online_cpus(). However even in this case we shouldn't afaics
"and" cpuset->cpus_allowed with cpu_online_map.

Oleg.

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