Re: [PATCH 1/4] mm/ksm: rename mm_slot members to ksm_slot for better readability.

[David Hildenbrand]

On 28.04.24 12:06, alexs@xxxxxxxxxx wrote:
> From: "Alex Shi (tencent)" <alexs@xxxxxxxxxx>
>
> mm_slot is a struct of mm, and ksm_mm_slot is named the same again in
> ksm_scan struct. Furthermore, the ksm_mm_slot pointer is named as
> mm_slot again in functions, beside with 'struct mm_slot' variable.
> That makes code readability pretty worse.
>
> struct ksm_mm_slot {
>          struct mm_slot slot;
> 	...
> };
>
> struct ksm_scan {
>          struct ksm_mm_slot *mm_slot;
> 	...
> };
>
> int __ksm_enter(struct mm_struct *mm)
> {
>          struct ksm_mm_slot *mm_slot;
>          struct mm_slot *slot;
> 	...
>
> So let's rename the mm_slot member to ksm_slot in ksm_scan, and ksm_slot
> for ksm_mm_slot* type variables in functions to reduce this confusing.
>
>   struct ksm_scan {
> -       struct ksm_mm_slot *mm_slot;
> +       struct ksm_mm_slot *ksm_slot;
>
> Signed-off-by: Alex Shi (tencent) <alexs@xxxxxxxxxx>
> Cc: David Hildenbrand <david@xxxxxxxxxx>

[...]

>   	}
>   	spin_unlock(&ksm_mmlist_lock);
>   
>   	if (easy_to_free) {
> -		mm_slot_free(mm_slot_cache, mm_slot);
> +		mm_slot_free(mm_slot_cache, ksm_slot);

And at this point I am not sure this is the right decision. You made 
that line more confusing.

Quite some churn for little (no?) benefit.


--
Cheers,

David / dhildenb