Re: [PATCH v5 2/2] watchdog: add watchdog_exclude sysctl to assist nohz

[Frederic Weisbecker <fweisbec@gmail.com>]

On Wed, Apr 08, 2015 at 03:21:00PM -0400, Chris Metcalf wrote:
> On 04/08/2015 10:01 AM, Frederic Weisbecker wrote:
> >How about creating smpboot_update_mask_percpu_thread() and handle it from smpboot,
> >this way future evolutions of smpboot won't overlook this cpumask live change?
> 
> It seemed like your proposed approach was actually a bit heavier-weight
> from the perspective of generic smp_hotplug_thread, so instead I just
> modified the proposed API to have a simple "valid_cpu()" callback,
> which I think is clear and won't be damaged by smpboot evolution.
> Let me know what you think.

You mean have the cpumask private to watchdog and implement valid_cpu()
on top of it?

Well this just pulls all the complexity to the smpboot thread instead of
the smpboot subsystem. If you implement it to smpboot, this will be reusable
for other possible smpboot threads than watchdog.

Eventually if you take into account Thomas review that we should rather park
cpu threads that aren't included in the cpumask, this should look like this
(warning: totally untested):

diff --git a/kernel/smpboot.c b/kernel/smpboot.c
index 40190f2..01bfb51 100644
--- a/kernel/smpboot.c
+++ b/kernel/smpboot.c
@@ -230,8 +230,10 @@ 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);
+	list_for_each_entry(cur, &hotplug_threads, list) {
+		if (cpumask_test_cpu(cpu, cur->cpumask))
+			smpboot_unpark_thread(cur, cpu);
+	}
 	mutex_unlock(&smpboot_threads_lock);
 }
 
@@ -288,7 +290,8 @@ int smpboot_register_percpu_thread(struct smp_hotplug_thread *plug_thread)
 			smpboot_destroy_threads(plug_thread);
 			goto out;
 		}
-		smpboot_unpark_thread(plug_thread, cpu);
+		if (cpumask_test_cpu(plug_thread->cpumask))
+			smpboot_unpark_thread(plug_thread, cpu);
 	}
 	list_add(&plug_thread->list, &hotplug_threads);
 out:
@@ -298,6 +301,41 @@ out:
 }
 EXPORT_SYMBOL_GPL(smpboot_register_percpu_thread);
 
+int smpboot_update_cpumask_percpu_thread(struct smp_hotplug_thread *plug_thread,
+					 cpumask_var_t new)
+{
+	cpumask_var_t tmp;
+	unsigned int cpu;
+
+	tmp = alloc_cpumask_var(&tmp, GFP_KERNEL);
+	if (!tmp)
+		return -ENOMEM;
+
+	get_online_cpus();
+	mutex_lock(&smpboot_threads_lock);
+
+	/* Park those that were exclusively enabled on old mask */
+	cpumask_andnot(tmp, plug_thread->cpumask, new);
+	for_each_cpu(cpu, tmp)
+		smpboot_park_thread(plug_thread, cpu);
+
+	/* Unpark those that are exclusively enabled on new mask */
+	cpumask_andnot(tmp, new, plug_thread->cpumask);
+	for_each_cpu(cpu, tmp) {
+		if (cpu_online(cpu))
+			smpboot_unpark_thread(plug_thread, cpu);
+	}
+	cpumask_copy(plug_thread->cpumask, new);
+
+	mutex_unlock(&smpboot_threads_lock);
+	put_online_cpus();
+
+	free_cpumask_var(tmp);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(smpboot_register_percpu_thread);
+
 /**
  * smpboot_unregister_percpu_thread - Unregister a per_cpu thread related to hotplug
  * @plug_thread:	Hotplug thread descriptor