Re: [PATCH v11 0/3] KSM: performance optimizations for rmap_walk_ksm

[David Hildenbrand (Arm)]

On 7/6/26 06:40, xu.xin16@xxxxxxxxxx wrote:
>> Again, thanks for persisting with this.  Great improvement and I'd love
>> to see us get this finished off.
>>
>> Please have a look through the AI review:
>>
>> 	https://sashiko.dev/#/patchset/20260703162253688u8Str9eFLR8TGCmo7nIOF@xxxxxxxxxx
>>
>> That 32-bit thing looks legit, kinda fatal but easy to address.
>>
> 
> Hi, Andrew, This is not a major issue. I think it's acceptable and detailed
> analysis is as follows:
> 
> The sashiko said:
> "
>> Looking at struct ksm_rmap_item in mm/ksm.c, the linear_page_index (unsigned
>> long) is placed in a union with an anonymous struct containing oldchecksum
>> (unsigned int), age (rmap_age_t, 1 byte), and remaining_skips (rmap_age_t,
>> 1 byte).
>> On 32-bit platforms, an unsigned long is only 4 bytes, while the anonymous
>> struct is at least 6 bytes. When break_cow(), remove_node_from_stable_tree(),
>> and remove_rmap_item_from_tree() assign rmap_item->linear_page_index = 0 to
>> reset the age-related information, it appears this will only zero the first 4
>> bytes (clearing oldchecksum). The age and remaining_skips fields would retain
>> their stale historical values from before the page was originally merged.
> 
> Indeed, on 32-bit CPU, that patch doesn't clear age-related information, but it
> doesn't introduce any issue because at this point the linear_page_index and
> age-related information do not overlap, and there is no conflict between them.
> This is clearly consistent with the logic before applying the patch, so there
> won't be any problem.

It keeps bringing that up, and we clearly document "might overlay age-related
information". So just ignore this.

-- 
Cheers,

David