[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <qgcvsfhoq3pnvxhdn73dopbtvi75oghbaydg27atqfp556u6sa@ixwdwi73lgkl>
Date: Wed, 16 Jul 2025 10:29:28 -0400
From: "Liam R. Howlett" <Liam.Howlett@...cle.com>
To: Nico Pache <npache@...hat.com>
Cc: linux-mm@...ck.org, linux-doc@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-trace-kernel@...r.kernel.org,
david@...hat.com, ziy@...dia.com, baolin.wang@...ux.alibaba.com,
lorenzo.stoakes@...cle.com, ryan.roberts@....com, dev.jain@....com,
corbet@....net, rostedt@...dmis.org, mhiramat@...nel.org,
mathieu.desnoyers@...icios.com, akpm@...ux-foundation.org,
baohua@...nel.org, willy@...radead.org, peterx@...hat.com,
wangkefeng.wang@...wei.com, usamaarif642@...il.com,
sunnanyong@...wei.com, vishal.moola@...il.com,
thomas.hellstrom@...ux.intel.com, yang@...amperecomputing.com,
kirill.shutemov@...ux.intel.com, aarcange@...hat.com,
raquini@...hat.com, anshuman.khandual@....com, catalin.marinas@....com,
tiwai@...e.de, will@...nel.org, dave.hansen@...ux.intel.com,
jack@...e.cz, cl@...two.org, jglisse@...gle.com, surenb@...gle.com,
zokeefe@...gle.com, hannes@...xchg.org, rientjes@...gle.com,
mhocko@...e.com, rdunlap@...radead.org, hughd@...gle.com
Subject: Re: [PATCH v9 01/14] khugepaged: rename hpage_collapse_* to
collapse_*
* Nico Pache <npache@...hat.com> [250713 20:33]:
> The hpage_collapse functions describe functions used by madvise_collapse
> and khugepaged. remove the unnecessary hpage prefix to shorten the
> function name.
>
> Reviewed-by: Zi Yan <ziy@...dia.com>
> Reviewed-by: Baolin Wang <baolin.wang@...ux.alibaba.com>
> Signed-off-by: Nico Pache <npache@...hat.com>
This is funny. I suggested this sort of thing in v7 but you said that
David H. said what to do, but then in v8 there was a discussion where
David said differently..
Yes, I much prefer dropping the prefix that is already implied by the
file for static inline functions than anything else from the names.
Thanks, this looks nicer.
> ---
> mm/khugepaged.c | 46 +++++++++++++++++++++++-----------------------
> 1 file changed, 23 insertions(+), 23 deletions(-)
>
> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> index a55fb1dcd224..eb0babb51868 100644
> --- a/mm/khugepaged.c
> +++ b/mm/khugepaged.c
> @@ -402,14 +402,14 @@ void __init khugepaged_destroy(void)
> kmem_cache_destroy(mm_slot_cache);
> }
>
> -static inline int hpage_collapse_test_exit(struct mm_struct *mm)
> +static inline int collapse_test_exit(struct mm_struct *mm)
> {
> return atomic_read(&mm->mm_users) == 0;
> }
...
> -static int hpage_collapse_scan_pmd(struct mm_struct *mm,
> +static int collapse_scan_pmd(struct mm_struct *mm,
> struct vm_area_struct *vma,
> unsigned long address, bool *mmap_locked,
> struct collapse_control *cc)
One thing I noticed here.
Usually we try to do two tab indents on arguments because it allows for
less lines and less churn on argument list edits.
That is, if you have two tabs then it does not line up with the code
below and allows more arguments on the same line.
It also means that if the name changes, then you don't have to change
the white space of the argument list.
On that note, the spacing is now off where the names changed, but this
isn't a huge deal and I suspect it changes later anyways? Anyways, this
is more of a nit than anything.. The example above looks like it didn't
line up to begin with.
...
Reviewed-by: Liam R. Howlett <Liam.Howlett@...cle.com>
Powered by blists - more mailing lists