Re: [PATCH v4 02/12] mm/rmap: Add try_to_unmap_hugetlb_one

[Lance Yang]

On 2026/6/22 16:13, Dev Jain wrote:
> On 18/06/26 3:31 pm, Lance Yang wrote:
> > On 2026/6/18 17:43, David Hildenbrand (Arm) wrote:
> > > On 6/18/26 11:09, Lance Yang wrote:
> > > > On 2026/6/18 15:55, David Hildenbrand (Arm) wrote:
> > > > > >       return __rste_to_pte(pte_val(*ptep));
> > > > > > }
> > > > > >
> > > > > >
> > > > > > So plain ptep_get() can feed raw huge-entry bits into pte_pfn(), and the
> > > > > > derived subpage can be wrong.
> > > > >
> > > > > Good question which impact that might have in practice?
> > > >
> > > > The subpage check can warn, but we still pass that subpage to
> > > > make_hwpoison_entry(). So the hwpoison marker can end up with the
> > > > wrong PFN?
> > > >
> > > > +    subpage = folio_page(folio, pte_pfn(pteval) - folio_pfn(folio));
> > > > +    VM_WARN_ON(folio_page(folio, 0) != subpage);
> > > > [...]
> > > > +    pteval = swp_entry_to_pte(make_hwpoison_entry(subpage));
> > >
> > > My s390x page table knowledge is a bit rusty.
> > >
> > > IIUC, it would be a problem if some PTE bits in segment/region entries (pmd/pud/
> > > ...) would pass the
> > >
> > >       pte_pfn(x) -> (pte_val(x) >> PAGE_SHIFT)
> > >
> > > check. I don't think this applies, because
> > >
> > > While
> > >      #define _SEGMENT_ENTRY_ORIGIN_LARGE ~0xfffffUL
> > >
> > > We also have
> > >
> > >      #define _SEGMENT_ENTRY_ORIGIN ~0x7ffUL
> > >
> > > So these bits are not actually used.
> > >
> > > What __rste_to_pte() primarily does is reshuffling present bits etc.
> > >
> > > So using any other bits besides the PFN would be problematic I guess.
> > >
> > > Am I wrong or isn't the present bit already at a different location? For
> > > prot-none hugetlb folios there might be a real issue, as the PTE present bit
> > > corresponds to the PMD/PUD read-permission bit.
> > >
> > >
> > > Oh my :)
> > >
> > > So yeah, we should probably fix that ahead of time unless I am missing
> > > something? Good that we separate that hugetlb crap out.
> >
> > Yeah, looks like this was already there before the split. Should this
> > be fixed separately?
>
> Same bug is there in try_to_migrate_one(), check_pte(), remove_migration_pte()
> and prot_none_hugetlb_entry() :)

Oh my :)