* [PATCH v2] lib/spinlock_debug.c: prevent an infinite recursive cycle in spin_dump()
@ 2016-01-18 0:58 Byungchul Park
2016-01-19 1:53 ` Byungchul Park
2016-01-27 0:11 ` Andrew Morton
0 siblings, 2 replies; 7+ messages in thread
From: Byungchul Park @ 2016-01-18 0:58 UTC (permalink / raw)
To: akpm; +Cc: mingo, linux-kernel, akinobu.mita, jack, torvalds
It causes an infinite recursive cycle when using CONFIG_DEBUG_SPINLOCK,
in the spin_dump(). Backtrace prints printk() -> console_trylock() ->
do_raw_spin_lock() -> spint_bug() -> spin_dump() -> printk()...
infinitely.
If the spin_bug() is called from a function like printk() which is
trying to obtain the console lock, we should prevent the debug spinlock
code from calling printk() again in that context.
Signed-off-by: Byungchul Park <byungchul.park@lge.com>
---
kernel/locking/spinlock_debug.c | 11 +++++++++++
kernel/printk/printk.c | 5 +++++
2 files changed, 16 insertions(+)
diff --git a/kernel/locking/spinlock_debug.c b/kernel/locking/spinlock_debug.c
index 0374a59..e745973 100644
--- a/kernel/locking/spinlock_debug.c
+++ b/kernel/locking/spinlock_debug.c
@@ -67,11 +67,22 @@ static void spin_dump(raw_spinlock_t *lock, const char *msg)
dump_stack();
}
+extern int is_console_lock(raw_spinlock_t *lock);
+
static void spin_bug(raw_spinlock_t *lock, const char *msg)
{
if (!debug_locks_off())
return;
+ /*
+ * If this function is called from a function like printk()
+ * which is trying to obtain the console lock, then we should
+ * not call printk() any more. Or it will cause an infinite
+ * recursive cycle!
+ */
+ if (unlikely(is_console_lock(lock)))
+ return;
+
spin_dump(lock, msg);
}
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 2ce8826..50ea552 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -119,6 +119,11 @@ static int __down_trylock_console_sem(unsigned long ip)
up(&console_sem);\
} while (0)
+int is_console_lock(raw_spinlock_t *lock)
+{
+ return &console_sem.lock == lock;
+}
+
/*
* This is used for debugging the mess that is the VT code by
* keeping track if we have the console semaphore held. It's
--
1.9.1
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH v2] lib/spinlock_debug.c: prevent an infinite recursive cycle in spin_dump()
2016-01-18 0:58 [PATCH v2] lib/spinlock_debug.c: prevent an infinite recursive cycle in spin_dump() Byungchul Park
@ 2016-01-19 1:53 ` Byungchul Park
2016-01-21 8:12 ` Byungchul Park
2016-01-27 0:11 ` Andrew Morton
1 sibling, 1 reply; 7+ messages in thread
From: Byungchul Park @ 2016-01-19 1:53 UTC (permalink / raw)
To: akpm; +Cc: mingo, linux-kernel, akinobu.mita, jack, torvalds
I missed the version change desciption when I sent this v2. This patch
worked for me. Please let me know if you have other opinions.
Changes from v1 to v2
- only change comment and commit message esp. replacing "deadlock"
with "infinite recursive cycle", since it is not an actual deadlock.
Thanks,
Byungchul
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] lib/spinlock_debug.c: prevent an infinite recursive cycle in spin_dump()
2016-01-19 1:53 ` Byungchul Park
@ 2016-01-21 8:12 ` Byungchul Park
0 siblings, 0 replies; 7+ messages in thread
From: Byungchul Park @ 2016-01-21 8:12 UTC (permalink / raw)
To: akpm; +Cc: mingo, linux-kernel, akinobu.mita, jack, torvalds
On Tue, Jan 19, 2016 at 10:53:56AM +0900, Byungchul Park wrote:
> I missed the version change desciption when I sent this v2. This patch
> worked for me. Please let me know if you have other opinions.
>
> Changes from v1 to v2
> - only change comment and commit message esp. replacing "deadlock"
> with "infinite recursive cycle", since it is not an actual deadlock.
I was careless. I think it should be fixed by another way, instead of
the way this patch suggested.
>
> Thanks,
> Byungchul
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] lib/spinlock_debug.c: prevent an infinite recursive cycle in spin_dump()
2016-01-18 0:58 [PATCH v2] lib/spinlock_debug.c: prevent an infinite recursive cycle in spin_dump() Byungchul Park
2016-01-19 1:53 ` Byungchul Park
@ 2016-01-27 0:11 ` Andrew Morton
2016-01-27 1:13 ` Peter Hurley
2016-01-27 2:00 ` Byungchul Park
1 sibling, 2 replies; 7+ messages in thread
From: Andrew Morton @ 2016-01-27 0:11 UTC (permalink / raw)
To: Byungchul Park; +Cc: mingo, linux-kernel, akinobu.mita, jack, torvalds
On Mon, 18 Jan 2016 09:58:12 +0900 Byungchul Park <byungchul.park@lge.com> wrote:
> It causes an infinite recursive cycle when using CONFIG_DEBUG_SPINLOCK,
> in the spin_dump(). Backtrace prints printk() -> console_trylock() ->
> do_raw_spin_lock() -> spint_bug() -> spin_dump() -> printk()...
> infinitely.
>
> If the spin_bug() is called from a function like printk() which is
> trying to obtain the console lock, we should prevent the debug spinlock
> code from calling printk() again in that context.
>
lol. Excellent.
> --- a/kernel/locking/spinlock_debug.c
> +++ b/kernel/locking/spinlock_debug.c
> @@ -67,11 +67,22 @@ static void spin_dump(raw_spinlock_t *lock, const char *msg)
> dump_stack();
> }
>
> +extern int is_console_lock(raw_spinlock_t *lock);
> +
> static void spin_bug(raw_spinlock_t *lock, const char *msg)
> {
> if (!debug_locks_off())
> return;
>
> + /*
> + * If this function is called from a function like printk()
> + * which is trying to obtain the console lock, then we should
> + * not call printk() any more. Or it will cause an infinite
> + * recursive cycle!
> + */
> + if (unlikely(is_console_lock(lock)))
> + return;
> +
> spin_dump(lock, msg);
> }
>
> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> index 2ce8826..50ea552 100644
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -119,6 +119,11 @@ static int __down_trylock_console_sem(unsigned long ip)
> up(&console_sem);\
> } while (0)
>
> +int is_console_lock(raw_spinlock_t *lock)
> +{
> + return &console_sem.lock == lock;
> +}
> +
> /*
> * This is used for debugging the mess that is the VT code by
> * keeping track if we have the console semaphore held. It's
I can't immediately think of anything better than this. It's a hack, but
it's a small and quite clear hack.
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH v2] lib/spinlock_debug.c: prevent an infinite recursive cycle in spin_dump()
2016-01-27 0:11 ` Andrew Morton
@ 2016-01-27 1:13 ` Peter Hurley
2016-01-27 2:00 ` Byungchul Park
1 sibling, 0 replies; 7+ messages in thread
From: Peter Hurley @ 2016-01-27 1:13 UTC (permalink / raw)
To: Andrew Morton, Byungchul Park
Cc: mingo, linux-kernel, akinobu.mita, jack, torvalds
On 01/26/2016 04:11 PM, Andrew Morton wrote:
> On Mon, 18 Jan 2016 09:58:12 +0900 Byungchul Park <byungchul.park@lge.com> wrote:
>
>> It causes an infinite recursive cycle when using CONFIG_DEBUG_SPINLOCK,
>> in the spin_dump(). Backtrace prints printk() -> console_trylock() ->
>> do_raw_spin_lock() -> spint_bug() -> spin_dump() -> printk()...
>> infinitely.
>>
>> If the spin_bug() is called from a function like printk() which is
>> trying to obtain the console lock, we should prevent the debug spinlock
>> code from calling printk() again in that context.
>>
>
> lol. Excellent.
[...]
> I can't immediately think of anything better than this. It's a hack, but
> it's a small and quite clear hack.
Andrew,
I think you may have missed this follow-up from Byungchul:
On 01/21/2016 12:12 AM, Byungchul Park wrote:
> I was careless. I think it should be fixed by another way, instead of
> the way this patch suggested.
Regards,
Peter Hurley
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] lib/spinlock_debug.c: prevent an infinite recursive cycle in spin_dump()
2016-01-27 0:11 ` Andrew Morton
2016-01-27 1:13 ` Peter Hurley
@ 2016-01-27 2:00 ` Byungchul Park
2016-01-27 2:22 ` Byungchul Park
1 sibling, 1 reply; 7+ messages in thread
From: Byungchul Park @ 2016-01-27 2:00 UTC (permalink / raw)
To: Andrew Morton; +Cc: mingo, linux-kernel, akinobu.mita, jack, torvalds
On Tue, Jan 26, 2016 at 04:11:24PM -0800, Andrew Morton wrote:
> I can't immediately think of anything better than this. It's a hack, but
> it's a small and quite clear hack.
I am sorry for making you confused. :( I think I need to find another way
to solve this problem. At first, when I wrote this patch, I was also
confused.
thanks,
byungchul
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] lib/spinlock_debug.c: prevent an infinite recursive cycle in spin_dump()
2016-01-27 2:00 ` Byungchul Park
@ 2016-01-27 2:22 ` Byungchul Park
0 siblings, 0 replies; 7+ messages in thread
From: Byungchul Park @ 2016-01-27 2:22 UTC (permalink / raw)
To: Andrew Morton; +Cc: mingo, linux-kernel, akinobu.mita, jack, torvalds
On Wed, Jan 27, 2016 at 11:00:14AM +0900, Byungchul Park wrote:
> On Tue, Jan 26, 2016 at 04:11:24PM -0800, Andrew Morton wrote:
> > I can't immediately think of anything better than this. It's a hack, but
> > it's a small and quite clear hack.
>
> I am sorry for making you confused. :( I think I need to find another way
> to solve this problem. At first, when I wrote this patch, I was also
> confused.
Even though this can fix symptoms..
>
> thanks,
> byungchul
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2016-01-27 2:22 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-01-18 0:58 [PATCH v2] lib/spinlock_debug.c: prevent an infinite recursive cycle in spin_dump() Byungchul Park
2016-01-19 1:53 ` Byungchul Park
2016-01-21 8:12 ` Byungchul Park
2016-01-27 0:11 ` Andrew Morton
2016-01-27 1:13 ` Peter Hurley
2016-01-27 2:00 ` Byungchul Park
2016-01-27 2:22 ` Byungchul Park
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.