From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753487AbcBAQQw (ORCPT ); Mon, 1 Feb 2016 11:16:52 -0500 Received: from mail-pa0-f65.google.com ([209.85.220.65]:34474 "EHLO mail-pa0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753248AbcBAQQu (ORCPT ); Mon, 1 Feb 2016 11:16:50 -0500 Date: Tue, 2 Feb 2016 01:14:58 +0900 From: Sergey Senozhatsky To: Peter Hurley Cc: Byungchul Park , akpm@linux-foundation.org, mingo@kernel.org, linux-kernel@vger.kernel.org, akinobu.mita@gmail.com, jack@suse.cz, torvalds@linux-foundation.org, Sergey Senozhatsky , Sergey Senozhatsky Subject: Re: [PATCH 3/3] spinlock_debug: panic on recursive lock spin_dump() Message-ID: <20160201161458.GA609@swordfish> References: <20160131123041.GA1306@swordfish> <1454243623-15109-1-git-send-email-sergey.senozhatsky@gmail.com> <1454243623-15109-3-git-send-email-sergey.senozhatsky@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1454243623-15109-3-git-send-email-sergey.senozhatsky@gmail.com> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On (01/31/16 21:33), Sergey Senozhatsky wrote: > spin_dump() calls printk() which can attempt to reacquire the > 'buggy' lock (one of printk's lock, or console device driver lock, > etc.) and thus spin_dump() will recurse into itself. how about splitting ->owner_cpu 4 bytes as: | | 1 byte spin bug recursion | 1 byte spin_dump recursion counter | 2 bytes owner cpu | | ? so we have 2 bytes to store the lock owner's smp_processor_id(). (I just don't want to extend spin lock with another 4 bytes to fix the recursion.) spin_dump() will increment lock's spin_dump recursion counter and decrement it upon the exit from spin_dump(). if lock's recursion counter is equal to U8_MAX -- panic() the system, we have recursed spin_dump() into spin_dump() enough... (U8_MAX recursive printk->spin_dump->printk calls look big enough). for spin_bug() recursion we don't have to maintain any counter: set SPIN_BUG_RECURSION bit at the beginning of spin_bug() and clear it at the end. if the lock already has SPIN_BUG_RECURSION bit set -- panic() the system. compile tested only. will test it tomorrow. --- kernel/locking/spinlock_debug.c | 32 +++++++++++++++++++++++++++----- 1 file changed, 27 insertions(+), 5 deletions(-) diff --git a/kernel/locking/spinlock_debug.c b/kernel/locking/spinlock_debug.c index 0374a59..54bcc3b 100644 --- a/kernel/locking/spinlock_debug.c +++ b/kernel/locking/spinlock_debug.c @@ -13,6 +13,9 @@ #include #include +#define SPIN_DUMP_RECURSION (1 << 16) +#define SPIN_BUG_RECURSION (1 << 31) + void __raw_spin_lock_init(raw_spinlock_t *lock, const char *name, struct lock_class_key *key) { @@ -26,7 +29,7 @@ void __raw_spin_lock_init(raw_spinlock_t *lock, const char *name, lock->raw_lock = (arch_spinlock_t)__ARCH_SPIN_LOCK_UNLOCKED; lock->magic = SPINLOCK_MAGIC; lock->owner = SPINLOCK_OWNER_INIT; - lock->owner_cpu = -1; + lock->owner_cpu = USHRT_MAX; } EXPORT_SYMBOL(__raw_spin_lock_init); @@ -44,14 +47,26 @@ void __rwlock_init(rwlock_t *lock, const char *name, lock->raw_lock = (arch_rwlock_t) __ARCH_RW_LOCK_UNLOCKED; lock->magic = RWLOCK_MAGIC; lock->owner = SPINLOCK_OWNER_INIT; - lock->owner_cpu = -1; + lock->owner_cpu = USHRT_MAX; } EXPORT_SYMBOL(__rwlock_init); +static void spin_recursion_panic(raw_spinlock_t *lock, const char *msg) +{ + panic("BUG: lock: %pS %s recursion on CPU#%d, %s/%d\n", + lock, msg, raw_smp_processor_id(), + current->comm, task_pid_nr(current)); +} + static void spin_dump(raw_spinlock_t *lock, const char *msg) { struct task_struct *owner = NULL; + u8 recursion_counter = (u8)(lock->owner_cpu >> 16); + + lock->owner_cpu += SPIN_DUMP_RECURSION; + if ((recursion_counter + 1) == U8_MAX) + spin_recursion_panic(lock, "spin_dump()"); if (lock->owner && lock->owner != SPINLOCK_OWNER_INIT) owner = lock->owner; @@ -63,8 +78,10 @@ static void spin_dump(raw_spinlock_t *lock, const char *msg) lock, lock->magic, owner ? owner->comm : "", owner ? task_pid_nr(owner) : -1, - lock->owner_cpu); + lock->owner_cpu == USHRT_MAX ? -1 : lock->owner_cpu); dump_stack(); + + lock->owner_cpu -= SPIN_DUMP_RECURSION; } static void spin_bug(raw_spinlock_t *lock, const char *msg) @@ -72,7 +89,12 @@ static void spin_bug(raw_spinlock_t *lock, const char *msg) if (!debug_locks_off()) return; + if (lock->owner_cpu & SPIN_BUG_RECURSION) + spin_recursion_panic(lock, "spin_bug()"); + + lock->owner_cpu |= SPIN_BUG_RECURSION; spin_dump(lock, msg); + lock->owner_cpu &= ~SPIN_BUG_RECURSION; } #define SPIN_BUG_ON(cond, lock, msg) if (unlikely(cond)) spin_bug(lock, msg) @@ -100,7 +122,7 @@ static inline void debug_spin_unlock(raw_spinlock_t *lock) SPIN_BUG_ON(lock->owner_cpu != raw_smp_processor_id(), lock, "wrong CPU"); lock->owner = SPINLOCK_OWNER_INIT; - lock->owner_cpu = -1; + lock->owner_cpu = USHRT_MAX; } static void __spin_lock_debug(raw_spinlock_t *lock) @@ -244,7 +266,7 @@ static inline void debug_write_unlock(rwlock_t *lock) RWLOCK_BUG_ON(lock->owner_cpu != raw_smp_processor_id(), lock, "wrong CPU"); lock->owner = SPINLOCK_OWNER_INIT; - lock->owner_cpu = -1; + lock->owner_cpu = USHRT_MAX; } #if 0 /* This can cause lockups */ -- 2.7.0.75.g3ee1e0f