Received-SPF: pass (google.com: domain of linux-kernel+bounces-26298-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Precedence: bulk
MIME-Version: 1.0
References: <20240102175338.62012-1-ryncsn@gmail.com> <20240102175338.62012-7-ryncsn@gmail.com>
 <87a5pg9qno.fsf@yhuang6-desk2.ccr.corp.intel.com> <CAMgjq7Ce6sCSTRd==N3ihrAcvVd2ggszdZuTwxDbFYcBWzcE_g@mail.gmail.com>
 <87wmsb1ia8.fsf@yhuang6-desk2.ccr.corp.intel.com>
In-Reply-To: <87wmsb1ia8.fsf@yhuang6-desk2.ccr.corp.intel.com>
From: Kairui Song <ryncsn@gmail.com>
Date: Tue, 16 Jan 2024 01:11:20 +0800
Message-ID: <CAMgjq7BERdjq5JESbvgNXteC1JCnSR2faM9OLY=1TaGz-PZxrA@mail.gmail.com>
Subject: Re: [PATCH v2 6/9] mm/swap: handle swapcache lookup in swapin_entry
To: "Huang, Ying" <ying.huang@intel.com>
Cc: linux-mm@kvack.org, Andrew Morton <akpm@linux-foundation.org>, 
	Chris Li <chrisl@kernel.org>, Hugh Dickins <hughd@google.com>, 
	Johannes Weiner <hannes@cmpxchg.org>, Matthew Wilcox <willy@infradead.org>, Michal Hocko <mhocko@suse.com>, 
	Yosry Ahmed <yosryahmed@google.com>, David Hildenbrand <david@redhat.com>, linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

Huang, Ying <ying.huang@intel.com> =E4=BA=8E2024=E5=B9=B41=E6=9C=8815=E6=97=
=A5=E5=91=A8=E4=B8=80 09:47=E5=86=99=E9=81=93=EF=BC=9A
>
> Kairui Song <ryncsn@gmail.com> writes:
>
> > Huang, Ying <ying.huang@intel.com> =E4=BA=8E2024=E5=B9=B41=E6=9C=888=E6=
=97=A5=E5=91=A8=E4=B8=80 16:28=E5=86=99=E9=81=93=EF=BC=9A
> >>
> >> Kairui Song <ryncsn@gmail.com> writes:
> >>
> >> > From: Kairui Song <kasong@tencent.com>
> >> >
> >> > Since all callers of swapin_entry need to check the swap cache first=
, we
> >> > can merge this common routine into swapin_entry, so it can be shared=
 and
> >> > optimized later.
> >> >
> >> > Also introduce a enum to better represent possible swap cache usage,=
 and
> >> > add some comments about it, make the usage of swap cache easier to
> >> > understand.
> >>
> >> I don't find any benefit to do this.  The code line number isn't
> >> reduced.  The concept of swap cache isn't hided either.
> >
> > Hi Ying
> >
> > Thanks for the comments.
> >
> > Maybe I should squash this with the following commit? The following
> > commit want to do cache lookup in swapin_entry to avoid a redundant
> > shadow lookup, it can help to improve the performance by about 4% for
> > swapin path.
>
> It's good to improve performance.  But I don't think we must put
> swap_cache_get_folio() in swapin_entry() to do that.  We can just get
> "shadow" from swap_cache_get_folio() and pass it to swapin_entry().

Good idea. My only concern is, if the argument list is getting too
long (7 args if we added all mpol, ilx, shadow here), will that cause
issue for readability or performance?