| Message ID | 20241002195547.30617-1-kanchana.p.sridhar@intel.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] mm: swap: Call count_mthp_stat() outside ifdef CONFIG_TRANSPARENT_HUGEPAGE. | expand |
On Wed, 2 Oct 2024 12:55:47 -0700 Kanchana P Sridhar <kanchana.p.sridhar@intel.com> wrote: > This patch moves the call to count_mthp_stat() in count_swpout_vm_event() > and in shrink_folio_list() to be outside the > "ifdef CONFIG_TRANSPARENT_HUGEPAGE" This is very apparent from reading the patch. Changelogs and code comments should explain "why", and avoid explaining "what". > based on changes made in commit > 246d3aa3e531 ("mm: cleanup count_mthp_stat() definition"). And I don't think that explains the reasons for this change either. So please resend with a changelog which fully explains the reasons for making this alteration.
> -----Original Message----- > From: Andrew Morton <akpm@linux-foundation.org> > Sent: Wednesday, October 2, 2024 1:37 PM > To: Sridhar, Kanchana P <kanchana.p.sridhar@intel.com> > Cc: linux-kernel@vger.kernel.org; linux-mm@kvack.org; > hannes@cmpxchg.org; yosryahmed@google.com; nphamcs@gmail.com; > chengming.zhou@linux.dev; ryan.roberts@arm.com; Huang, Ying > <ying.huang@intel.com>; 21cnbao@gmail.com; Feghali, Wajdi K > <wajdi.k.feghali@intel.com>; Gopal, Vinodh <vinodh.gopal@intel.com> > Subject: Re: [PATCH v2] mm: swap: Call count_mthp_stat() outside ifdef > CONFIG_TRANSPARENT_HUGEPAGE. > > On Wed, 2 Oct 2024 12:55:47 -0700 Kanchana P Sridhar > <kanchana.p.sridhar@intel.com> wrote: > > > This patch moves the call to count_mthp_stat() in > count_swpout_vm_event() > > and in shrink_folio_list() to be outside the > > "ifdef CONFIG_TRANSPARENT_HUGEPAGE" > > This is very apparent from reading the patch. Changelogs and code > comments should explain "why", and avoid explaining "what". > > > based on changes made in commit > > 246d3aa3e531 ("mm: cleanup count_mthp_stat() definition"). > > And I don't think that explains the reasons for this change either. > > So please resend with a changelog which fully explains the reasons for > making this alteration. Thanks Andrew, for the comments. Sure, I will resend with these suggestions incorporated! Thanks, Kanchana
diff --git a/mm/page_io.c b/mm/page_io.c index 4aa34862676f..a28d28b6b3ce 100644 --- a/mm/page_io.c +++ b/mm/page_io.c @@ -289,8 +289,8 @@ static inline void count_swpout_vm_event(struct folio *folio) count_memcg_folio_events(folio, THP_SWPOUT, 1); count_vm_event(THP_SWPOUT); } - count_mthp_stat(folio_order(folio), MTHP_STAT_SWPOUT); #endif + count_mthp_stat(folio_order(folio), MTHP_STAT_SWPOUT); count_memcg_folio_events(folio, PSWPOUT, folio_nr_pages(folio)); count_vm_events(PSWPOUT, folio_nr_pages(folio)); } diff --git a/mm/vmscan.c b/mm/vmscan.c index dc7a285b256b..50dc06d55b1d 100644 --- a/mm/vmscan.c +++ b/mm/vmscan.c @@ -1257,8 +1257,8 @@ static unsigned int shrink_folio_list(struct list_head *folio_list, THP_SWPOUT_FALLBACK, 1); count_vm_event(THP_SWPOUT_FALLBACK); } - count_mthp_stat(order, MTHP_STAT_SWPOUT_FALLBACK); #endif + count_mthp_stat(order, MTHP_STAT_SWPOUT_FALLBACK); if (!add_to_swap(folio)) goto activate_locked_split; }
This patch moves the call to count_mthp_stat() in count_swpout_vm_event() and in shrink_folio_list() to be outside the "ifdef CONFIG_TRANSPARENT_HUGEPAGE" based on changes made in commit 246d3aa3e531 ("mm: cleanup count_mthp_stat() definition"). Signed-off-by: Kanchana P Sridhar <kanchana.p.sridhar@intel.com> Reviewed-by: Nhat Pham <nphamcs@gmail.com> --- mm/page_io.c | 2 +- mm/vmscan.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)