| Message ID | 20220623235153.2623702-3-naoya.horiguchi@linux.dev (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | mm, hwpoison: enable 1GB hugepage support (v2) | expand |
On 2022/6/24 7:51, Naoya Horiguchi wrote: > From: Naoya Horiguchi <naoya.horiguchi@nec.com> > > Originally copy_hugetlb_page_range() handles migration entries and hwpoisone s/hwpoisone/hwpoisoned/ > entries in similar manner. But recently the related code path has more code > for migration entries, and when is_writable_migration_entry() was converted > to !is_readable_migration_entry(), hwpoison entries on source processes got > to be unexpectedly updated (which is legitimate for migration entries, but > not for hwpoison entries). This results in unexpected serious issues like > kernel panic when foking processes with hwpoison entries in pmd. s/foking/forking/ > > Separate the if branch into one for hwpoison entries and one for migration > entries. > > Fixes: 6c287605fd56 ("mm: remember exclusively mapped anonymous pages with PG_anon_exclusive") > Signed-off-by: Naoya Horiguchi <naoya.horiguchi@nec.com> > Cc: <stable@vger.kernel.org> # 5.18 This makes sense to me. Thanks for fixing this. Reviewed-by: Miaohe Lin <linmiaohe@huawei.com> > --- > mm/hugetlb.c | 9 +++++++-- > 1 file changed, 7 insertions(+), 2 deletions(-) > > diff --git a/mm/hugetlb.c b/mm/hugetlb.c > index c538278170a2..f59f43c06601 100644 > --- a/mm/hugetlb.c > +++ b/mm/hugetlb.c > @@ -4784,8 +4784,13 @@ int copy_hugetlb_page_range(struct mm_struct *dst, struct mm_struct *src, > * sharing with another vma. > */ > ; > - } else if (unlikely(is_hugetlb_entry_migration(entry) || > - is_hugetlb_entry_hwpoisoned(entry))) { > + } else if (unlikely(is_hugetlb_entry_hwpoisoned(entry))) { > + bool uffd_wp = huge_pte_uffd_wp(entry); > + > + if (!userfaultfd_wp(dst_vma) && uffd_wp) > + entry = huge_pte_clear_uffd_wp(entry); > + set_huge_swap_pte_at(dst, addr, dst_pte, entry, sz); > + } else if (unlikely(is_hugetlb_entry_migration(entry))) { > swp_entry_t swp_entry = pte_to_swp_entry(entry); > bool uffd_wp = huge_pte_uffd_wp(entry); > >
On 06/24/22 08:51, Naoya Horiguchi wrote: > From: Naoya Horiguchi <naoya.horiguchi@nec.com> > > Originally copy_hugetlb_page_range() handles migration entries and hwpoisone > entries in similar manner. But recently the related code path has more code > for migration entries, and when is_writable_migration_entry() was converted > to !is_readable_migration_entry(), hwpoison entries on source processes got > to be unexpectedly updated (which is legitimate for migration entries, but > not for hwpoison entries). This results in unexpected serious issues like > kernel panic when foking processes with hwpoison entries in pmd. > > Separate the if branch into one for hwpoison entries and one for migration > entries. > > Fixes: 6c287605fd56 ("mm: remember exclusively mapped anonymous pages with PG_anon_exclusive") > Signed-off-by: Naoya Horiguchi <naoya.horiguchi@nec.com> > Cc: <stable@vger.kernel.org> # 5.18 > --- > mm/hugetlb.c | 9 +++++++-- > 1 file changed, 7 insertions(+), 2 deletions(-) Thank you! Reviewed-by: Mike Kravetz <mike.kravetz@oracle.com> (with typos pointed out by Miaohe Lin) Just curious, are there any hwpoisoned tests in any test suite? I run libhugetlbfs tests and ltp on a regular basis which sometimes catch regressions. If there are no tests in any suite today, this might be something we want to consider for future work.
On Fri, Jun 24, 2022 at 05:23:41PM +0800, Miaohe Lin wrote: > On 2022/6/24 7:51, Naoya Horiguchi wrote: > > From: Naoya Horiguchi <naoya.horiguchi@nec.com> > > > > Originally copy_hugetlb_page_range() handles migration entries and hwpoisone > > s/hwpoisone/hwpoisoned/ > > > entries in similar manner. But recently the related code path has more code > > for migration entries, and when is_writable_migration_entry() was converted > > to !is_readable_migration_entry(), hwpoison entries on source processes got > > to be unexpectedly updated (which is legitimate for migration entries, but > > not for hwpoison entries). This results in unexpected serious issues like > > kernel panic when foking processes with hwpoison entries in pmd. > > s/foking/forking/ Sorry for many typos. I somehow forgot to run spell checker. > > > > > Separate the if branch into one for hwpoison entries and one for migration > > entries. > > > > Fixes: 6c287605fd56 ("mm: remember exclusively mapped anonymous pages with PG_anon_exclusive") > > Signed-off-by: Naoya Horiguchi <naoya.horiguchi@nec.com> > > Cc: <stable@vger.kernel.org> # 5.18 > > This makes sense to me. Thanks for fixing this. > > Reviewed-by: Miaohe Lin <linmiaohe@huawei.com> Thank you for reviewing! - Naoya Horiguchi > > > --- > > mm/hugetlb.c | 9 +++++++-- > > 1 file changed, 7 insertions(+), 2 deletions(-) > > > > diff --git a/mm/hugetlb.c b/mm/hugetlb.c > > index c538278170a2..f59f43c06601 100644 > > --- a/mm/hugetlb.c > > +++ b/mm/hugetlb.c > > @@ -4784,8 +4784,13 @@ int copy_hugetlb_page_range(struct mm_struct *dst, struct mm_struct *src, > > * sharing with another vma. > > */ > > ; > > - } else if (unlikely(is_hugetlb_entry_migration(entry) || > > - is_hugetlb_entry_hwpoisoned(entry))) { > > + } else if (unlikely(is_hugetlb_entry_hwpoisoned(entry))) { > > + bool uffd_wp = huge_pte_uffd_wp(entry); > > + > > + if (!userfaultfd_wp(dst_vma) && uffd_wp) > > + entry = huge_pte_clear_uffd_wp(entry); > > + set_huge_swap_pte_at(dst, addr, dst_pte, entry, sz); > > + } else if (unlikely(is_hugetlb_entry_migration(entry))) { > > swp_entry_t swp_entry = pte_to_swp_entry(entry); > > bool uffd_wp = huge_pte_uffd_wp(entry); > > > >
On Fri, Jun 24, 2022 at 01:57:57PM -0700, Mike Kravetz wrote: > On 06/24/22 08:51, Naoya Horiguchi wrote: > > From: Naoya Horiguchi <naoya.horiguchi@nec.com> > > > > Originally copy_hugetlb_page_range() handles migration entries and hwpoisone > > entries in similar manner. But recently the related code path has more code > > for migration entries, and when is_writable_migration_entry() was converted > > to !is_readable_migration_entry(), hwpoison entries on source processes got > > to be unexpectedly updated (which is legitimate for migration entries, but > > not for hwpoison entries). This results in unexpected serious issues like > > kernel panic when foking processes with hwpoison entries in pmd. > > > > Separate the if branch into one for hwpoison entries and one for migration > > entries. > > > > Fixes: 6c287605fd56 ("mm: remember exclusively mapped anonymous pages with PG_anon_exclusive") > > Signed-off-by: Naoya Horiguchi <naoya.horiguchi@nec.com> > > Cc: <stable@vger.kernel.org> # 5.18 > > --- > > mm/hugetlb.c | 9 +++++++-- > > 1 file changed, 7 insertions(+), 2 deletions(-) > > Thank you! > > Reviewed-by: Mike Kravetz <mike.kravetz@oracle.com> > (with typos pointed out by Miaohe Lin) > > Just curious, are there any hwpoisoned tests in any test suite? I run > libhugetlbfs tests and ltp on a regular basis which sometimes catch > regressions. If there are no tests in any suite today, this might be > something we want to consider for future work. mce-tests (https://git.kernel.org/pub/scm/utils/cpu/mce/mce-test.git) has some test cases about hwpoison (under cases/function/hwpoison/), but this tool mostly focuses on MCE and does not have enough hwpoison testcases. So I'm maintaining my own test suite (https://github.com/nhoriguchi/mm_regression) for long to help my own hwpoison development/maintenance. I'd like to make this tool more handy so that other developers or testsuites can easily run the hwpoison testing, although I need more work for it. Thanks, Naoya Horiguchi
On Fri, Jun 24, 2022 at 08:51:46AM +0900, Naoya Horiguchi wrote: > From: Naoya Horiguchi <naoya.horiguchi@nec.com> > > Originally copy_hugetlb_page_range() handles migration entries and hwpoisone > entries in similar manner. But recently the related code path has more code > for migration entries, and when is_writable_migration_entry() was converted > to !is_readable_migration_entry(), hwpoison entries on source processes got > to be unexpectedly updated (which is legitimate for migration entries, but > not for hwpoison entries). This results in unexpected serious issues like > kernel panic when foking processes with hwpoison entries in pmd. > > Separate the if branch into one for hwpoison entries and one for migration > entries. > > Fixes: 6c287605fd56 ("mm: remember exclusively mapped anonymous pages with PG_anon_exclusive") > Signed-off-by: Naoya Horiguchi <naoya.horiguchi@nec.com> Thanks for fixing it. Reviewed-by: Muchun Song <songmuchun@bytedance.com>
diff --git a/mm/hugetlb.c b/mm/hugetlb.c index c538278170a2..f59f43c06601 100644 --- a/mm/hugetlb.c +++ b/mm/hugetlb.c @@ -4784,8 +4784,13 @@ int copy_hugetlb_page_range(struct mm_struct *dst, struct mm_struct *src, * sharing with another vma. */ ; - } else if (unlikely(is_hugetlb_entry_migration(entry) || - is_hugetlb_entry_hwpoisoned(entry))) { + } else if (unlikely(is_hugetlb_entry_hwpoisoned(entry))) { + bool uffd_wp = huge_pte_uffd_wp(entry); + + if (!userfaultfd_wp(dst_vma) && uffd_wp) + entry = huge_pte_clear_uffd_wp(entry); + set_huge_swap_pte_at(dst, addr, dst_pte, entry, sz); + } else if (unlikely(is_hugetlb_entry_migration(entry))) { swp_entry_t swp_entry = pte_to_swp_entry(entry); bool uffd_wp = huge_pte_uffd_wp(entry);