Received-SPF: pass (google.com: domain of linux-kernel+bounces-147833-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Precedence: bulk
MIME-Version: 1.0
References: <20240409082631.187483-1-21cnbao@gmail.com> <20240409082631.187483-6-21cnbao@gmail.com>
 <87frvk24x8.fsf@yhuang6-desk2.ccr.corp.intel.com> <CAGsJ_4yoz=UH+=Z+ZmRy_9=vu_JWuOXnNO0gFtkBZ-rKJppMPA@mail.gmail.com>
 <87bk6822gh.fsf@yhuang6-desk2.ccr.corp.intel.com>
In-Reply-To: <87bk6822gh.fsf@yhuang6-desk2.ccr.corp.intel.com>
From: Barry Song <21cnbao@gmail.com>
Date: Wed, 17 Apr 2024 13:48:36 +1200
Message-ID: <CAGsJ_4wOtkGhJL-daZfYyPacqypKHjK16_AqC+Y-1KrwXMNmHA@mail.gmail.com>
Subject: Re: [PATCH v2 5/5] mm: add per-order mTHP swpin_refault counter
To: "Huang, Ying" <ying.huang@intel.com>
Cc: akpm@linux-foundation.org, linux-mm@kvack.org, 
	baolin.wang@linux.alibaba.com, chrisl@kernel.org, david@redhat.com, 
	hanchuanhua@oppo.com, hannes@cmpxchg.org, hughd@google.com, 
	kasong@tencent.com, ryan.roberts@arm.com, surenb@google.com, 
	v-songbaohua@oppo.com, willy@infradead.org, xiang@kernel.org, 
	yosryahmed@google.com, yuzhao@google.com, ziy@nvidia.com, 
	linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Wed, Apr 17, 2024 at 1:40=E2=80=AFPM Huang, Ying <ying.huang@intel.com> =
wrote:
>
> Barry Song <21cnbao@gmail.com> writes:
>
> > On Wed, Apr 17, 2024 at 8:47=E2=80=AFAM Huang, Ying <ying.huang@intel.c=
om> wrote:
> >>
> >> Barry Song <21cnbao@gmail.com> writes:
> >>
> >> > From: Barry Song <v-songbaohua@oppo.com>
> >> >
> >> > Currently, we are handling the scenario where we've hit a
> >> > large folio in the swapcache, and the reclaiming process
> >> > for this large folio is still ongoing.
> >> >
> >> > Signed-off-by: Barry Song <v-songbaohua@oppo.com>
> >> > ---
> >> >  include/linux/huge_mm.h | 1 +
> >> >  mm/huge_memory.c        | 2 ++
> >> >  mm/memory.c             | 1 +
> >> >  3 files changed, 4 insertions(+)
> >> >
> >> > diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h
> >> > index c8256af83e33..b67294d5814f 100644
> >> > --- a/include/linux/huge_mm.h
> >> > +++ b/include/linux/huge_mm.h
> >> > @@ -269,6 +269,7 @@ enum mthp_stat_item {
> >> >       MTHP_STAT_ANON_ALLOC_FALLBACK,
> >> >       MTHP_STAT_ANON_SWPOUT,
> >> >       MTHP_STAT_ANON_SWPOUT_FALLBACK,
> >> > +     MTHP_STAT_ANON_SWPIN_REFAULT,
> >>
> >> This is different from the refault concept used in other place in mm
> >> subystem.  Please check the following code
> >>
> >>         if (shadow)
> >>                 workingset_refault(folio, shadow);
> >>
> >> in __read_swap_cache_async().
> >
> > right. it is slightly different as refault can also cover the case foli=
os
> > have been entirely released and a new page fault happens soon
> > after it.
> > Do you have a better name for this?
> > MTHP_STAT_ANON_SWPIN_UNDER_RECLAIM
> > or
> > MTHP_STAT_ANON_SWPIN_RECLAIMING ?
>
> TBH, I don't think we need this counter.  It's important for you during
> implementation.  But I don't think it's important for end users.

Okay. If we can't find a shared interest between the
implementer and user, I'm perfectly fine with keeping it
local only for debugging purposes.

>
> --
> Best Regards,
> Huang, Ying
>
> >>
> >> >       __MTHP_STAT_COUNT
> >> >  };
> >>
> >> --
> >> Best Regards,
> >> Huang, Ying
> >>
> >> > diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> >> > index d8d2ed80b0bf..fb95345b0bde 100644
> >> > --- a/mm/huge_memory.c
> >> > +++ b/mm/huge_memory.c
> >> > @@ -556,12 +556,14 @@ DEFINE_MTHP_STAT_ATTR(anon_alloc, MTHP_STAT_AN=
ON_ALLOC);
> >> >  DEFINE_MTHP_STAT_ATTR(anon_alloc_fallback, MTHP_STAT_ANON_ALLOC_FAL=
LBACK);
> >> >  DEFINE_MTHP_STAT_ATTR(anon_swpout, MTHP_STAT_ANON_SWPOUT);
> >> >  DEFINE_MTHP_STAT_ATTR(anon_swpout_fallback, MTHP_STAT_ANON_SWPOUT_F=
ALLBACK);
> >> > +DEFINE_MTHP_STAT_ATTR(anon_swpin_refault, MTHP_STAT_ANON_SWPIN_REFA=
ULT);
> >> >
> >> >  static struct attribute *stats_attrs[] =3D {
> >> >       &anon_alloc_attr.attr,
> >> >       &anon_alloc_fallback_attr.attr,
> >> >       &anon_swpout_attr.attr,
> >> >       &anon_swpout_fallback_attr.attr,
> >> > +     &anon_swpin_refault_attr.attr,
> >> >       NULL,
> >> >  };
> >> >
> >> > diff --git a/mm/memory.c b/mm/memory.c
> >> > index 9818dc1893c8..acc023795a4d 100644
> >> > --- a/mm/memory.c
> >> > +++ b/mm/memory.c
> >> > @@ -4167,6 +4167,7 @@ vm_fault_t do_swap_page(struct vm_fault *vmf)
> >> >               nr_pages =3D nr;
> >> >               entry =3D folio->swap;
> >> >               page =3D &folio->page;
> >> > +             count_mthp_stat(folio_order(folio), MTHP_STAT_ANON_SWP=
IN_REFAULT);
> >> >       }
> >> >
> >> >  check_pte: