Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1759469AbXJOGTW (ORCPT ); Mon, 15 Oct 2007 02:19:22 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1754181AbXJOGTN (ORCPT ); Mon, 15 Oct 2007 02:19:13 -0400 Received: from e35.co.us.ibm.com ([32.97.110.153]:50826 "EHLO e35.co.us.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753677AbXJOGTM (ORCPT ); Mon, 15 Oct 2007 02:19:12 -0400 Date: Mon, 15 Oct 2007 11:48:44 +0530 From: Gautham R Shenoy To: Andrew Morton Cc: linux-kernel@vger.kernel.org, a.p.zijlstra@chello.nl, Dipankar Sarma Subject: [PATCH] Add irq protection in the percpu-counters cpu-hotplug-callback path Message-ID: <20071015061844.GA15728@in.ibm.com> Reply-To: ego@in.ibm.com References: <20071011213126.cf92efb7.akpm@linux-foundation.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20071011213126.cf92efb7.akpm@linux-foundation.org> User-Agent: Mutt/1.5.12-2006-07-14 Sender: linux-kernel-owner@vger.kernel.org X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5515 Lines: 148 Hi Andrew, While running regular cpu-offline tests on 2.6.23-mm1, I hit the following lockdep warning. It was triggered because some of the per-cpu counters and thus their locks are accessed from IRQ context. This can cause a deadlock if it interrupts a cpu-offline thread which is transferring a dead-cpu's counts to the global counter. Please find the patch for the same below. Tested on i386. Thanks and Regards gautham. =====================Warning! =========================================== [root@llm43]# ./all_hotplug_once CPU 1 is now offline ================================= [ INFO: inconsistent lock state ] 2.6.23-mm1 #3 --------------------------------- inconsistent {in-softirq-W} -> {softirq-on-W} usage. sh/7103 [HC0[0]:SC0[0]:HE1:SE1] takes: (&percpu_counter_irqsafe){-+..}, at: [] percpu_counter_hotcpu_callback+0x22/0x67 {in-softirq-W} state was registered at: [] __lock_acquire+0x40d/0xb4a [] __lock_acquire+0xb04/0xb4a [] lock_acquire+0x5f/0x79 [] __percpu_counter_add+0x62/0xad [] _spin_lock+0x21/0x2c [] __percpu_counter_add+0x62/0xad [] __percpu_counter_add+0x62/0xad [] test_clear_page_writeback+0x88/0xc5 [] end_page_writeback+0x20/0x3c [] end_buffer_async_write+0x133/0x181 [] __lock_acquire+0xb04/0xb4a [] end_bio_bh_io_sync+0x21/0x29 [] end_bio_bh_io_sync+0x0/0x29 [] bio_endio+0x27/0x29 [] dec_pending+0x17d/0x199 [] clone_endio+0x73/0x9f [] clone_endio+0x0/0x9f [] bio_endio+0x27/0x29 [] __end_that_request_first+0x150/0x2c0 [] scsi_end_request+0x1d/0xab [] mempool_free+0x63/0x67 [] scsi_io_completion+0x108/0x2c7 [] blk_done_softirq+0x51/0x5c [] __do_softirq+0x68/0xdb [] do_softirq+0x36/0x51 [] irq_exit+0x43/0x4e [] do_IRQ+0x73/0x83 [] common_interrupt+0x2e/0x34 [] add_to_swap+0x23/0x66 [] mwait_idle_with_hints+0x3b/0x3f [] mwait_idle+0x0/0xf [] cpu_idle+0x9a/0xc7 [] 0xffffffff irq event stamp: 4007 hardirqs last enabled at (4007): [] __mutex_lock_slowpath+0x21d/0x241 hardirqs last disabled at (4006): [] __mutex_lock_slowpath+0x5b/0x241 softirqs last enabled at (2130): [] __rcu_offline_cpu+0x2f/0x5a softirqs last disabled at (2128): [] _spin_lock_bh+0x8/0x31 other info that might help us debug this: 6 locks held by sh/7103: #0: (&buffer->mutex){--..}, at: [] sysfs_write_file+0x22/0xdb #1: (cpu_add_remove_lock){--..}, at: [] cpu_down+0x13/0x36 #2: (sched_hotcpu_mutex){--..}, at: [] migration_call+0x26/0x36a #3: (cache_chain_mutex){--..}, at: [] cpuup_callback+0x28/0x1f9 #4: (workqueue_mutex){--..}, at: [] workqueue_cpu_callback+0x26/0xca #5: (percpu_counters_lock){--..}, at: [] percpu_counter_hotcpu_callback+0x13/0x67 stack backtrace: [] print_usage_bug+0x101/0x10b [] mark_lock+0x249/0x3f0 [] __lock_acquire+0x474/0xb4a [] lock_acquire+0x5f/0x79 [] percpu_counter_hotcpu_callback+0x22/0x67 [] _spin_lock+0x21/0x2c [] percpu_counter_hotcpu_callback+0x22/0x67 [] percpu_counter_hotcpu_callback+0x22/0x67 [] notifier_call_chain+0x2a/0x47 [] raw_notifier_call_chain+0x9/0xc [] _cpu_down+0x174/0x221 [] cpu_down+0x25/0x36 [] store_online+0x24/0x56 [] store_online+0x0/0x56 [] sysdev_store+0x1e/0x22 [] sysfs_write_file+0xa7/0xdb [] sysfs_write_file+0x0/0xdb [] vfs_write+0x83/0xf6 [] sys_write+0x3c/0x63 [] sysenter_past_esp+0x5f/0x99 ======================= ---> From: Gautham R Shenoy Some of the per-cpu counters and thus their locks are accessed from IRQ contexts. This can cause a deadlock if it interrupts a cpu-offline thread which is transferring a dead-cpu's counts to the global counter. Add appropriate IRQ protection in the cpu-hotplug callback path. Signed-off-by: Gautham R Shenoy --- lib/percpu_counter.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) Index: linux-2.6.23/lib/percpu_counter.c =================================================================== --- linux-2.6.23.orig/lib/percpu_counter.c +++ linux-2.6.23/lib/percpu_counter.c @@ -124,12 +124,13 @@ static int __cpuinit percpu_counter_hotc mutex_lock(&percpu_counters_lock); list_for_each_entry(fbc, &percpu_counters, list) { s32 *pcount; + unsigned long flags; - spin_lock(&fbc->lock); + spin_lock_irqsave(&fbc->lock, flags); pcount = per_cpu_ptr(fbc->counters, cpu); fbc->count += *pcount; *pcount = 0; - spin_unlock(&fbc->lock); + spin_unlock_irqrestore(&fbc->lock, flags); } mutex_unlock(&percpu_counters_lock); return NOTIFY_OK; -- Gautham R Shenoy Linux Technology Center IBM India. "Freedom comes with a price tag of responsibility, which is still a bargain, because Freedom is priceless!" - 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/