Re: [PATCH] rcu: avoid alloc_pages() when recording stack

From: Jun Miao <jun.miao@windriver.com>
To: paulmck@kernel.org, Uladzislau Rezki <urezki@gmail.com>
Cc: Josh Triplett <josh@joshtriplett.org>,
	Steven Rostedt <rostedt@goodmis.org>,
	Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
	Lai Jiangshan <jiangshanlai@gmail.com>,
	Joel Fernandes <joel@joelfernandes.org>,
	Dmitry Vyukov <dvyukov@google.com>,
	qiang.zhang1211@gmail.com, RCU <rcu@vger.kernel.org>,
	LKML <linux-kernel@vger.kernel.org>,
	miaojun0823@163.com, ryabinin.a.a@gmail.com,
	Alexander Potapenko <glider@google.com>,
	jianwei.hu@windriver.com, melver@google.com
Subject: Re: [PATCH] rcu: avoid alloc_pages() when recording stack
Date: Thu, 4 Nov 2021 09:09:24 +0800	[thread overview]
Message-ID: <309b8284-1c31-7cc4-eb40-ba6d8d136c09@windriver.com> (raw)
In-Reply-To: <20211103212117.GA631708@paulmck-ThinkPad-P17-Gen-1>


On 11/4/21 5:21 AM, Paul E. McKenney wrote:
> [Please note: This e-mail is from an EXTERNAL e-mail address]
>
> On Wed, Nov 03, 2021 at 11:13:15AM -0700, Paul E. McKenney wrote:
>> On Wed, Nov 03, 2021 at 02:55:48PM +0100, Uladzislau Rezki wrote:
>>> On Wed, Nov 3, 2021 at 7:51 AM Jun Miao <jun.miao@windriver.com> wrote:
>>>>
>>>> On 11/2/21 10:53 PM, Uladzislau Rezki wrote:
>>>>> [Please note: This e-mail is from an EXTERNAL e-mail address]
>>>>>
>>>>>> Add KASAN maintainers
>>>>>>
>>>>>> On 11/1/21 6:31 PM, Jun Miao wrote:
>>>>>>> The default kasan_record_aux_stack() calls stack_depot_save() with GFP_NOWAIT,
>>>>>>> which in turn can then call alloc_pages(GFP_NOWAIT, ...).  In general, however,
>>>>>>> it is not even possible to use either GFP_ATOMIC nor GFP_NOWAIT in certain
>>>>>>> non-preemptive contexts/RT kernel including raw_spin_locks (see gfp.h and ab00db216c9c7).
>>>>>>>
>>>>>>> Fix it by instructing stackdepot to not expand stack storage via alloc_pages()
>>>>>>> in case it runs out by using kasan_record_aux_stack_noalloc().
>>>>>>>
>>>>>>> Jianwei Hu reported:
>>>>>>>     BUG: sleeping function called from invalid context at kernel/locking/rtmutex.c:969
>>>>>>>     in_atomic(): 0, irqs_disabled(): 1, non_block: 0, pid: 15319, name: python3
>>>>>>>     INFO: lockdep is turned off.
>>>>>>>     irq event stamp: 0
>>>>>>>     hardirqs last  enabled at (0): [<0000000000000000>] 0x0
>>>>>>>     hardirqs last disabled at (0): [<ffffffff856c8b13>] copy_process+0xaf3/0x2590
>>>>>>>     softirqs last  enabled at (0): [<ffffffff856c8b13>] copy_process+0xaf3/0x2590
>>>>>>>     softirqs last disabled at (0): [<0000000000000000>] 0x0
>>>>>>>     CPU: 6 PID: 15319 Comm: python3 Tainted: G        W  O 5.15-rc7-preempt-rt #1
>>>>>>>     Hardware name: Supermicro SYS-E300-9A-8C/A2SDi-8C-HLN4F, BIOS 1.1b 12/17/2018
>>>>>>>     Call Trace:
>>>>>>>      show_stack+0x52/0x58
>>>>>>>      dump_stack+0xa1/0xd6
>>>>>>>      ___might_sleep.cold+0x11c/0x12d
>>>>>>>      rt_spin_lock+0x3f/0xc0
>>>>>>>      rmqueue+0x100/0x1460
>>>>>>>      rmqueue+0x100/0x1460
>>>>>>>      mark_usage+0x1a0/0x1a0
>>>>>>>      ftrace_graph_ret_addr+0x2a/0xb0
>>>>>>>      rmqueue_pcplist.constprop.0+0x6a0/0x6a0
>>>>>>>       __kasan_check_read+0x11/0x20
>>>>>>>       __zone_watermark_ok+0x114/0x270
>>>>>>>       get_page_from_freelist+0x148/0x630
>>>>>>>       is_module_text_address+0x32/0xa0
>>>>>>>       __alloc_pages_nodemask+0x2f6/0x790
>>>>>>>       __alloc_pages_slowpath.constprop.0+0x12d0/0x12d0
>>>>>>>       create_prof_cpu_mask+0x30/0x30
>>>>>>>       alloc_pages_current+0xb1/0x150
>>>>>>>       stack_depot_save+0x39f/0x490
>>>>>>>       kasan_save_stack+0x42/0x50
>>>>>>>       kasan_save_stack+0x23/0x50
>>>>>>>       kasan_record_aux_stack+0xa9/0xc0
>>>>>>>       __call_rcu+0xff/0x9c0
>>>>>>>       call_rcu+0xe/0x10
>>>>>>>       put_object+0x53/0x70
>>>>>>>       __delete_object+0x7b/0x90
>>>>>>>       kmemleak_free+0x46/0x70
>>>>>>>       slab_free_freelist_hook+0xb4/0x160
>>>>>>>       kfree+0xe5/0x420
>>>>>>>       kfree_const+0x17/0x30
>>>>>>>       kobject_cleanup+0xaa/0x230
>>>>>>>       kobject_put+0x76/0x90
>>>>>>>       netdev_queue_update_kobjects+0x17d/0x1f0
>>>>>>>       ... ...
>>>>>>>       ksys_write+0xd9/0x180
>>>>>>>       __x64_sys_write+0x42/0x50
>>>>>>>       do_syscall_64+0x38/0x50
>>>>>>>       entry_SYSCALL_64_after_hwframe+0x44/0xa9
>>>>>>>
>>>>>>> Fixes: 84109ab58590 ("rcu: Record kvfree_call_rcu() call stack for KASAN")
>>>>>>> Fixes: 26e760c9a7c8 ("rcu: kasan: record and print call_rcu() call stack")
>>>>>>> Reported-by: Jianwei Hu <jianwei.hu@windriver.com>
>>>>>>> Signed-off-by: Jun Miao <jun.miao@windriver.com>
>>>>>>> ---
>>>>>>>     kernel/rcu/tree.c | 4 ++--
>>>>>>>     1 file changed, 2 insertions(+), 2 deletions(-)
>>>>>>>
>>>>>>> diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
>>>>>>> index 8270e58cd0f3..2c1034580f15 100644
>>>>>>> --- a/kernel/rcu/tree.c
>>>>>>> +++ b/kernel/rcu/tree.c
>>>>>>> @@ -3026,7 +3026,7 @@ __call_rcu(struct rcu_head *head, rcu_callback_t func)
>>>>>>>         head->func = func;
>>>>>>>         head->next = NULL;
>>>>>>>         local_irq_save(flags);
>>>>>>> -     kasan_record_aux_stack(head);
>>>>>>> +     kasan_record_aux_stack_noalloc(head);
>>>>>>>         rdp = this_cpu_ptr(&rcu_data);
>>>>>>>
>>>>>>>         /* Add the callback to our list. */
>>>>>>> @@ -3591,7 +3591,7 @@ void kvfree_call_rcu(struct rcu_head *head, rcu_callback_t func)
>>>>>>>                 return;
>>>>>>>         }
>>>>>>>
>>>>>>> -     kasan_record_aux_stack(ptr);
>>>>>>> +     kasan_record_aux_stack_noalloc(ptr);
>>>>>>>         success = add_ptr_to_bulk_krc_lock(&krcp, &flags, ptr, !head);
>>>>>>>         if (!success) {
>>>>>>>                 run_page_cache_worker(krcp);
>>>>> Yep an allocation is tricky here. This change looks correct to me at
>>>>> least from the point that it does not allocate.
>>>>>
>>>>> --
>>>>> Uladzislau Rezki
>>>> Thanks your approval. Could you like to give me a review?
>>>>
>>> Reviewed-by: Uladzislau Rezki (Sony) <urezki@gmail.com>
>> I have queued it for review and testing, thank you both!  I do have
>> some remaining concerns about this code being starved for memory.  I am
>> wondering if the code needs to check the interrupt state.  And perhaps
>> also whether locks are held.  I of course will refrain from sending
>> this to mainline until these concerns are resolved.
>>
>> Marco, Dmitry, thoughts?
> Well, the compiler does have an opinion:
>
> kernel/rcu/tree.c: In function ‘__call_rcu’:
> kernel/rcu/tree.c:3029:2: error: implicit declaration of function ‘kasan_record_aux_stack_noalloc’; did you mean ‘kasan_record_aux_stack’? [-Werror=implicit-function-declaration]
>   3029 |  kasan_record_aux_stack_noalloc(head);
>        |  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>        |  kasan_record_aux_stack
>
> I get the same message after merging in current mainline.
>
> I have therefore dropped this patch for the time being.
>
>                                                          Thanx, Paul
Hi Paul E,
The kasan_record_aux_stack_noalloc() is just introduce to linux-next 
now, and marking "Notice: this object is not reachable from any branch." 
in commit.
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/include/linux/kasan.h?h=next-20211029&id=2f64acf6b653d01fbdc92a693f12bbf71a205926

--- ---
Jun Miao