| Message ID | 20221112121537.1634-1-thunder.leizhen@huawei.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | mm/vmalloc: allow mem_dump_obj() to be called in interrupt context | expand |
On Sat, 12 Nov 2022 20:15:37 +0800 Zhen Lei <thunder.leizhen@huawei.com> wrote: > The function mem_dump_obj() can sometimes provide valuable debugging > information, but it cannot be called in an interrupt context because > spinlock vmap_area_lock has not been protected against IRQs. If the > current task has held the lock before hard/soft interrupt handler calls > mem_dump_obj(), simply abandoning the dump operation can avoid deadlock. > That is, no deadlock occurs in extreme cases, and dump succeeds in most > cases. > > ... > > --- a/mm/vmalloc.c > +++ b/mm/vmalloc.c > @@ -4034,6 +4034,9 @@ bool vmalloc_dump_obj(void *object) > struct vm_struct *vm; > void *objp = (void *)PAGE_ALIGN((unsigned long)object); > > + if (unlikely(spin_is_locked(&vmap_area_lock))) > + return false; > + > vm = find_vm_area(objp); > if (!vm) > return false; Yes, but this will worsen the current uses of this function. Consider the case where task A wants to call vmalloc_dump_obj() but task B holds vmap_area_lock. No problem, task A will simply spin until task B is done. But after this patch, task A's call to vmalloc_dump_obj() will return without having done anything.
On 2022/11/15 8:54, Andrew Morton wrote: > On Sat, 12 Nov 2022 20:15:37 +0800 Zhen Lei <thunder.leizhen@huawei.com> wrote: > >> The function mem_dump_obj() can sometimes provide valuable debugging >> information, but it cannot be called in an interrupt context because >> spinlock vmap_area_lock has not been protected against IRQs. If the >> current task has held the lock before hard/soft interrupt handler calls >> mem_dump_obj(), simply abandoning the dump operation can avoid deadlock. >> That is, no deadlock occurs in extreme cases, and dump succeeds in most >> cases. >> >> ... >> >> --- a/mm/vmalloc.c >> +++ b/mm/vmalloc.c >> @@ -4034,6 +4034,9 @@ bool vmalloc_dump_obj(void *object) >> struct vm_struct *vm; >> void *objp = (void *)PAGE_ALIGN((unsigned long)object); >> >> + if (unlikely(spin_is_locked(&vmap_area_lock))) >> + return false; >> + >> vm = find_vm_area(objp); >> if (!vm) >> return false; > > Yes, but this will worsen the current uses of this function. Consider > the case where task A wants to call vmalloc_dump_obj() but task B holds > vmap_area_lock. No problem, task A will simply spin until task B is > done. > > But after this patch, task A's call to vmalloc_dump_obj() will return > without having done anything. Oh, right, this problem occurs when task A and task B run on two different cores. > > . >
On 2022/11/15 10:01, Leizhen (ThunderTown) wrote: > > > On 2022/11/15 8:54, Andrew Morton wrote: >> On Sat, 12 Nov 2022 20:15:37 +0800 Zhen Lei <thunder.leizhen@huawei.com> wrote: >> >>> The function mem_dump_obj() can sometimes provide valuable debugging >>> information, but it cannot be called in an interrupt context because >>> spinlock vmap_area_lock has not been protected against IRQs. If the >>> current task has held the lock before hard/soft interrupt handler calls >>> mem_dump_obj(), simply abandoning the dump operation can avoid deadlock. >>> That is, no deadlock occurs in extreme cases, and dump succeeds in most >>> cases. >>> >>> ... >>> >>> --- a/mm/vmalloc.c >>> +++ b/mm/vmalloc.c >>> @@ -4034,6 +4034,9 @@ bool vmalloc_dump_obj(void *object) >>> struct vm_struct *vm; >>> void *objp = (void *)PAGE_ALIGN((unsigned long)object); >>> >>> + if (unlikely(spin_is_locked(&vmap_area_lock))) >>> + return false; >>> + >>> vm = find_vm_area(objp); >>> if (!vm) >>> return false; >> >> Yes, but this will worsen the current uses of this function. Consider >> the case where task A wants to call vmalloc_dump_obj() but task B holds >> vmap_area_lock. No problem, task A will simply spin until task B is >> done. >> >> But after this patch, task A's call to vmalloc_dump_obj() will return >> without having done anything. > > Oh, right, this problem occurs when task A and task B run on > two different cores. I've rethought it, this can be solved by adding in_interrupt(). > > >> >> . >> >
diff --git a/mm/vmalloc.c b/mm/vmalloc.c index ccaa461998f3c37..cdd36c5a1aa16f8 100644 --- a/mm/vmalloc.c +++ b/mm/vmalloc.c @@ -4034,6 +4034,9 @@ bool vmalloc_dump_obj(void *object) struct vm_struct *vm; void *objp = (void *)PAGE_ALIGN((unsigned long)object); + if (unlikely(spin_is_locked(&vmap_area_lock))) + return false; + vm = find_vm_area(objp); if (!vm) return false;
The function mem_dump_obj() can sometimes provide valuable debugging information, but it cannot be called in an interrupt context because spinlock vmap_area_lock has not been protected against IRQs. If the current task has held the lock before hard/soft interrupt handler calls mem_dump_obj(), simply abandoning the dump operation can avoid deadlock. That is, no deadlock occurs in extreme cases, and dump succeeds in most cases. Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com> --- mm/vmalloc.c | 3 +++ 1 file changed, 3 insertions(+)