On 2022/11/28 16:33, Zhang, Qiang1 wrote: > On 2022/11/23 7:05, Zhang, Qiang1 wrote: >> >> Gently ping 😊 >> >> Thanks >> Zqiang >> >>> Currently, the mem_dump_obj() is invoked in call_rcu(), the >>> call_rcu() is maybe invoked in non-preemptive code segment, for >>> object allocated from vmalloc(), the following scenarios may occur: >>> >>> CPU 0 >>> tasks context >>> spin_lock(&vmap_area_lock) >>> Interrupt context >>> call_rcu() >>> mem_dump_obj >>> vmalloc_dump_obj >>> spin_lock(&vmap_area_lock) <--deadlock >>> >>> and for PREEMPT-RT kernel, the spinlock will convert to sleepable >>> lock, so the vmap_area_lock spinlock not allowed to get in >>> non-preemptive code segment. therefore, this commit make the >>> vmalloc_dump_obj() call in a clean context. >>> >>> Signed-off-by: Zqiang <qiang1.zhang@xxxxxxxxx> >>> --- >>> v1->v2: >>> add IS_ENABLED(CONFIG_PREEMPT_RT) check. >>> v2->v3: >>> change commit message and add some comment. >>> >>> mm/util.c | 4 +++- >>> mm/vmalloc.c | 25 +++++++++++++++++++++++++ >>> 2 files changed, 28 insertions(+), 1 deletion(-) >>> >>> diff --git a/mm/util.c b/mm/util.c >>> index 12984e76767e..2b0222a728cc 100644 >>> --- a/mm/util.c >>> +++ b/mm/util.c >>> @@ -1128,7 +1128,9 @@ void mem_dump_obj(void *object) >>> return; >>> >>> if (virt_addr_valid(object)) >>> - type = "non-slab/vmalloc memory"; >>> + type = "non-slab memory"; >>> + else if (is_vmalloc_addr(object)) >>> + type = "vmalloc memory"; >>> else if (object == NULL) >>> type = "NULL pointer"; >>> else if (object == ZERO_SIZE_PTR) >>> diff --git a/mm/vmalloc.c b/mm/vmalloc.c index >>> ccaa461998f3..4351eafbe7ab 100644 >>> --- a/mm/vmalloc.c >>> +++ b/mm/vmalloc.c >>> @@ -4034,6 +4034,31 @@ bool vmalloc_dump_obj(void *object) >>> struct vm_struct *vm; >>> void *objp = (void *)PAGE_ALIGN((unsigned long)object); >>> >>> + /* for non-vmalloc addr, return directly */ >>> + if (!is_vmalloc_addr(objp)) >>> + return false; >>> + >>> + /** >>> + * for non-Preempt-RT kernel, return directly. otherwise not >>> + * only needs to determine whether it is in the interrupt context >>> + * (in_interrupt())to avoid deadlock, but also to avoid acquire >>> + * vmap_area_lock spinlock in disables interrupts or preempts >>> + * critical sections, because the vmap_area_lock spinlock convert >>> + * to sleepable lock >>> + */ >>> + if (IS_ENABLED(CONFIG_PREEMPT_RT) && !preemptible()) >>> + return false; >>> + >>> + /** >>> + * get here, for Preempt-RT kernel, it means that we are in >>> + * preemptible context(preemptible() is true), it also means >>> + * that the in_interrupt() will return false. >>> + * for non-Preempt-RT kernel, only needs to determine whether >>> + * it is in the interrupt context(in_interrupt()) to avoid deadlock >>> + */ >>> + if (in_interrupt()) >>> + return false; >> >> >> We want mem_dump_obj() to work properly in the interrupt context. But with this if statement, it's impossible to work properly. > > This is to avoid the following scenarios, because, call_rcu() can be invoked in hard irq or > softirq context, so mem_dump_obj() not dump some details info. OK. Sorry, I'm confusing your issue with what I'm doing right now. https://lkml.org/lkml/2022/11/16/913 I need "if (in_interrupt() && spin_is_locked(&vmap_area_lock))". So mem_dump_obj() can work well in interrupt, except the task was interrupted in the critical section of vmap_area_lock. > > CPU 0 > tasks context > spin_lock(&vmap_area_lock) > Interrupt or softirq context > call_rcu() > mem_dump_obj > vmalloc_dump_obj > spin_lock(&vmap_area_lock) <--deadlock > > because mem_dump_obj() only used by RCU, I'm not sure if this modification is appropriate, > need to hear from Paul. > > Thanks > Zqiang > > >> >> Here's my test case: >> void *tst_p; >> >> void my_irqwork_handler(struct irq_work *work) { >> void *p = tst_p; >> >> printk("enter my_irqwork_handler: CPU=%d, locked=%d\n", smp_processor_id(), tst_is_locked()); >> mem_dump_obj(p); >> vfree(p); >> } >> >> static void test_mem_dump(void) >> { >> struct irq_work work = IRQ_WORK_INIT_HARD(my_irqwork_handler); >> >> tst_p = vmalloc(PAGE_SIZE); >> if (!tst_p) { >> printk("vmalloc failed\n"); >> return; >> } >> printk("enter test_mem_dump: CPU=%d\n", smp_processor_id()); >> >> //tst_lock(); >> irq_work_queue(&work); >> //tst_unlock(); >> >> printk("leave test_mem_dump: CPU=%d\n", smp_processor_id()); } >> >> Test result: >> [ 45.212941] enter test_mem_dump: CPU=0 >> [ 45.213280] enter my_irqwork_handler: CPU=0, locked=0 >> [ 45.213546] vmalloc memory >> [ 45.213996] leave test_mem_dump: CPU=0 >> >>> + >>> vm = find_vm_area(objp); >>> if (!vm) >>> return false; >>> -- >>> 2.25.1 >> >> >> -- >> Regards, >> Zhen Lei -- Regards, Zhen Lei
