| Message ID | 20220618073243.9423-1-linmiaohe@huawei.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | mm: stricter check on THP migration entry | expand |
On Sat, Jun 18, 2022 at 03:32:43PM +0800, Miaohe Lin wrote: > When VM_BUG_ON check for THP migration entry, the existing code only check > thp_migration_supported case, but not for !thp_migration_supported case. > If !thp_migration_supported() and !pmd_present(), the original code may > dead loop in theory. To make the VM_BUG_ON check more consistent, we need > to catch both cases. This patch makes no sense to me. > Signed-off-by: Miaohe Lin <linmiaohe@huawei.com> > --- > mm/memory.c | 7 +++---- > 1 file changed, 3 insertions(+), 4 deletions(-) > > diff --git a/mm/memory.c b/mm/memory.c > index fee2884481f2..6a9a17c7f58a 100644 > --- a/mm/memory.c > +++ b/mm/memory.c > @@ -5043,10 +5043,9 @@ static vm_fault_t __handle_mm_fault(struct vm_area_struct *vma, > > barrier(); > if (unlikely(is_swap_pmd(vmf.orig_pmd))) { > - VM_BUG_ON(thp_migration_supported() && > - !is_pmd_migration_entry(vmf.orig_pmd)); > - if (is_pmd_migration_entry(vmf.orig_pmd)) > - pmd_migration_entry_wait(mm, vmf.pmd); > + VM_BUG_ON(!thp_migration_supported() || > + !is_pmd_migration_entry(vmf.orig_pmd)); > + pmd_migration_entry_wait(mm, vmf.pmd); > return 0; > } > if (pmd_trans_huge(vmf.orig_pmd) || pmd_devmap(vmf.orig_pmd)) { > -- > 2.23.0 > >
On 2022/6/20 12:48, Matthew Wilcox wrote: > On Sat, Jun 18, 2022 at 03:32:43PM +0800, Miaohe Lin wrote: >> When VM_BUG_ON check for THP migration entry, the existing code only check >> thp_migration_supported case, but not for !thp_migration_supported case. >> If !thp_migration_supported() and !pmd_present(), the original code may >> dead loop in theory. To make the VM_BUG_ON check more consistent, we need >> to catch both cases. > > This patch makes no sense to me. Could you please explain it more? This patch is inspired by below commit indeed: 28b0ee3fb350 ("mm/gup.c: stricter check on THP migration entry during follow_pmd_mask"). Thanks. > >> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com> >> --- >> mm/memory.c | 7 +++---- >> 1 file changed, 3 insertions(+), 4 deletions(-) >> >> diff --git a/mm/memory.c b/mm/memory.c >> index fee2884481f2..6a9a17c7f58a 100644 >> --- a/mm/memory.c >> +++ b/mm/memory.c >> @@ -5043,10 +5043,9 @@ static vm_fault_t __handle_mm_fault(struct vm_area_struct *vma, >> >> barrier(); >> if (unlikely(is_swap_pmd(vmf.orig_pmd))) { >> - VM_BUG_ON(thp_migration_supported() && >> - !is_pmd_migration_entry(vmf.orig_pmd)); >> - if (is_pmd_migration_entry(vmf.orig_pmd)) >> - pmd_migration_entry_wait(mm, vmf.pmd); >> + VM_BUG_ON(!thp_migration_supported() || >> + !is_pmd_migration_entry(vmf.orig_pmd)); >> + pmd_migration_entry_wait(mm, vmf.pmd); >> return 0; >> } >> if (pmd_trans_huge(vmf.orig_pmd) || pmd_devmap(vmf.orig_pmd)) { >> -- >> 2.23.0 >> >> > > . >
diff --git a/mm/memory.c b/mm/memory.c index fee2884481f2..6a9a17c7f58a 100644 --- a/mm/memory.c +++ b/mm/memory.c @@ -5043,10 +5043,9 @@ static vm_fault_t __handle_mm_fault(struct vm_area_struct *vma, barrier(); if (unlikely(is_swap_pmd(vmf.orig_pmd))) { - VM_BUG_ON(thp_migration_supported() && - !is_pmd_migration_entry(vmf.orig_pmd)); - if (is_pmd_migration_entry(vmf.orig_pmd)) - pmd_migration_entry_wait(mm, vmf.pmd); + VM_BUG_ON(!thp_migration_supported() || + !is_pmd_migration_entry(vmf.orig_pmd)); + pmd_migration_entry_wait(mm, vmf.pmd); return 0; } if (pmd_trans_huge(vmf.orig_pmd) || pmd_devmap(vmf.orig_pmd)) {
When VM_BUG_ON check for THP migration entry, the existing code only check thp_migration_supported case, but not for !thp_migration_supported case. If !thp_migration_supported() and !pmd_present(), the original code may dead loop in theory. To make the VM_BUG_ON check more consistent, we need to catch both cases. Signed-off-by: Miaohe Lin <linmiaohe@huawei.com> --- mm/memory.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)