Messages in this thread | ![/](/images/icornerl.gif) | | Date | Tue, 30 Apr 2024 14:53:48 +0200 | Subject | Re: [PATCH 1/4] mm/ksm: rename mm_slot members to ksm_slot for better readability. | From | David Hildenbrand <> |
| |
On 28.04.24 12:06, alexs@kernel.org wrote: > From: "Alex Shi (tencent)" <alexs@kernel.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@kernel.org> > Cc: David Hildenbrand <david@redhat.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
| ![\](/images/icornerr.gif) |