Re: resv_huge_page underflow with userfaultfd test

[Mike Kravetz <mike.kravetz@oracle.com>]

On 5/7/21 2:21 PM, Mina Almasry wrote:
> Hi folks,
> 
> I ran into a bug that I'm not sure how to solve so I'm wondering if
> anyone has suggestions on what the issue could be and how to
> investigate. I added the WARN_ON_ONCE() here to catch instances of
> resv_huge_pages underflowing:
> 
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index 629aa4c2259c..7d763eed650f 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -1165,7 +1165,21 @@ static struct page
> *dequeue_huge_page_vma(struct hstate *h,
>         page = dequeue_huge_page_nodemask(h, gfp_mask, nid, nodemask);
>         if (page && !avoid_reserve && vma_has_reserves(vma, chg)) {
>                 SetHPageRestoreReserve(page);
> +               WARN_ON_ONCE(!h->resv_huge_pages);
>                 h->resv_huge_pages--;
>         }
> 
> And ran the userfaultfd selftests like so:
> 
> echo 1024 > /proc/sys/vm/nr_hugepages
> mkdir -p /mnt/huge
> mount -t hugetlbfs none /mnt/huge
> ./tools/testings/selftests/vm/userfaultfd hugetlb_shared 1024 200
> /mnt/huge/userfaultfd_test
> 
> And run into this warning indicating this test does discover an underflow:
> 
> [   11.163403] ------------[ cut here ]------------
> [   11.163404] WARNING: CPU: 0 PID: 237 at mm/hugetlb.c:1178
> alloc_huge_page+0x558/0x5a0
> [   11.163413] Modules linked in:
> [   11.163419] CPU: 0 PID: 237 Comm: userfaultfd Not tainted 5.12.0-dbg-DEV #135
> [   11.163424] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
> BIOS 1.14.0-2 04/01/2014
> [   11.163429] RIP: 0010:alloc_huge_page+0x558/0x5a0
> [   11.163432] Code: b0 00 0f 85 3d ff ff ff e9 2a ff ff ff be 01 00
> 00 00 48 89 df e8 18 e7 ff ff 48 f7 d8 4c 89 ef 48 89 c6 e8 da d7 ff
> ff eb 8c <0f> 0b 4d 8b 85 c0 00 00 00 e9 95 fd ff ff e8 35 59 84 00 4c
> 897
> [   11.163434] RSP: 0018:ffff94bb0073fc80 EFLAGS: 00010046
> [   11.163436] RAX: 0000000000000080 RBX: 0000000000000000 RCX: 5fa252c406a76700
> [   11.163438] RDX: c0000000ffff7fff RSI: 0000000000000004 RDI:
> 0000000000017ffd
> [   11.163439] RBP: ffff94bb0073fcf8 R08: 0000000000000000 R09:
> ffffffff9813ba70
> [   11.163440] R10: 00000000ffff7fff R11: 0000000000000000 R12:
> ffff8ac7800558c8
> [   11.163442] R13: ffffffff993f8880 R14: 00007f0dfa200000 R15:
> ffffed85453e0000
> [   11.163443] FS:  00007f0d731fc700(0000) GS:ffff8acba9400000(0000)
> knlGS:0000000000000000
> [   11.163445] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [   11.163448] CR2: 00007f0e65e00028 CR3: 0000000108d50003 CR4:
> 0000000000370ef0
> [   11.163452] DR0: 0000000000000000 DR1: 0000000000000000 DR2:
> 0000000000000000
> [   11.163453] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7:
> 0000000000000400
> [   11.163455] Call Trace:
> [   11.163468]  hugetlb_mcopy_atomic_pte+0xcb/0x450
> [   11.163477]  mcopy_atomic+0xa08/0xd60
> [   11.163480]  ? __might_fault+0x56/0x80
> [   11.163493]  userfaultfd_ioctl+0xb18/0xd60
> [   11.163502]  __se_sys_ioctl+0x77/0xc0
> [   11.163507]  __x64_sys_ioctl+0x1d/0x20
> [   11.163510]  do_syscall_64+0x3f/0x80
> [   11.163515]  entry_SYSCALL_64_after_hwframe+0x44/0xae
> [   11.163519] RIP: 0033:0x45ec87
> [   11.163531] Code: 3c 1c 48 f7 d8 49 39 c4 72 b8 e8 64 63 03 00 85
> c0 78 bd 48 83 c4 08 4c 89 e0 5b 41 5c c3 0f 1f 44 00 00 b8 10 00 00
> 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 b8 ff ff ff f7 d8 64 89
> 018
> [   11.163532] RSP: 002b:00007f0d731fc248 EFLAGS: 00000206 ORIG_RAX:
> 0000000000000010
> [   11.163534] RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 000000000045ec87
> [   11.163536] RDX: 00007f0d731fc290 RSI: 00000000c028aa03 RDI: 0000000000000004
> [   11.163537] RBP: 00007f0d731fc270 R08: 00000000004022b3 R09: 00007f0d731fc700
> [   11.163538] R10: 00007f0d731fc9d0 R11: 0000000000000206 R12: 00007fff610cd82e
> [   11.163539] R13: 00007fff610cd82f R14: 00007f0d731fc400 R15: 0000000001002000
> [   11.163549] irq event stamp: 722
> [   11.163550] hardirqs last  enabled at (721): [<ffffffff967cd41b>]
> kmem_cache_alloc_trace+0x1db/0x370
> [   11.163558] hardirqs last disabled at (722): [<ffffffff9700c052>]
> _raw_spin_lock_irq+0x32/0x80
> [   11.163560] softirqs last  enabled at (130): [<ffffffff9654e0d6>]
> __irq_exit_rcu+0xf6/0x100
> [   11.163564] softirqs last disabled at (125): [<ffffffff9654e0d6>]
> __irq_exit_rcu+0xf6/0x100
> [   11.163567] ---[ end trace 358ac5c76c211ea1 ]---
> 
> Debugging further I find the resv_huge_pages underflows by 1
> temporarily during the run of the test multiple times, but a
> __free_huge_page() is always subsequently called that overflows it
> back to 0. resv_huge_pages is always 0 at the end of the test. I've
> initially looked at this as I suspected a problem in the
> resv_huge_pages accounting, but seems the resv_huge_pages accounting
> is fine in itself as it correctly decrements resv_huge_pages when a
> page is allocated from reservation and correctly increments it back up
> when that page is freed.
> 
> I'm not that familiar with the userfaultfd/hugetlb code so I was
> hoping to solicit some suggestions for what the issue could be. Things
> I've tried so far:
> 
> - Adding code that prevents resv_huge_pages to underflow causes the
> test to fail, so it seems in this test the calling code actually
> expects to be able to temporarily allocate 1 more page than the VMA
> has reserved, which seems like a bug maybe?
> - Modifying hugetlb_mcopy_atomic_pte() to not use reserved pages
> causes the test to fail again. Doin that and overprovisioning
> /proc/sys/vm/nr_hugepages causes the test to pass again but I'm not
> sure that's right (not familiar with the code).
> - The failure gets reproduced as far back as 5.11, so it doesn't seem
> to be related to any recent changes.

Hi Mina,

I am fairly confident the issue is with hugetlb_mcopy_atomic_pte.  It
does not detect/handle the case where a page cache page already exists
when in MCOPY_ATOMIC_NORMAL mode.  If you add a printk/warning after the
failure of huge_add_to_page_cache, these will generally correspond to
the underflow.  From a reservation POV, if the page exists in the cache
the reservation was already consumed.  The call to alloc_huge_page will
'consume' another reservation which can lead to the underflow.  As you
noted, this underflow gets cleaned up in the error path.  However, we
should prevent it from happening as we do not want anyone making
decisions on that underflow value.

hugetlb_mcopy_atomic_pte should check for a page in the cache and if it
exists use it in MCOPY_ATOMIC_NORMAL.  This code is quite tricky and my
first simple attempt at this did not work.  I am happy to continue
working on this.  However, if you or anyone else want to jump in and fix
feel free.
-- 
Mike Kravetz