Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:2 as permitted sender) client-ip=2620:137:e000::3:2;
MIME-Version: 1.0
References: <20231119194740.94101-1-ryncsn@gmail.com> <20231119194740.94101-4-ryncsn@gmail.com>
 <CAF8kJuP3OyJ3e2ttiDrqJ4Ucs8BbysaDtuUY3STaJUC8oVkZkQ@mail.gmail.com>
In-Reply-To: <CAF8kJuP3OyJ3e2ttiDrqJ4Ucs8BbysaDtuUY3STaJUC8oVkZkQ@mail.gmail.com>
From:   Kairui Song <ryncsn@gmail.com>
Date:   Thu, 23 Nov 2023 01:33:17 +0800
Message-ID: <CAMgjq7COk3TXrHJmpWbu-DY8ZZWcRehot+rD87kG0_Z6Z3XuSg@mail.gmail.com>
Subject: Re: [PATCH 03/24] mm/swap: move no readahead swapin code to a stand
 alone helper
To:     Chris Li <chrisl@kernel.org>
Cc:     linux-mm@kvack.org, Andrew Morton <akpm@linux-foundation.org>,
        "Huang, Ying" <ying.huang@intel.com>,
        David Hildenbrand <david@redhat.com>,
        Hugh Dickins <hughd@google.com>,
        Johannes Weiner <hannes@cmpxchg.org>,
        Matthew Wilcox <willy@infradead.org>,
        Michal Hocko <mhocko@suse.com>, linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

Chris Li <chrisl@kernel.org> =E4=BA=8E2023=E5=B9=B411=E6=9C=8821=E6=97=A5=
=E5=91=A8=E4=BA=8C 13:35=E5=86=99=E9=81=93=EF=BC=9A
>
> On Sun, Nov 19, 2023 at 11:48=E2=80=AFAM Kairui Song <ryncsn@gmail.com> w=
rote:
> >
> > From: Kairui Song <kasong@tencent.com>
> >
> > No feature change, simply move the routine to a standalone function to
> > be used later. The error path handling is copied from the "out_page"
> > label, to make the code change minimized for easier reviewing.
> >
> > Signed-off-by: Kairui Song <kasong@tencent.com>
> > ---
> >  mm/memory.c     | 33 +++++----------------------------
> >  mm/swap.h       |  2 ++
> >  mm/swap_state.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 55 insertions(+), 28 deletions(-)
> >
> > diff --git a/mm/memory.c b/mm/memory.c
> > index 70ffa867b1be..fba4a5229163 100644
> > --- a/mm/memory.c
> > +++ b/mm/memory.c
> > @@ -3794,7 +3794,6 @@ vm_fault_t do_swap_page(struct vm_fault *vmf)
> >         swp_entry_t entry;
> >         pte_t pte;
> >         vm_fault_t ret =3D 0;
> > -       void *shadow =3D NULL;
> >
> >         if (!pte_unmap_same(vmf))
> >                 goto out;
> > @@ -3858,33 +3857,11 @@ vm_fault_t do_swap_page(struct vm_fault *vmf)
> >         if (!folio) {
> >                 if (data_race(si->flags & SWP_SYNCHRONOUS_IO) &&
> >                     __swap_count(entry) =3D=3D 1) {
> > -                       /* skip swapcache */
> > -                       folio =3D vma_alloc_folio(GFP_HIGHUSER_MOVABLE,=
 0,
> > -                                               vma, vmf->address, fals=
e);
> > -                       if (folio) {
> > -                               __folio_set_locked(folio);
> > -                               __folio_set_swapbacked(folio);
> > -
> > -                               if (mem_cgroup_swapin_charge_folio(foli=
o,
> > -                                                       vma->vm_mm, GFP=
_KERNEL,
> > -                                                       entry)) {
> > -                                       ret =3D VM_FAULT_OOM;
> > -                                       goto out_page;
> > -                               }
> > -                               mem_cgroup_swapin_uncharge_swap(entry);
> > -
> > -                               shadow =3D get_shadow_from_swap_cache(e=
ntry);
> > -                               if (shadow)
> > -                                       workingset_refault(folio, shado=
w);
> > -
> > -                               folio_add_lru(folio);
> > -                               page =3D &folio->page;
> > -
> > -                               /* To provide entry to swap_readpage() =
*/
> > -                               folio->swap =3D entry;
> > -                               swap_readpage(page, true, NULL);
> > -                               folio->private =3D NULL;
> > -                       }
> > +                       /* skip swapcache and readahead */
> > +                       page =3D swapin_no_readahead(entry, GFP_HIGHUSE=
R_MOVABLE,
> > +                                               vmf);
>
> A minor point,  swapin_no_readahead() is expressed in negative words.
>
> Better use positive words to express what the function does rather
> than what the function does not do.
> I am terrible at naming functions myself. I can think of something
> along the lines of:
> swapin_direct (no cache).
> swapin_minimal?
> swapin_entry_only?
>
> Please suggest better names for basic, bare minimum.

Thanks for the suggestions, I prefer swapin_direct here, will update
with this name and also make it return a folio directly.