Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1764132AbXHFLGT (ORCPT ); Mon, 6 Aug 2007 07:06:19 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1759225AbXHFLGJ (ORCPT ); Mon, 6 Aug 2007 07:06:09 -0400 Received: from merkurneu.hrz.uni-giessen.de ([134.176.2.3]:63658 "EHLO merkurneu.hrz.uni-giessen.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1760570AbXHFLGH (ORCPT ); Mon, 6 Aug 2007 07:06:07 -0400 From: Marc Dietrich To: Johannes Berg Subject: Re: [NFS] 2.6.23-rc1-mm2 Date: Mon, 6 Aug 2007 13:05:23 +0200 User-Agent: KMail/1.9.5 Cc: Oleg Nesterov , Trond Myklebust , Andrew Morton , Neil Brown , nfs@lists.sourceforge.net, linux-kernel@vger.kernel.org, Ingo Molnar , Peter Zijlstra References: <20070731230932.a9459617.akpm@linux-foundation.org> <20070803172137.GA3783@tv-sign.ru> <1186381496.21957.30.camel@johannes.berg> In-Reply-To: <1186381496.21957.30.camel@johannes.berg> MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-15" Content-Transfer-Encoding: 7bit Content-Disposition: inline Message-Id: <200708061305.37208.marc.dietrich@ap.physik.uni-giessen.de> X-HRZ-JLUG-MailScanner-Information: Passed JLUG virus check X-HRZ-JLUG-MailScanner: No virus found X-MailScanner-From: marc.dietrich@ap.physik.uni-giessen.de Sender: linux-kernel-owner@vger.kernel.org X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 6049 Lines: 149 Hi, Am Monday 06 August 2007 08:24 schrieb Johannes Berg: > On Fri, 2007-08-03 at 21:21 +0400, Oleg Nesterov wrote: > > To avoid a possible confusion: it is still OK if work->func() flushes > > its own workqueue, so strictly speaking this trace is false positive, > > but it would be very nice if we can get rid of this practice. > > I just had a thought: we could get rid of this warning by using a > read-lock here. That way, flushing from within a work function (which > would be seen as read-after-read recursive lock) won't trigger this > warning. Patch below. This would, however, also get rid of any warnings > for run_workqueue recursion. Which again we may or may not want, the > code inidicates that it should be allowed up to a depth of three. > > However, the question whether we should allow flush_workqueue from > within a struct work is mainly an API policy issue; it doesn't hurt to > flush a workqueue from within a work, but it is probably nearer the > intent to use targeted cancel_work_sync() or such. OTOH, one could > imagine situations where multiple different work structs are on that > workqueue belonging to the same subsystem and then the general > flush_scheduled_work() call is the only way to guarantee nothing is on > scheduled at a given point... I don't feel qualified to make the > decision for or against allowing this use of the API at this point. > > Marc, do you have an easy way to trigger this warning? Could you verify > that it goes away with the patch below applied? just booting into X is enough. I applied the patch, but now I get: ================================= [ INFO: inconsistent lock state ] 2.6.23-rc1-mm2 #4 --------------------------------- inconsistent {softirq-on-W} -> {in-softirq-W} usage. swapper/0 [HC0[0]:SC1[1]:HE1:SE0] takes: (rpc_credcache_lock){-+..}, at: [] _atomic_dec_and_lock+0x17/0x60 {softirq-on-W} state was registered at: [] __lock_acquire+0x650/0x1030 [] lock_acquire+0x61/0x80 [] _spin_lock+0x2c/0x40 [] _atomic_dec_and_lock+0x17/0x60 [] put_rpccred+0x5d/0x100 [sunrpc] [] rpcauth_unbindcred+0x21/0x60 [sunrpc] [] a0 [sunrpc] [] rpc_call_sync+0x30/0x40 [sunrpc] [] rpcb_register+0xdb/0x180 [sunrpc] [] svc_register+0x93/0x160 [sunrpc] [] __svc_create+0x1ee/0x220 [sunrpc] [] svc_create+0x13/0x20 [sunrpc] [] nfs_callback_up+0x82/0x120 [nfs] [] nfs_get_client+0x176/0x390 [nfs] [] nfs4_set_client+0x31/0x190 [nfs] [] nfs4_create_server+0x63/0x3b0 [nfs] [] nfs4_get_sb+0x346/0x5b0 [nfs] [] vfs_kern_mount+0x94/0x110 [] do_mount+0x1f2/0x7d0 [] sys_mount+0x66/0xa0 [] syscall_call+0x7/0xb [] 0xffffffff irq event stamp: 5277830 hardirqs last enabled at (5277830): [] kmem_cache_free+0x8a/0xc0 hardirqs last disabled at (5277829): [] kmem_cache_free+0x52/0xc0 softirqs last enabled at (5277798): [] __do_softirq+0xa3/0xc0 softirqs last disabled at (5277817): [] do_softirq+0x47/0x50 other info that might help us debug this: no locks held by swapper/0. stack backtrace: [] show_trace_log_lvl+0x1a/0x30 [] show_trace+0x12/0x20 [] dump_stack+0x15/0x20 [] print_usage_bug+0x153/0x160 [] mark_lock+0x449/0x620 [] __lock_acquire+0x604/0x1030 [] lock_acquire+0x61/0x80 [] _spin_lock+0x2c/0x40 [] _atomic_dec_and_lock+0x17/0x60 [] put_rpccred+0x5d/0x100 [sunrpc] [] nfs_free_delegation_callback+0x13/0x20 [nfs] [] __rcu_process_callbacks+0x6a/0x1c0 [] rcu_process_callbacks+0x12/0x30 [] tasklet_action+0x38/0x80 [] __do_softirq+0x55/0xc0 [] do_softirq+0x47/0x50 [] irq_exit+0x35/0x40 [] smp_apic_timer_interrupt+0x43/0x80 [] apic_timer_interrupt+0x33/0x38 [] cpuidle_idle_call+0x6f/0x90 [] cpu_idle+0x43/0x70 [] rest_init+0x47/0x50 [] start_kernel+0x22a/0x2b0 [<00000000>] 0x0 ======================= also, sometimes this kernel hangs because of nfs accessing processes remain in D state. Marc > johannes > > --- > kernel/workqueue.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > --- wireless-dev.orig/kernel/workqueue.c 2007-08-06 08:11:23.297846657 > +0200 +++ wireless-dev/kernel/workqueue.c 2007-08-06 08:19:54.727846657 > +0200 @@ -272,7 +272,7 @@ static void run_workqueue(struct cpu_wor > > BUG_ON(get_wq_data(work) != cwq); > work_clear_pending(work); > - lock_acquire(&cwq->wq->lockdep_map, 0, 0, 0, 2, _THIS_IP_); > + lock_acquire(&cwq->wq->lockdep_map, 0, 0, 1, 2, _THIS_IP_); > lock_acquire(&lockdep_map, 0, 0, 0, 2, _THIS_IP_); > f(work); > lock_release(&lockdep_map, 1, _THIS_IP_); > @@ -395,7 +395,7 @@ void fastcall flush_workqueue(struct wor > int cpu; > > might_sleep(); > - lock_acquire(&wq->lockdep_map, 0, 0, 0, 2, _THIS_IP_); > + lock_acquire(&wq->lockdep_map, 0, 0, 1, 2, _THIS_IP_); > lock_release(&wq->lockdep_map, 1, _THIS_IP_); > for_each_cpu_mask(cpu, *cpu_map) > flush_cpu_workqueue(per_cpu_ptr(wq->cpu_wq, cpu)); > @@ -779,7 +779,7 @@ static void cleanup_workqueue_thread(str > if (cwq->thread == NULL) > return; > > - lock_acquire(&cwq->wq->lockdep_map, 0, 0, 0, 2, _THIS_IP_); > + lock_acquire(&cwq->wq->lockdep_map, 0, 0, 1, 2, _THIS_IP_); > lock_release(&cwq->wq->lockdep_map, 1, _THIS_IP_); > > flush_cpu_workqueue(cwq); -- "The enemy uses unauthorized weapons." Lord Arthur Ponsonby, "Falsehood in Wartime: Propaganda Lies of the First World War", 1928 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/