| Message ID | 12a693da-19c8-dd2c-ea6a-0a5dc9d2db27@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | mm/hugetlb: Don't put_page in lock of hugetlb_lock | expand |
On Sat 04-05-19 20:28:24, Zhiqiang Liu wrote: > From: Kai Shen <shenkai8@huawei.com> > > spinlock recursion happened when do LTP test: > #!/bin/bash > ./runltp -p -f hugetlb & > ./runltp -p -f hugetlb & > ./runltp -p -f hugetlb & > ./runltp -p -f hugetlb & > ./runltp -p -f hugetlb & > > The dtor returned by get_compound_page_dtor in __put_compound_page > may be the function of free_huge_page which will lock the hugetlb_lock, > so don't put_page in lock of hugetlb_lock. > > BUG: spinlock recursion on CPU#0, hugemmap05/1079 > lock: hugetlb_lock+0x0/0x18, .magic: dead4ead, .owner: hugemmap05/1079, .owner_cpu: 0 > Call trace: > dump_backtrace+0x0/0x198 > show_stack+0x24/0x30 > dump_stack+0xa4/0xcc > spin_dump+0x84/0xa8 > do_raw_spin_lock+0xd0/0x108 > _raw_spin_lock+0x20/0x30 > free_huge_page+0x9c/0x260 > __put_compound_page+0x44/0x50 > __put_page+0x2c/0x60 > alloc_surplus_huge_page.constprop.19+0xf0/0x140 > hugetlb_acct_memory+0x104/0x378 > hugetlb_reserve_pages+0xe0/0x250 > hugetlbfs_file_mmap+0xc0/0x140 > mmap_region+0x3e8/0x5b0 > do_mmap+0x280/0x460 > vm_mmap_pgoff+0xf4/0x128 > ksys_mmap_pgoff+0xb4/0x258 > __arm64_sys_mmap+0x34/0x48 > el0_svc_common+0x78/0x130 > el0_svc_handler+0x38/0x78 > el0_svc+0x8/0xc > > Fixes: 9980d744a0 ("mm, hugetlb: get rid of surplus page accounting tricks") > Signed-off-by: Kai Shen <shenkai8@huawei.com> > Signed-off-by: Feilong Lin <linfeilong@huawei.com> > Reported-by: Wang Wang <wangwang2@huawei.com> You are right. I must have completely missed that put_page path unconditionally takes the hugetlb_lock for hugetlb pages. Thanks for fixing this. I think this should be marked for stable because it is not hard to imagine a regular user might trigger this. Acked-by: Michal Hocko <mhocko@suse.com> > --- > mm/hugetlb.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/mm/hugetlb.c b/mm/hugetlb.c > index 6cdc7b2..c1e7b81 100644 > --- a/mm/hugetlb.c > +++ b/mm/hugetlb.c > @@ -1574,8 +1574,9 @@ static struct page *alloc_surplus_huge_page(struct hstate *h, gfp_t gfp_mask, > */ > if (h->surplus_huge_pages >= h->nr_overcommit_huge_pages) { > SetPageHugeTemporary(page); > + spin_unlock(&hugetlb_lock); > put_page(page); > - page = NULL; > + return NULL; > } else { > h->surplus_huge_pages++; > h->surplus_huge_pages_node[page_to_nid(page)]++; > -- > 1.8.3.1 > >
> On Sat 04-05-19 20:28:24, Zhiqiang Liu wrote: >> From: Kai Shen <shenkai8@huawei.com> >> >> spinlock recursion happened when do LTP test: >> #!/bin/bash >> ./runltp -p -f hugetlb & >> ./runltp -p -f hugetlb & >> ./runltp -p -f hugetlb & >> ./runltp -p -f hugetlb & >> ./runltp -p -f hugetlb & >> >> Fixes: 9980d744a0 ("mm, hugetlb: get rid of surplus page accounting tricks") >> Signed-off-by: Kai Shen <shenkai8@huawei.com> >> Signed-off-by: Feilong Lin <linfeilong@huawei.com> >> Reported-by: Wang Wang <wangwang2@huawei.com> > > You are right. I must have completely missed that put_page path > unconditionally takes the hugetlb_lock for hugetlb pages. > > Thanks for fixing this. I think this should be marked for stable > because it is not hard to imagine a regular user might trigger this. > > Acked-by: Michal Hocko <mhocko@suse.com> Thank you for your reply. I will add Acked-by: Michal Hocko <mhocko@suse.com> in the v2 patch.
diff --git a/mm/hugetlb.c b/mm/hugetlb.c index 6cdc7b2..c1e7b81 100644 --- a/mm/hugetlb.c +++ b/mm/hugetlb.c @@ -1574,8 +1574,9 @@ static struct page *alloc_surplus_huge_page(struct hstate *h, gfp_t gfp_mask, */ if (h->surplus_huge_pages >= h->nr_overcommit_huge_pages) { SetPageHugeTemporary(page); + spin_unlock(&hugetlb_lock); put_page(page); - page = NULL; + return NULL; } else { h->surplus_huge_pages++; h->surplus_huge_pages_node[page_to_nid(page)]++;