Re: [PATCH] mm/mm_slot: make sure slot is the first element of its wrapper structure

[David Hildenbrand <david@redhat.com>]

On 15.09.25 03:47, Wei Yang wrote:
> On Sun, Sep 14, 2025 at 01:05:48PM +0800, Lance Yang wrote:
>> Hi Wei,
>>
>> On 2025/9/14 08:00, Wei Yang wrote:
>>> When using mm_slot in ksm/khugepaged, there is code snip like:
>>>
>>> 	slot = mm_slot_lookup(mm_slots_hash, mm);
>>> 	mm_slot = mm_slot_entry(slot, struct ksm_mm_slot, slot);
>>> 	if (mm_slot && ..) {
>>> 	}
>>>
>>> This is only valid when mm_slot is the first element of its wrapper
>>> structure, otherwise a NULL slot would converted to a mm_slot with
>>> negative value. And current code thinks it is valid and continue.
>>>
>>> Current code works since mm_slot is the first element, but make sure it
>>> won't be disturbed.
>>
>> Good catch! That's indeed quite brittle ;)
>>
>> Just one nit below.
>>
>>>
>>> Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
>>> ---
>>>    mm/khugepaged.c | 5 ++++-
>>>    mm/ksm.c        | 5 ++++-
>>>    2 files changed, 8 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
>>> index af5f5c80fe4e..668e74ad33b7 100644
>>> --- a/mm/khugepaged.c
>>> +++ b/mm/khugepaged.c
>>> @@ -108,7 +108,7 @@ struct collapse_control {
>>>     * @slot: hash lookup from mm to mm_slot
>>>     */
>>>    struct khugepaged_mm_slot {
>>> -	struct mm_slot slot;
>>> +	struct mm_slot slot;	/* keep it the first element */
>>>    };
>>>    /**
>>> @@ -2382,6 +2382,9 @@ static unsigned int khugepaged_scan_mm_slot(unsigned int pages, int *result,
>>>    	struct vm_area_struct *vma;
>>>    	int progress = 0;
>>> +	BUILD_BUG_ON_MSG(mm_slot_entry(NULL, struct khugepaged_mm_slot, slot),
>>> +			"slot should be the first element");
>>> +
>>
>> I wonder if this BUILD_BUG_ON_MSG() in khugepaged_scan_mm_slot() would
>> be better placed in khugepaged_init(), like you did in ksm_init()?
> 
> khugepaged_mm_slot is defined in khugepaged.c, maybe we don't want to export
> it.
> 

We should probably just use a static_assert right nxt to the struct and 
make sure that offsetof(struct khugepaged_mm_slot, slot) == 0 ?

-- 
Cheers

David / dhildenb