| 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
| ||
|
Message-ID: <20150410015842.GG18314@lerouge>
Date: Fri, 10 Apr 2015 03:58:43 +0200
From: Frederic Weisbecker <fweisbec@...il.com>
To: Chris Metcalf <cmetcalf@...hip.com>
Cc: Thomas Gleixner <tglx@...utronix.de>,
Don Zickus <dzickus@...hat.com>, linux-kernel@...r.kernel.org,
Peter Zijlstra <peterz@...radead.org>
Subject: Re: [PATCH] smpboot: allow excluding cpus from the smpboot threads
On Thu, Apr 09, 2015 at 04:29:01PM -0400, Chris Metcalf wrote:
> This change allows some cores to be excluded from running the
> smp_hotplug_thread tasks. The motivating example for this is
> the watchdog threads, which by default we don't want to run
> on any enabled nohz_full cores.
>
> A new smp_hotplug_thread field is introduced, "valid_cpu", which
> is an optional pointer to a function that returns per-cpu whether
> or not the given smp_hotplug_thread should run on that core; the
> function is called when deciding whether to unpark the thread.
>
> If a change is made to which cpus are valid, the
> smpboot_repark_percpu_thread() function should be called and
> threads will be suitably parked and unparked.
>
> Signed-off-by: Chris Metcalf <cmetcalf@...hip.com>
> ---
> Thomas, how does this look? If this seems about right, I'll fold
> in your feedback and put out a patch set that includes the matching
> changes to the watchdog and, if Frederic will take it for the
> nohz queue to the timer tree, send it up that way. This is just
> compile-tested so far since I have to wrap up for the time being
> and head home. Final patch will actually be tested :-)
>
> I took Frederic's suggested patch from a 10,000 foot viewpoint and
> modified it to stick with the valid_cpu() callback approach.
>
> p.s. I think the smpboot_thread_schedule() declaration in
> linux/smpboot.h is dead; there doesn't seem to be a definition.
>
> include/linux/smpboot.h | 4 ++++
> kernel/smpboot.c | 37 +++++++++++++++++++++++++++++++++----
> 2 files changed, 37 insertions(+), 4 deletions(-)
>
> diff --git a/include/linux/smpboot.h b/include/linux/smpboot.h
> index 13e929679550..7dedbf92420e 100644
> --- a/include/linux/smpboot.h
> +++ b/include/linux/smpboot.h
> @@ -27,6 +27,8 @@ struct smpboot_thread_data;
> * @pre_unpark: Optional unpark function, called before the thread is
> * unparked (cpu online). This is not guaranteed to be
> * called on the target cpu of the thread. Careful!
> + * @valid_cpu: Optional function, called when unparking the threads,
> + * to limit the set of cpus on which threads are unparked.
I'm not sure why this needs to be a callback instead of a pointer to a cpumask
that smpboot can handle by itself. In fact I don't understand why you want to stick with
this valid_cpu() approach.
> * @selfparking: Thread is not parked by the park function.
> * @thread_comm: The base name of the thread
> */
> @@ -41,12 +43,14 @@ struct smp_hotplug_thread {
> void (*park)(unsigned int cpu);
> void (*unpark)(unsigned int cpu);
> void (*pre_unpark)(unsigned int cpu);
> + int (*valid_cpu)(unsigned int cpu);
> bool selfparking;
> const char *thread_comm;
> };
>
> int smpboot_register_percpu_thread(struct smp_hotplug_thread *plug_thread);
> void smpboot_unregister_percpu_thread(struct smp_hotplug_thread *plug_thread);
> +void smpboot_repark_percpu_thread(struct smp_hotplug_thread *plug_thread);
> int smpboot_thread_schedule(void);
>
> #endif
> diff --git a/kernel/smpboot.c b/kernel/smpboot.c
> index 40190f28db35..c7dd768a4599 100644
> --- a/kernel/smpboot.c
> +++ b/kernel/smpboot.c
> @@ -218,11 +218,13 @@ int smpboot_create_threads(unsigned int cpu)
>
> static void smpboot_unpark_thread(struct smp_hotplug_thread *ht, unsigned int cpu)
> {
> - struct task_struct *tsk = *per_cpu_ptr(ht->store, cpu);
> + if (!ht->valid_cpu || ht->valid_cpu(cpu)) {
> + struct task_struct *tsk = *per_cpu_ptr(ht->store, cpu);
>
> - if (ht->pre_unpark)
> - ht->pre_unpark(cpu);
> - kthread_unpark(tsk);
> + if (ht->pre_unpark)
> + ht->pre_unpark(cpu);
> + kthread_unpark(tsk);
> + }
> }
>
> void smpboot_unpark_threads(unsigned int cpu)
> @@ -314,3 +316,30 @@ void smpboot_unregister_percpu_thread(struct smp_hotplug_thread *plug_thread)
> put_online_cpus();
> }
> EXPORT_SYMBOL_GPL(smpboot_unregister_percpu_thread);
> +
> +/**
> + * smpboot_repark_percpu_thread - Adjust which per_cpu hotplug threads stay parked
> + * @plug_thread: Hotplug thread descriptor
> + *
> + * After changing what the valid_cpu() callback will return, call this
> + * function to let appropriate threads park and unpark.
> + */
> +void smpboot_repark_percpu_thread(struct smp_hotplug_thread *plug_thread)
That looks to me a bit of an unecessary indirect way to tell "update cpumask".
> +{
> + unsigned int cpu;
> +
> + if (!plug_thread->valid_cpu)
> + return;
> +
> + get_online_cpus();
> + mutex_lock(&smpboot_threads_lock);
> + for_each_online_cpu(cpu) {
> + if (plug_thread->valid_cpu(cpu))
> + smpboot_unpark_thread(plug_thread, cpu);
> + else
> + smpboot_park_thread(plug_thread, cpu);
> + }
> + mutex_unlock(&smpboot_threads_lock);
> + put_online_cpus();
> +}
> +EXPORT_SYMBOL_GPL(smpboot_repark_percpu_thread);
> --
> 2.1.2
>
--
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