linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: "Liam R. Howlett" <Liam.Howlett@oracle.com>
To: Nico Pache <npache@redhat.com>
Cc: linux-mm@kvack.org, linux-doc@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-trace-kernel@vger.kernel.org,
	david@redhat.com, ziy@nvidia.com, baolin.wang@linux.alibaba.com,
	lorenzo.stoakes@oracle.com, ryan.roberts@arm.com,
	dev.jain@arm.com, corbet@lwn.net, rostedt@goodmis.org,
	mhiramat@kernel.org, mathieu.desnoyers@efficios.com,
	akpm@linux-foundation.org, baohua@kernel.org,
	willy@infradead.org, peterx@redhat.com,
	wangkefeng.wang@huawei.com, usamaarif642@gmail.com,
	sunnanyong@huawei.com, vishal.moola@gmail.com,
	thomas.hellstrom@linux.intel.com, yang@os.amperecomputing.com,
	kirill.shutemov@linux.intel.com, aarcange@redhat.com,
	raquini@redhat.com, anshuman.khandual@arm.com,
	catalin.marinas@arm.com, tiwai@suse.de, will@kernel.org,
	dave.hansen@linux.intel.com, jack@suse.cz, cl@gentwo.org,
	jglisse@google.com, surenb@google.com, zokeefe@google.com,
	hannes@cmpxchg.org, rientjes@google.com, mhocko@suse.com,
	rdunlap@infradead.org, hughd@google.com
Subject: Re: [PATCH v9 01/14] khugepaged: rename hpage_collapse_* to collapse_*
Date: Wed, 16 Jul 2025 10:29:28 -0400	[thread overview]
Message-ID: <qgcvsfhoq3pnvxhdn73dopbtvi75oghbaydg27atqfp556u6sa@ixwdwi73lgkl> (raw)
In-Reply-To: <20250714003207.113275-2-npache@redhat.com>

* Nico Pache <npache@redhat.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@nvidia.com>
> Reviewed-by: Baolin Wang <baolin.wang@linux.alibaba.com>
> Signed-off-by: Nico Pache <npache@redhat.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@oracle.com>


  parent reply	other threads:[~2025-07-16 14:30 UTC|newest]

Thread overview: 51+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-07-14  0:31 [PATCH v9 00/14] khugepaged: mTHP support Nico Pache
2025-07-14  0:31 ` [PATCH v9 01/14] khugepaged: rename hpage_collapse_* to collapse_* Nico Pache
2025-07-15 15:39   ` David Hildenbrand
2025-07-16 14:29   ` Liam R. Howlett [this message]
2025-07-16 15:20     ` David Hildenbrand
2025-07-17  7:21     ` Nico Pache
2025-07-25 16:43   ` Lorenzo Stoakes
2025-07-25 22:35     ` Nico Pache
2025-07-14  0:31 ` [PATCH v9 02/14] introduce collapse_single_pmd to unify khugepaged and madvise_collapse Nico Pache
2025-07-15 15:53   ` David Hildenbrand
2025-07-23  1:56     ` Nico Pache
2025-07-16 15:12   ` Liam R. Howlett
2025-07-23  1:55     ` Nico Pache
2025-07-14  0:31 ` [PATCH v9 03/14] khugepaged: generalize hugepage_vma_revalidate for mTHP support Nico Pache
2025-07-15 15:55   ` David Hildenbrand
2025-07-14  0:31 ` [PATCH v9 04/14] khugepaged: generalize alloc_charge_folio() Nico Pache
2025-07-16 13:46   ` David Hildenbrand
2025-07-17  7:22     ` Nico Pache
2025-07-14  0:31 ` [PATCH v9 05/14] khugepaged: generalize __collapse_huge_page_* for mTHP support Nico Pache
2025-07-16 13:52   ` David Hildenbrand
2025-07-17  7:22     ` Nico Pache
2025-07-16 14:02   ` David Hildenbrand
2025-07-17  7:23     ` Nico Pache
2025-07-17 15:54     ` Lorenzo Stoakes
2025-07-25 16:09   ` Lorenzo Stoakes
2025-07-25 22:37     ` Nico Pache
2025-07-14  0:31 ` [PATCH v9 06/14] khugepaged: introduce collapse_scan_bitmap " Nico Pache
2025-07-16 14:03   ` David Hildenbrand
2025-07-17  7:23     ` Nico Pache
2025-07-16 15:38   ` Liam R. Howlett
2025-07-17  7:24     ` Nico Pache
2025-07-14  0:32 ` [PATCH v9 07/14] khugepaged: add " Nico Pache
2025-07-14  0:32 ` [PATCH v9 08/14] khugepaged: skip collapsing mTHP to smaller orders Nico Pache
2025-07-16 14:32   ` David Hildenbrand
2025-07-17  7:24     ` Nico Pache
2025-07-14  0:32 ` [PATCH v9 09/14] khugepaged: avoid unnecessary mTHP collapse attempts Nico Pache
2025-07-18  2:14   ` Baolin Wang
2025-07-18 22:34     ` Nico Pache
2025-07-14  0:32 ` [PATCH v9 10/14] khugepaged: allow khugepaged to check all anonymous mTHP orders Nico Pache
2025-07-16 15:28   ` David Hildenbrand
2025-07-17  7:25     ` Nico Pache
2025-07-18  8:40       ` David Hildenbrand
2025-07-14  0:32 ` [PATCH v9 11/14] khugepaged: kick khugepaged for enabling none-PMD-sized mTHPs Nico Pache
2025-07-14  0:32 ` [PATCH v9 12/14] khugepaged: improve tracepoints for mTHP orders Nico Pache
2025-07-22 15:39   ` David Hildenbrand
2025-07-14  0:32 ` [PATCH v9 13/14] khugepaged: add per-order mTHP khugepaged stats Nico Pache
2025-07-18  5:04   ` Baolin Wang
2025-07-18 21:00     ` Nico Pache
2025-07-19  4:42       ` Baolin Wang
2025-07-14  0:32 ` [PATCH v9 14/14] Documentation: mm: update the admin guide for mTHP collapse Nico Pache
2025-07-15  0:39 ` [PATCH v9 00/14] khugepaged: mTHP support Andrew Morton

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=qgcvsfhoq3pnvxhdn73dopbtvi75oghbaydg27atqfp556u6sa@ixwdwi73lgkl \
    --to=liam.howlett@oracle.com \
    --cc=aarcange@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=anshuman.khandual@arm.com \
    --cc=baohua@kernel.org \
    --cc=baolin.wang@linux.alibaba.com \
    --cc=catalin.marinas@arm.com \
    --cc=cl@gentwo.org \
    --cc=corbet@lwn.net \
    --cc=dave.hansen@linux.intel.com \
    --cc=david@redhat.com \
    --cc=dev.jain@arm.com \
    --cc=hannes@cmpxchg.org \
    --cc=hughd@google.com \
    --cc=jack@suse.cz \
    --cc=jglisse@google.com \
    --cc=kirill.shutemov@linux.intel.com \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=linux-trace-kernel@vger.kernel.org \
    --cc=lorenzo.stoakes@oracle.com \
    --cc=mathieu.desnoyers@efficios.com \
    --cc=mhiramat@kernel.org \
    --cc=mhocko@suse.com \
    --cc=npache@redhat.com \
    --cc=peterx@redhat.com \
    --cc=raquini@redhat.com \
    --cc=rdunlap@infradead.org \
    --cc=rientjes@google.com \
    --cc=rostedt@goodmis.org \
    --cc=ryan.roberts@arm.com \
    --cc=sunnanyong@huawei.com \
    --cc=surenb@google.com \
    --cc=thomas.hellstrom@linux.intel.com \
    --cc=tiwai@suse.de \
    --cc=usamaarif642@gmail.com \
    --cc=vishal.moola@gmail.com \
    --cc=wangkefeng.wang@huawei.com \
    --cc=will@kernel.org \
    --cc=willy@infradead.org \
    --cc=yang@os.amperecomputing.com \
    --cc=ziy@nvidia.com \
    --cc=zokeefe@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).