Re: new warning on sysrq kernel crash trigger

[Rik van Riel <riel@redhat.com>]

On 12/11/2015 03:44 PM, Ani Sinha wrote:
> 
> 
> On Thu, 10 Dec 2015, Paul E. McKenney wrote:
> 
>> On Thu, Dec 10, 2015 at 03:57:09PM -0800, Ani Sinha wrote:
>>> Hi guys
>>>
>>> I am noticing a new warning in linux 3.18 which we did not see before
>>> in linux 3.4 :
>>>
>>> bash-4.1# echo c > /proc/sysrq-trigger
>>> [  978.807185] BUG: sleeping function called from invalid context at
>>> ../arch/x86/mm/fault.c:1187
>>> [  978.909816] in_atomic(): 0, irqs_disabled(): 0, pid: 4706, name: bash
>>> [  978.987358] Preemption disabled at:[<ffffffff81484339>] printk+0x48/0x4a
>>>
>>>
>>> I have bisected this to the following change :
>>>
>>> commit 984d74a72076a12b400339973e8c98fd2fcd90e5
>>> Author: Rik van Riel <riel@redhat.com>
>>> Date:   Fri Jun 6 14:38:13 2014 -0700
>>>
>>>     sysrq: rcu-ify __handle_sysrq
>>>
>>>
>>> the rcu_read_lock() in handle_sysrq() bumps up
>>> current->rcu_read_lock_nesting. Hence, in __do_page_fault() when it
>>> calls might_sleep() in x86/mm/fault.c line 1191,
>>> preempt_count_equals(0) returns false and hence the warning is
>>> printed.
>>>
>>> One way to handle this would be to do something like this:
>>>
>>> diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c
>>> index eef44d9..d4dbe22 100644
>>> --- a/arch/x86/mm/fault.c
>>> +++ b/arch/x86/mm/fault.c
>>> @@ -1132,7 +1132,7 @@ __do_page_fault(struct pt_regs *regs, unsigned
>>> long error_code,
>>>   * If we're in an interrupt, have no user context or are running
>>>   * in a region with pagefaults disabled then we must not take the fault
>>>   */
>>> - if (unlikely(faulthandler_disabled() || !mm)) {
>>> + if (unlikely(faulthandler_disabled() || rcu_preempt_depth() || !mm)) {
>>
>> This works if CONFIG_PREEMPT=y, but if CONFIG_PREEMPT=n, then
>> rcu_preempt_depth() unconditionally returns zero.  And if
>> CONFIG_PREEMPT_COUNT=y && CONFIG_PREEMPT=n, you would still see
>> the might_sleep() splat.
>>
>> Maybe use SRCU instead of RCU for this purpose?
>>
> 
> From ae232ce3fb167b2ad363bfac7aab69001bc55a50 Mon Sep 17 00:00:00 2001
> From: Ani Sinha <ani@arista.com>
> Date: Fri, 11 Dec 2015 12:07:42 -0800
> Subject: [PATCH 1/1] Fix 'sleeping function called from invalid context'
>  warning in sysrq generated crash.
> 
> Commit 984d74a72076a1 ("sysrq: rcu-ify __handle_sysrq") 
> replaced spin_lock_irqsave() calls with
> rcu_read_lock() calls in sysrq. Since rcu_read_lock() does not
> disable preemption, faulthandler_disabled() in
> __do_page_fault() in x86/fault.c returns false. When the code
> later calls might_sleep() in the pagefault handler, we get the
> following warning:
> 
> BUG: sleeping function called from invalid context at ../arch/x86/mm/fault.c:1187
> in_atomic(): 0, irqs_disabled(): 0, pid: 4706, name: bash
> Preemption disabled at:[<ffffffff81484339>] printk+0x48/0x4a
> 
> To fix this, replace RCU call in handle_sysrq() to use SRCU.

The sysrq code can be called from irq context.

Trying to use SRCU from an irq context sounds like it could
be a bad idea, though admittedly I do not know enough about
SRCU to know for sure :)


-- 
All rights reversed