linux-kernel - Re: [PATCH 1/4] mm/ksm: rename mm_slot members to ksm

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <24688466-6815-4aac-a8b9-4373a534727f@redhat.com>
Date: Tue, 30 Apr 2024 14:53:48 +0200
From: David Hildenbrand <david@...hat.com>
To: alexs@...nel.org, Andrew Morton <akpm@...ux-foundation.org>,
 linux-mm@...ck.org, linux-kernel@...r.kernel.org, willy@...radead.org,
 izik.eidus@...ellosystems.com
Subject: Re: [PATCH 1/4] mm/ksm: rename mm_slot members to ksm_slot for better
 readability.

On 28.04.24 12:06, alexs@...nel.org wrote:
> From: "Alex Shi (tencent)" <alexs@...nel.org>
> 
> 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@...nel.org>
> Cc: David Hildenbrand <david@...hat.com>

[...]

>   	}
>   	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