Re: [RFC PATCH 0/4] mm, bpf: BPF based THP adjustment

[Zi Yan <ziy@nvidia.com>]

On 30 Apr 2025, at 10:38, Yafang Shao wrote:

> On Wed, Apr 30, 2025 at 9:19 PM Zi Yan <ziy@nvidia.com> wrote:
>>
>> On 29 Apr 2025, at 22:33, Yafang Shao wrote:
>>
>>> On Tue, Apr 29, 2025 at 11:09 PM Zi Yan <ziy@nvidia.com> wrote:
>>>>
>>>> Hi Yafang,
>>>>
>>>> We recently added a new THP entry in MAINTAINERS file[1], do you mind ccing
>>>> people there in your next version? (I added them here)
>>>>
>>>> [1] https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git/tree/MAINTAINERS?h=mm-everything#n15589
>>>
>>> Thanks for your reminder.
>>> I will add the maintainers and reviewers in the next version.
>>>
>>>>
>>>> On Mon Apr 28, 2025 at 10:41 PM EDT, Yafang Shao wrote:
>>>>> In our container environment, we aim to enable THP selectively—allowing
>>>>> specific services to use it while restricting others. This approach is
>>>>> driven by the following considerations:
>>>>>
>>>>> 1. Memory Fragmentation
>>>>>    THP can lead to increased memory fragmentation, so we want to limit its
>>>>>    use across services.
>>>>> 2. Performance Impact
>>>>>    Some services see no benefit from THP, making its usage unnecessary.
>>>>> 3. Performance Gains
>>>>>    Certain workloads, such as machine learning services, experience
>>>>>    significant performance improvements with THP, so we enable it for them
>>>>>    specifically.
>>>>>
>>>>> Since multiple services run on a single host in a containerized environment,
>>>>> enabling THP globally is not ideal. Previously, we set THP to madvise,
>>>>> allowing selected services to opt in via MADV_HUGEPAGE. However, this
>>>>> approach had limitation:
>>>>>
>>>>> - Some services inadvertently used madvise(MADV_HUGEPAGE) through
>>>>>   third-party libraries, bypassing our restrictions.
>>>>
>>>> Basically, you want more precise control of THP enablement and the
>>>> ability of overriding madvise() from userspace.
>>>>
>>>> In terms of overriding madvise(), do you have any concrete example of
>>>> these third-party libraries? madvise() users are supposed to know what
>>>> they are doing, so I wonder why they are causing trouble in your
>>>> environment.
>>>
>>> To my knowledge, jemalloc [0] supports THP.
>>> Applications using jemalloc typically rely on its default
>>> configurations rather than explicitly enabling or disabling THP. If
>>> the system is configured with THP=madvise, these applications may
>>> automatically leverage THP where appropriate
>>>
>>> [0]. https://github.com/jemalloc/jemalloc
>>
>> It sounds like a userspace issue. For jemalloc, if applications require
>> it, can't you replace the jemalloc with a one compiled with --disable-thp
>> to work around the issue?
>
> That’s not the issue this patchset is trying to address or work
> around. I believe we should focus on the actual problem it's meant to
> solve.
>
> By the way, you might not raise this question if you were managing a
> large fleet of servers. We're a platform provider, but we don’t
> maintain all the packages ourselves. Users make their own choices
> based on their specific requirements. It's not a feasible solution for
> us to develop and maintain every package.

Basically, user wants to use THP, but as a service provider, you think
differently, so want to override userspace choice. Am I getting it right?

>
>>
>>>
>>>>
>>>>>
>>>>> To address this issue, we initially hooked the __x64_sys_madvise() syscall,
>>>>> which is error-injectable, to blacklist unwanted services. While this
>>>>> worked, it was error-prone and ineffective for services needing always mode,
>>>>> as modifying their code to use madvise was impractical.
>>>>>
>>>>> To achieve finer-grained control, we introduced an fmod_ret-based solution.
>>>>> Now, we dynamically adjust THP settings per service by hooking
>>>>> hugepage_global_{enabled,always}() via BPF. This allows us to set THP to
>>>>> enable or disable on a per-service basis without global impact.
>>>>
>>>> hugepage_global_*() are whole system knobs. How did you use it to
>>>> achieve per-service control? In terms of per-service, does it mean
>>>> you need per-memcg group (I assume each service has its own memcg) THP
>>>> configuration?
>>>
>>> With this new BPF hook, we can manage THP behavior either per-service
>>> or per-memory.
>>> In our use case, we’ve chosen memcg-based control for finer-grained
>>> management. Below is a simplified example of our implementation:
>>>
>>> struct{
>>>         __uint(type, BPF_MAP_TYPE_HASH);
>>>         __uint(max_entries, 4096);      /* usually there won't too
>>> many cgroups */
>>>         __type(key, u64);
>>>         __type(value, u32);
>>>         __uint(map_flags, BPF_F_NO_PREALLOC);
>>> } thp_whitelist SEC(".maps");
>>>
>>> SEC("fmod_ret/mm_bpf_thp_vma_allowable")
>>> int BPF_PROG(thp_vma_allowable, struct vm_area_struct *vma)
>>> {
>>>         struct cgroup_subsys_state *css;
>>>         struct css_set *cgroups;
>>>         struct mm_struct *mm;
>>>         struct cgroup *cgroup;
>>>         struct cgroup *parent;
>>>         struct task_struct *p;
>>>         u64 cgrp_id;
>>>
>>>         if (!vma)
>>>                 return 0;
>>>
>>>         mm = vma->vm_mm;
>>>         if (!mm)
>>>                 return 0;
>>>
>>>         p = mm->owner;
>>>         cgroups = p->cgroups;
>>>         cgroup = cgroups->subsys[memory_cgrp_id]->cgroup;
>>>         cgrp_id = cgroup->kn->id;
>>>
>>>         /* Allow the tasks in the thp_whiltelist to use THP. */
>>>         if (bpf_map_lookup_elem(&thp_whitelist, &cgrp_id))
>>>             return 1;
>>>         return 0;
>>> }
>>>
>>> I chose not to include this in the self-tests to avoid the complexity
>>> of setting up cgroups for testing purposes. However, in patch #4 of
>>> this series, I've included a simpler example demonstrating task-level
>>> control.
>>
>> For task-level control, why not using prctl(PR_SET_THP_DISABLE)?
>
> You’ll need to modify the user-space code—and again, this likely
> wouldn’t be a concern if you were managing a large fleet of servers.
>
>>
>>> For service-level control, we could potentially utilize BPF task local
>>> storage as an alternative approach.
>>
>> +cgroup people
>>
>> For service-level control, there was a proposal of adding cgroup based
>> THP control[1]. You might need a strong use case to convince people.
>>
>> [1] https://lore.kernel.org/linux-mm/20241030083311.965933-1-gutierrez.asier@huawei-partners.com/
>
> Thanks for the reference. I've reviewed the related discussion, and if
> I understand correctly, the proposal was rejected by the maintainers.

I wonder why your approach is better than the cgroup based THP control proposal.

--
Best Regards,
Yan, Zi