On 03/04/2024 09:22, David Hildenbrand wrote:
> On 03.04.24 05:55, Barry Song wrote:
>> From: Barry Song <v-songbaohua@xxxxxxxx>
>>
>> Profiling a system blindly with mTHP has become challenging due
>> to the lack of visibility into its operations. Presenting the
>> success rate of mTHP allocations appears to be pressing need.
>>
>> Recently, I've been experiencing significant difficulty debugging
>> performance improvements and regressions without these figures.
>> It's crucial for us to understand the true effectiveness of
>> mTHP in real-world scenarios, especially in systems with
>> fragmented memory.
>>
>> This patch sets up the framework for per-order mTHP counters,
>> starting with the introduction of anon_alloc_success and
>> anon_alloc_fail counters. Incorporating additional counters
>> should now be straightforward as well.
>>
>> Signed-off-by: Barry Song <v-songbaohua@xxxxxxxx>
>> ---
>> -v3:
>> * save some memory as order-0 and order-1 can't be THP, Ryan;
>> * rename to anon_alloc as right now we only support anon to address
>> David's comment;
>> * drop a redundant "else", Ryan
>>
>> include/linux/huge_mm.h | 18 ++++++++++++++
>> mm/huge_memory.c | 54 +++++++++++++++++++++++++++++++++++++++++
>> mm/memory.c | 2 ++
>> 3 files changed, 74 insertions(+)
>>
>> diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h
>> index e896ca4760f6..5e9af6be9537 100644
>> --- a/include/linux/huge_mm.h
>> +++ b/include/linux/huge_mm.h
>> @@ -70,6 +70,7 @@ extern struct kobj_attribute shmem_enabled_attr;
>> * (which is a limitation of the THP implementation).
>> */
>> #define THP_ORDERS_ALL_ANON ((BIT(PMD_ORDER + 1) - 1) & ~(BIT(0) | BIT(1)))
>> +#define THP_MIN_ORDER 2
>> /*
>> * Mask of all large folio orders supported for file THP.
>> @@ -264,6 +265,23 @@ unsigned long thp_vma_allowable_orders(struct
>> vm_area_struct *vma,
>> enforce_sysfs, orders);
>> }
>> +enum thp_event_item {
>> + THP_ANON_ALLOC_SUCCESS,
>> + THP_ANON_ALLOC_FAIL,
>> + NR_THP_EVENT_ITEMS
>> +};
>
> Maybe use a prefix that resembles matches the enum name and is "obviously"
> different to the ones in vm_event_item.h, like
>
> enum thp_event {
> THP_EVENT_ANON_ALLOC_SUCCESS,
> THP_EVENT_ANON_ALLOC_FAIL,
> __THP_EVENT_COUNT,
> };
FWIW, I'd personally replace "event" with "stat". For me "event" only ever
increments, but "stat" can increment and decrement. An event is a type of stat.
You are only adding events for now, but we have identified a need for inc/dec
stats that will be added in future.
