[PATCH v7 1/3] smpboot: allow excluding cpus from the smpboot threads

Chris Metcalf
2015-04-10T20:48:36Z

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, "cpumask", which
is an optional pointer to a cpumask that indicates whether
or not the given smp_hotplug_thread should run on that core; the
cpumask is checked when deciding whether to unpark the thread.

If a change is made to the cpumask, the
smpboot_update_cpumask_percpu_thread() function should be called and
threads will be suitably parked and unparked.

Signed-off-by: Chris Metcalf <cmetcalf@ezchip.com>
---
I don't know why it is necessary to explicitly unpark the threads in
smpboot_destroy_threads() before destroying them. We can't even do it
in the same for_each_possible_cpu() loop as the kthread_stop() call;
it appears all the threads on online cores must be unparked prior to
trying to stop all the threads, or the system hangs.

Also, one unexpected consequence of leaving threads in TASK_PARKED
state is that we can actually see them in /proc! This isn't normally
true since we usually just park them briefly during cpu offlining.
/proc/NNN/stat{,us} reports the parked threads as "R (running)", even
though they are waiting in __kthread_parkme(). I proposed a fix for
this in a new patch 3/3 in this patch series.

In all honesty, I'm still fond of the model where we just do_exit(0)
the threads that we don't want, as soon as registration creates them.
(We can still support the watchdog_cpumask sysctl easily enough.)
This doesn't add any new semantics to smpboot (for good or for bad),
and more importantly it makes it clear that we don't have watchdog
tasks running on the nohz_full cores, which otherwise are going to
make people continually wonder what's going on until they carefully
read the code. But I'm OK with the direction laid out in this patch
if it's the consensus preferred model.

v7: change from valid_cpu() callback to optional cpumask field
park smpboot threads rather than just not creating them

v6: change from an "exclude" data pointer to a more generic
valid_cpu() callback [Frederic]

v5: use alloc_cpumask_var() [Sasha Levin]
switch from watchdog_exclude to watchdog_cpumask [Frederic]
simplify the smp_hotplug_thread API to watchdog [Frederic]
add Don's Acked-by

include/linux/smpboot.h | 6 ++++++
kernel/smpboot.c | 42 ++++++++++++++++++++++++++++++++++++++----
2 files changed, 44 insertions(+), 4 deletions(-)

diff --git a/include/linux/smpboot.h b/include/linux/smpboot.h
index d600afb21926..fb9ed92201a5 100644
--- a/include/linux/smpboot.h
+++ b/include/linux/smpboot.h
@@ -27,6 +27,10 @@ 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!
+ * @cpumask: Optional pointer to a set of possible cores to
+ * allow threads to come unparked on.
+ * You must call smpboot_update_cpumask_percpu_thread()
+ * after any updates to the pointed-to mask.
* @selfparking: Thread is not parked by the park function.
* @thread_comm: The base name of the thread
*/
@@ -41,11 +45,13 @@ struct smp_hotplug_thread {
void (*park)(unsigned int cpu);
void (*unpark)(unsigned int cpu);
void (*pre_unpark)(unsigned int cpu);
+ struct cpumask *cpumask;
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_update_cpumask_percpu_thread(struct smp_hotplug_thread *plug_thread);

#endif
diff --git a/kernel/smpboot.c b/kernel/smpboot.c
index c697f73d82d6..12bd9b57a682 100644
--- a/kernel/smpboot.c
+++ b/kernel/smpboot.c
@@ -219,11 +219,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->cpumask == NULL || cpumask_test_cpu(cpu, ht->cpumask)) {
+ 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)
@@ -258,6 +260,13 @@ static void smpboot_destroy_threads(struct smp_hotplug_thread *ht)
{
unsigned int cpu;

+ /* Unpark any threads that were voluntarily parked. */
+ if (ht->cpumask) {
+ for_each_online_cpu(cpu)
+ if (!cpumask_test_cpu(cpu, ht->cpumask))
+ kthread_unpark(*per_cpu_ptr(ht->store, cpu));
+ }
+
/* We need to destroy also the parked threads of offline cpus */
for_each_possible_cpu(cpu) {
struct task_struct *tsk = *per_cpu_ptr(ht->store, cpu);
@@ -316,6 +325,31 @@ void smpboot_unregister_percpu_thread(struct smp_hotplug_thread *plug_thread)
}
EXPORT_SYMBOL_GPL(smpboot_unregister_percpu_thread);

+/**
+ * smpboot_update_cpumask_percpu_thread - Adjust which per_cpu hotplug threads stay parked
+ * @plug_thread: Hotplug thread descriptor
+ *
+ * After changing any bits in the mask pointed to by "cpumask", call this
+ * function to let appropriate threads park and unpark.
+ */
+void smpboot_update_cpumask_percpu_thread(struct smp_hotplug_thread *plug_thread)
+{
+ unsigned int cpu;
+
+ get_online_cpus();
+ mutex_lock(&smpboot_threads_lock);
+ for_each_online_cpu(cpu) {
+ if (plug_thread->cpumask == NULL ||
+ cpumask_test_cpu(cpu, plug_thread->cpumask))
+ 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_update_cpumask_percpu_thread);
+
static DEFINE_PER_CPU(atomic_t, cpu_hotplug_state) = ATOMIC_INIT(CPU_POST_DEAD);

/*
--
2.1.2