On 8/21/19 11:50 PM, Herbert Xu wrote:
On Fri, Aug 09, 2019 at 05:06:03PM -0400, Daniel Jordan wrote:
diff --git a/kernel/padata.c b/kernel/padata.c
index d056276a96ce..01460ea1d160 100644
--- a/kernel/padata.c
+++ b/kernel/padata.c
@@ -702,10 +702,7 @@ static int __padata_remove_cpu(struct padata_instance
*pinst, int cpu)
struct parallel_data *pd = NULL;
if (cpumask_test_cpu(cpu, cpu_online_mask)) {
-
- if (!padata_validate_cpumask(pinst, pinst->cpumask.pcpu) ||
- !padata_validate_cpumask(pinst, pinst->cpumask.cbcpu))
- __padata_stop(pinst);
+ __padata_stop(pinst);
pd = padata_alloc_pd(pinst, pinst->cpumask.pcpu,
pinst->cpumask.cbcpu);
@@ -716,6 +713,9 @@ static int __padata_remove_cpu(struct padata_instance
*pinst, int cpu)
cpumask_clear_cpu(cpu, pd->cpumask.cbcpu);
cpumask_clear_cpu(cpu, pd->cpumask.pcpu);
+ if (padata_validate_cpumask(pinst, pd->cpumask.pcpu) &&
+ padata_validate_cpumask(pinst, pd->cpumask.cbcpu))
+ __padata_start(pinst);
}
I looked back at the original code and in fact the original
assumption is to call this after cpu_online_mask has been modified.
So I suspect we need to change the state at which this is called
by CPU hotplug.
Yes the state idea is good, it's cleaner to have the CPU out of the online mask
ahead of time.
I think we'll need two states. We want a CPU being offlined to already be
removed from the online cpumask so and'ing the user-supplied and online masks
reflects conditions after the hotplug operation is finished. For the same
reason we want a CPU being onlined to already be in the online mask, and we can
use the existing hotplug state for that, though we'd need a new padata-specific
state for the offline case.
IOW the commit that broke this is 30e92153b4e6.
I don't think 30e92153b4e6 is the one since the commit before that only allows
__padata_remove_cpu to do its work if @cpu is in the online mask, so the call
happens before cpu_online_mask has been modified. Same story for the very
first padata commit, so it seems like that should actually be Fixes.
This would also allow us to get rid of the two cpumask_clear_cpu
calls on pd->cpumask which is just bogus as you should only ever
modify the pd->cpumask prior to the padata_repalce call (because
the readers are not serialised with respect to this).
Yeah, makes sense.
Daniel
