linux-kernel - Re: [PATCH] affine kernel threads to specified cpumask

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: <e76aedad-8c55-1651-007d-6e17882403cb@windriver.com>
Date:   Mon, 23 Mar 2020 09:29:23 -0600
From:   Chris Friesen <chris.friesen@...driver.com>
To:     Marcelo Tosatti <mtosatti@...hat.com>, linux-kernel@...r.kernel.org
Cc:     Christoph Lameter <cl@...ux.com>, Vu Tran <vu.tran@...driver.com>,
        Jim Somerville <Jim.Somerville@...driver.com>,
        Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [PATCH] affine kernel threads to specified cpumask

On 3/23/2020 7:54 AM, Marcelo Tosatti wrote:
> 
> This is a kernel enhancement to configure the cpu affinity of kernel
> threads via kernel boot option kthread_cpus=<cpulist>.
> 
> With kthread_cpus specified, the cpumask is immediately applied upon
> thread launch. This does not affect kernel threads that specify cpu
> and node.
> 
> This allows CPU isolation (that is not allowing certain threads
> to execute on certain CPUs) without using the isolcpus= parameter,
> making it possible to enable load balancing on such CPUs
> during runtime.
> 
> Note-1: this is based off on MontaVista's patch at
> https://github.com/starlingx-staging/stx-integ/blob/master/kernel/kernel-std/centos/patches/affine-compute-kernel-threads.patch

It's Wind River, not MontaVista. :)

> Difference being that this patch is limited to modifying
> kernel thread cpumask: Behaviour of other threads can
> be controlled via cgroups or sched_setaffinity.

What cgroup would the usermode helpers called by the kernel end up in?
Same as init?

Assuming that's covered, I'm good with this patch.

<snip>

> +static struct cpumask user_cpu_kthread_mask __read_mostly;
> +static int user_cpu_kthread_mask_valid __read_mostly;

Would it be cleaner to get rid of user_cpu_kthread_mask_valid and just
move the "if (!cpumask_empty" check into init_kthread_cpumask()?  I'm
not really opinionated, just thinking out loud.

> +int __init init_kthread_cpumask(void)
> +{
> +	if (user_cpu_kthread_mask_valid == 1)
> +		cpumask_copy(&__cpu_kthread_mask, &user_cpu_kthread_mask);
> +	else
> +		cpumask_copy(&__cpu_kthread_mask, cpu_all_mask);
> +
> +	return 0;
> +}
> +
> +static int __init kthread_setup(char *str)
> +{
> +	cpulist_parse(str, &user_cpu_kthread_mask);
> +	if (!cpumask_empty(&user_cpu_kthread_mask))
> +		user_cpu_kthread_mask_valid = 1;
> +
> +	return 1;
> +}