From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754672AbZESQO7 (ORCPT ); Tue, 19 May 2009 12:14:59 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1753496AbZESQOw (ORCPT ); Tue, 19 May 2009 12:14:52 -0400 Received: from mx2.redhat.com ([66.187.237.31]:56361 "EHLO mx2.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753443AbZESQOw (ORCPT ); Tue, 19 May 2009 12:14:52 -0400 Date: Tue, 19 May 2009 18:09:36 +0200 From: Oleg Nesterov To: Johannes Berg Cc: Ingo Molnar , Zdenek Kabelac , "Rafael J. Wysocki" , Peter Zijlstra , Linux Kernel Mailing List Subject: Re: INFO: possible circular locking dependency at cleanup_workqueue_thread Message-ID: <20090519160936.GA25720@redhat.com> References: <20090517071834.GA8507@elte.hu> <1242559101.28127.63.camel@johannes.local> <20090518194749.GA3501@redhat.com> <1242723104.17164.5.camel@johannes.local> <20090519120010.GA14782@redhat.com> <1242747203.4797.39.camel@johannes.local> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1242747203.4797.39.camel@johannes.local> User-Agent: Mutt/1.5.18 (2008-05-17) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 05/19, Johannes Berg wrote: > > On Tue, 2009-05-19 at 14:00 +0200, Oleg Nesterov wrote: > > > > I'm not familiar enough with the code -- but what are we really trying > > > to do in CPU_POST_DEAD? It seems to me that at that time things must > > > already be off the CPU, so ...? > > > > Yes, this cpu is dead, we should do cleanup_workqueue_thread() to kill > > cwq->thread. > > > > > On the other hand that calls > > > flush_cpu_workqueue() so it seems it would actually wait for the work to > > > be executed on some other CPU, within the CPU_POST_DEAD notification? > > > > Yes. Because we can't just kill cwq->thread, we can have the pending > > work_structs so we have to flush. > > > > Why can't we move these works to another CPU? We can, but this doesn't > > really help. Because in any case we should at least wait for > > cwq->current_work to complete. > > > > Why do we use CPU_POST_DEAD, and not (say) CPU_DEAD to flush/kill ? > > Because work->func() can sleep in get_online_cpus(), we can't flush > > until we drop cpu_hotplug.lock. > > Right. But exactly this happens in the hibernate case -- not sure I understand your "exactly this" ;) But your explanation of the deadlock below looks great! > the hibernate > code calls kernel/cpu.c:disable_nonboot_cpus() which calls _cpu_down() > which calls raw_notifier_call_chain(&cpu_chain, CPU_POST_DEAD... Sadly, > it does so while holding the cpu_add_remove_lock, which is happens to > have the dependencies outlined in the original email... > > The same happens in cpu_down() (without leading _) which you can trigger > from sysfs by manually removing the CPU, so it's not hibernate specific. except I don't understand how cpu_add_remove_lock makes the difference... And thus I can't understand why cpu_down() (called lockless) have the same problems. Please see below. > Anyway, you can have a deadlock like this: > > CPU 3 CPU 2 CPU 1 > suspend/hibernate > something: > rtnl_lock() device_pm_lock() > -> mutex_lock(&dpm_list_mtx) > > mutex_lock(&dpm_list_mtx) > > linkwatch_work > -> rtnl_lock() > disable_nonboot_cpus() let's suppose disable_nonboot_cpus() does not take cpu_add_remove_lock, > -> flush CPU 3 workqueue in this case the deadlock is still here? We can't flush because we hold the lock (dpm_list_mtx) which depends on another lock taken by work->func(), the "classical" bug with flush. No? Oleg.