On Fri, Aug 09, 2019 at 03:28:56PM -0400, Daniel Jordan wrote:
> On a 5.2 kernel, lockdep complains when offlining a CPU and writing to a
> parallel_cpumask sysfs file.
>
> echo 0 > /sys/devices/system/cpu/cpu1/online
> echo ff > /sys/kernel/pcrypt/pencrypt/parallel_cpumask
>
> ======================================================
> WARNING: possible circular locking dependency detected
> 5.2.0-padata-base+ #19 Not tainted
> ------------------------------------------------------
> cpuhp/1/13 is trying to acquire lock:
> ... (&pinst->lock){+.+.}, at: padata_cpu_prep_down+0x37/0x70
>
> but task is already holding lock:
> ... (cpuhp_state-down){+.+.}, at: cpuhp_thread_fun+0x34/0x240
>
> which lock already depends on the new lock.
>
> padata doesn't take cpu_hotplug_lock and pinst->lock in a consistent
> order. Which should be first? CPU hotplug calls into padata with
> cpu_hotplug_lock already held, so it should have priority.
Yeah this is clearly a bug but I think we need tackle something
else first.
> diff --git a/kernel/padata.c b/kernel/padata.c
> index b60cc3dcee58..d056276a96ce 100644
> --- a/kernel/padata.c
> +++ b/kernel/padata.c
> @@ -487,9 +487,7 @@ static void __padata_stop(struct padata_instance *pinst)
>
> synchronize_rcu();
>
> - get_online_cpus();
> padata_flush_queues(pinst->pd);
> - put_online_cpus();
> }
As I pointed earlier, the whole concept of flushing the queues is
suspect. So we should tackle that first and it may obviate the need
to do get_online_cpus completely if the flush call disappears.
My main worry is that you're adding an extra lock around synchronize_rcu
and that is always something that should be done only after careful
investigation.
Cheers,
--
Email: Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
