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

[Dev Jain <dev.jain@arm.com>]

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() :)