Re: [PATCH v1] hugetlb: support FOLL_FORCE|FOLL_WRITE

David Hildenbrand
2024-12-04T19:30:20Z

On 04.12.24 20:13, Guillaume Morin wrote:
> On 04 Dec 20:01, David Hildenbrand wrote:
>>
>> On 04.12.24 19:26, Guillaume Morin wrote:
>>
>> Patch prefix should likely be "mm/hugetlb: ..."
>>
>>> FOLL_FORCE|FOLL_WRITE has never been properly supported for hugetlb
>>> mappings. Since 1d8d14641fd94, we explicitly reject it. However
>>
>> "Since commit 1d8d14641fd9 ("mm/hugetlb: support write-faults in shared
>> mappings") ..."
>
> Will fix in v2.
>
>>
>>> running software on hugetlb mappings is a useful optimization.
>>> Multiple tools allow to use that such as Intel iodlr or
>>> libhugetlbfs.
>>
>> It would be better to link to the actual request where people ran into that
>> when using PTRACE_POKETEXT
>>
>> That hugetlb is getting used is rather obvious :)
>
> Well, allow me to point out that I said running software on a hugetlb
> mapping, not generally using hugetlb.

Well, yes, but that's not really big news. People have been doing that
(and rewriting their apps using libhugetlbfs to place text on huge pages
pre file THP) for decades. :)

See below.

>
> That said, which link are you referring to? The only discussion I am
> aware of is off mailing lists.

Oh, indeed, I could have sworn it was public.

I'd write something like

"Eric reported that PTRACE_POKETEXT fails when applications use hugetlb
for mapping text using huge pages. Before commit 1d8d14641fd9,
PTRACE_POKETEXT worked by accident, but it was buggy and silently ended
up mapping pages writable into the page tables even though VM_WRITE was
not set.

In general, FOLL_FORCE|FOLL_WRITE does currently not work with hugetlb.
Let's implement FOLL_FORCE|FOLL_WRITE properly for hugetlb, such that
what used to work in the past by accident now properly works, allowing
applications using hugetlb for text etc. to get properly debugged.

This change might also be required to implement uprobes support for
hugetlb [1].

[1] link to our discussion
"

--
Cheers,

David / dhildenb