Re: [RFC patch 2/5] smpboot: Provide infrastructure for percpu hotplug threads

Paul E. McKenney
2012-06-13T18:33:18Z

On Wed, Jun 13, 2012 at 11:00:54AM -0000, Thomas Gleixner wrote:
> Provide a generic interface for setting up and tearing down percpu
> threads.
>
> On registration the threads for already online cpus are created and
> started. On deregistration (modules) the threads are stoppped.
>
> During hotplug operations the threads are created, started, parked and
> unparked. The datastructure for registration provides a pointer to
> percpu storage space and optional setup, cleanup, park, unpark
> functions. These functions are called when the thread state changes.
>
> Thread functions should look like the following:
>
> int thread_fn(void *cookie)
> {
> while (!smpboot_thread_check_parking(cookie)) {
> do_stuff();
> }
> return 0;
> }

Very cool!!!

So I am currently trying to apply this to RCU's per-CPU kthread.
I don't believe that I need to mess with RCU's per-rcu_node kthread
because it can just have its affinity adjusted when the first CPU
onlines and the last CPU offlines for the corresponding rcu_node.

One question below about the order of parking.

Also, I have not yet figured out how this avoids a parked thread waking
up while the CPU is offline, but I am probably still missing something.

Thanx, Paul

> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> ---
> include/linux/smpboot.h | 40 +++++++++
> kernel/cpu.c | 6 +
> kernel/smpboot.c | 205 ++++++++++++++++++++++++++++++++++++++++++++++++
> kernel/smpboot.h | 4
> 4 files changed, 255 insertions(+)
>
> Index: tip/include/linux/smpboot.h
> ===================================================================
> --- /dev/null
> +++ tip/include/linux/smpboot.h
> @@ -0,0 +1,40 @@
> +#ifndef _LINUX_SMPBOOT_H
> +#define _LINUX_SMPBOOT_H
> +
> +#include <linux/types.h>
> +
> +struct task_struct;
> +/* Cookie handed to the thread_fn*/
> +struct smpboot_thread_data;
> +
> +/**
> + * struct smp_hotplug_thread - CPU hotplug related thread descriptor
> + * @store: Pointer to per cpu storage for the task pointers
> + * @list: List head for core management
> + * @thread_fn: The associated thread function
> + * @setup: Optional setup function, called when the thread gets
> + * operational the first time
> + * @cleanup: Optional cleanup function, called when the thread
> + * should stop (module exit)
> + * @park: Optional park function, called when the thread is
> + * parked (cpu offline)
> + * @unpark: Optional unpark function, called when the thread is
> + * unparked (cpu online)
> + * @thread_comm: The base name of the thread
> + */
> +struct smp_hotplug_thread {
> + struct task_struct __percpu **store;
> + struct list_head list;
> + int (*thread_fn)(void *data);
> + void (*setup)(unsigned int cpu);
> + void (*cleanup)(unsigned int cpu, bool online);
> + void (*park)(unsigned int cpu);
> + void (*unpark)(unsigned int cpu);
> + 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);
> +int smpboot_thread_check_parking(struct smpboot_thread_data *td);
> +
> +#endif
> Index: tip/kernel/cpu.c
> ===================================================================
> --- tip.orig/kernel/cpu.c
> +++ tip/kernel/cpu.c
> @@ -280,6 +280,7 @@ static int __ref _cpu_down(unsigned int
> __func__, cpu);
> goto out_release;
> }
> + smpboot_park_threads(cpu);
>
> err = __stop_machine(take_cpu_down, &tcd_param, cpumask_of(cpu));
> if (err) {
> @@ -354,6 +355,10 @@ static int __cpuinit _cpu_up(unsigned in
> goto out;
> }
>
> + ret = smpboot_create_threads(cpu);
> + if (ret)
> + goto out;
> +
> ret = __cpu_notify(CPU_UP_PREPARE | mod, hcpu, -1, &nr_calls);
> if (ret) {
> nr_calls--;
> @@ -370,6 +375,7 @@ static int __cpuinit _cpu_up(unsigned in
>
> /* Now call notifier in preparation. */
> cpu_notify(CPU_ONLINE | mod, hcpu);
> + smpboot_unpark_threads(cpu);
>
> out_notify:
> if (ret != 0)
> Index: tip/kernel/smpboot.c
> ===================================================================
> --- tip.orig/kernel/smpboot.c
> +++ tip/kernel/smpboot.c
> @@ -1,11 +1,17 @@
> /*
> * Common SMP CPU bringup/teardown functions
> */
> +#include <linux/cpu.h>
> #include <linux/err.h>
> #include <linux/smp.h>
> #include <linux/init.h>
> +#include <linux/list.h>
> +#include <linux/slab.h>
> #include <linux/sched.h>
> +#include <linux/export.h>
> #include <linux/percpu.h>
> +#include <linux/kthread.h>
> +#include <linux/smpboot.h>
>
> #include "smpboot.h"
>
> @@ -65,3 +71,202 @@ void __init idle_threads_init(void)
> }
> }
> #endif
> +
> +static LIST_HEAD(hotplug_threads);
> +static DEFINE_MUTEX(smpboot_threads_lock);
> +
> +struct smpboot_thread_data {
> + unsigned int cpu;
> + unsigned int status;
> + struct smp_hotplug_thread *ht;
> +};
> +
> +enum {
> + HP_THREAD_NONE = 0,
> + HP_THREAD_ACTIVE,
> + HP_THREAD_PARKED,
> +};
> +
> +/**
> + * smpboot_thread_check_parking - percpu hotplug thread loop function
> + * @td: thread data pointer
> + *
> + * Checks for thread stop and park conditions. Calls the necessary
> + * setup, cleanup, park and unpark functions for the registered
> + * thread.
> + *
> + * Returns 1 when the thread should exit, 0 otherwise.
> + */
> +int smpboot_thread_check_parking(struct smpboot_thread_data *td)
> +{
> + struct smp_hotplug_thread *ht = td->ht;
> +
> +again:
> + if (kthread_should_stop()) {
> + if (ht->cleanup)
> + ht->cleanup(td->cpu, cpu_online(td->cpu));
> + kfree(td);
> + return 1;
> + }
> +
> + BUG_ON(td->cpu != smp_processor_id());
> +
> + if (kthread_should_park()) {
> + if (ht->park && td->status == HP_THREAD_ACTIVE) {
> + ht->park(td->cpu);
> + td->status = HP_THREAD_PARKED;
> + }
> + kthread_parkme();
> + /* We might have been woken for stop */
> + goto again;
> + }
> +
> + switch (td->status) {
> + case HP_THREAD_NONE:
> + if (ht->setup)
> + ht->setup(td->cpu);
> + td->status = HP_THREAD_ACTIVE;
> + break;
> + case HP_THREAD_PARKED:
> + if (ht->unpark)
> + ht->unpark(td->cpu);
> + td->status = HP_THREAD_ACTIVE;
> + break;
> + }
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(smpboot_thread_check_parking);
> +
> +static int
> +__smpboot_create_thread(struct smp_hotplug_thread *ht, unsigned int cpu)
> +{
> + struct task_struct *tsk = *per_cpu_ptr(ht->store, cpu);
> + struct smpboot_thread_data *td;
> +
> + if (tsk)
> + return 0;
> +
> + td = kzalloc_node(sizeof(*td), GFP_KERNEL, cpu_to_node(cpu));
> + if (!td)
> + return -ENOMEM;
> + td->cpu = cpu;
> + td->ht = ht;
> +
> + tsk = kthread_create_on_cpu(ht->thread_fn, td, cpu, ht->thread_comm);
> + if (IS_ERR(tsk))
> + return PTR_ERR(tsk);
> +
> + get_task_struct(tsk);
> + *per_cpu_ptr(ht->store, cpu) = tsk;
> + return 0;
> +}
> +
> +int smpboot_create_threads(unsigned int cpu)
> +{
> + struct smp_hotplug_thread *cur;
> + int ret = 0;
> +
> + mutex_lock(&smpboot_threads_lock);
> + list_for_each_entry(cur, &hotplug_threads, list) {
> + ret = __smpboot_create_thread(cur, cpu);
> + if (ret)
> + break;
> + }
> + mutex_unlock(&smpboot_threads_lock);
> + return ret;
> +}
> +
> +static void smpboot_unpark_thread(struct smp_hotplug_thread *ht, unsigned int cpu)
> +{
> + struct task_struct *tsk = *per_cpu_ptr(ht->store, cpu);
> +
> + kthread_unpark(tsk);
> +}
> +
> +void smpboot_unpark_threads(unsigned int cpu)
> +{
> + struct smp_hotplug_thread *cur;
> +
> + mutex_lock(&smpboot_threads_lock);
> + list_for_each_entry(cur, &hotplug_threads, list)
> + smpboot_unpark_thread(cur, cpu);
> + mutex_unlock(&smpboot_threads_lock);
> +}
> +
> +static void smpboot_park_thread(struct smp_hotplug_thread *ht, unsigned int cpu)
> +{
> + struct task_struct *tsk = *per_cpu_ptr(ht->store, cpu);
> +
> + if (tsk)
> + kthread_park(tsk);
> +}
> +
> +void smpboot_park_threads(unsigned int cpu)
> +{
> + struct smp_hotplug_thread *cur;
> +
> + mutex_lock(&smpboot_threads_lock);
> + list_for_each_entry(cur, &hotplug_threads, list)

Shouldn't this be list_for_each_entry_reverse()? Yes, the notifiers
still run in the same order for both online and offline, but all uses
of smpboot_park_threads() would be new, so should be OK with the
proper ordering, right?

> + smpboot_park_thread(cur, cpu);
> + mutex_unlock(&smpboot_threads_lock);
> +}
> +
> +static void smpboot_destroy_threads(struct smp_hotplug_thread *ht)
> +{
> + unsigned int 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);
> +
> + if (tsk) {
> + kthread_stop(tsk);
> + put_task_struct(tsk);
> + *per_cpu_ptr(ht->store, cpu) = NULL;
> + }
> + }
> +}
> +
> +/**
> + * smpboot_register_percpu_thread - Register a per_cpu thread related to hotplug
> + * @plug_thread: Hotplug thread descriptor
> + *
> + * Creates and starts the threads on all online cpus.
> + */
> +int smpboot_register_percpu_thread(struct smp_hotplug_thread *plug_thread)
> +{
> + unsigned int cpu;
> + int ret = 0;
> +
> + mutex_lock(&smpboot_threads_lock);
> + for_each_online_cpu(cpu) {
> + ret = __smpboot_create_thread(plug_thread, cpu);
> + if (ret) {
> + smpboot_destroy_threads(plug_thread);
> + goto out;
> + }
> + smpboot_unpark_thread(plug_thread, cpu);
> + }
> + list_add(&plug_thread->list, &hotplug_threads);
> +out:
> + mutex_unlock(&smpboot_threads_lock);
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(smpboot_register_percpu_thread);
> +
> +/**
> + * smpboot_unregister_percpu_thread - Unregister a per_cpu thread related to hotplug
> + * @plug_thread: Hotplug thread descriptor
> + *
> + * Stops all threads on all possible cpus.
> + */
> +void smpboot_unregister_percpu_thread(struct smp_hotplug_thread *plug_thread)
> +{
> + get_online_cpus();
> + mutex_lock(&smpboot_threads_lock);
> + list_del(&plug_thread->list);
> + smpboot_destroy_threads(plug_thread);
> + mutex_unlock(&smpboot_threads_lock);
> + put_online_cpus();
> +}
> +EXPORT_SYMBOL_GPL(smpboot_unregister_percpu_thread);
> Index: tip/kernel/smpboot.h
> ===================================================================
> --- tip.orig/kernel/smpboot.h
> +++ tip/kernel/smpboot.h
> @@ -13,4 +13,8 @@ static inline void idle_thread_set_boot_
> static inline void idle_threads_init(void) { }
> #endif
>
> +int smpboot_create_threads(unsigned int cpu);
> +void smpboot_park_threads(unsigned int cpu);
> +void smpboot_unpark_threads(unsigned int cpu);
> +
> #endif
>
>