Received-SPF: pass (google.com: domain of linux-kernel+bounces-149446-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
Precedence: bulk
MIME-Version: 1.0
References: <20240417160842.76665-1-ryncsn@gmail.com> <20240417160842.76665-8-ryncsn@gmail.com>
 <CAGsJ_4xv8m-Xjih0PmKD1PcUSGVRsti8EH0cbStZOFmX+YhnFA@mail.gmail.com>
In-Reply-To: <CAGsJ_4xv8m-Xjih0PmKD1PcUSGVRsti8EH0cbStZOFmX+YhnFA@mail.gmail.com>
From: Kairui Song <ryncsn@gmail.com>
Date: Thu, 18 Apr 2024 10:42:35 +0800
Message-ID: <CAMgjq7Am+5ftvAW4X2xOhAZ+zotSR8gD8oG+_CV=pJvsqy2Oyw@mail.gmail.com>
Subject: Re: [PATCH 7/8] mm: drop page_index/page_file_offset and convert swap
 helpers to use folio
To: Barry Song <21cnbao@gmail.com>
Cc: linux-mm@kvack.org, Andrew Morton <akpm@linux-foundation.org>, 
	"Huang, Ying" <ying.huang@intel.com>, Matthew Wilcox <willy@infradead.org>, Chris Li <chrisl@kernel.org>, 
	Barry Song <v-songbaohua@oppo.com>, Ryan Roberts <ryan.roberts@arm.com>, Neil Brown <neilb@suse.de>, 
	Minchan Kim <minchan@kernel.org>, Hugh Dickins <hughd@google.com>, 
	David Hildenbrand <david@redhat.com>, Yosry Ahmed <yosryahmed@google.com>, linux-fsdevel@vger.kernel.org, 
	linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Thu, Apr 18, 2024 at 9:55=E2=80=AFAM Barry Song <21cnbao@gmail.com> wrot=
e:
>
> On Thu, Apr 18, 2024 at 4:12=E2=80=AFAM Kairui Song <ryncsn@gmail.com> wr=
ote:
> >
> > From: Kairui Song <kasong@tencent.com>
> >
> > When applied on swap cache pages, page_index / page_file_offset was use=
d
> > to retrieve the swap cache index or swap file offset of a page, and the=
y
> > have their folio equivalence version: folio_index / folio_file_pos.
> >
> > We have eliminated all users for page_index / page_file_offset, everyth=
ing
> > is using folio_index / folio_file_pos now, so remove the old helpers.
> >
> > Then convert the implementation of folio_index / folio_file_pos to
> > to use folio natively.
> >
> > After this commit, all users that might encounter mixed usage of swap
> > cache and page cache will only use following two helpers:
> >
> > folio_index (calls __folio_swap_cache_index)
> > folio_file_pos (calls __folio_swap_file_pos)
> >
> > The offset in swap file and index in swap cache is still basically the
> > same thing at this moment, but will be different in following commits.
> >
> > Signed-off-by: Kairui Song <kasong@tencent.com>
>
> Hi Kairui, thanks !
>
> I also find it rather odd that folio_file_page() is utilized for both
> swp and file.
>
> mm/memory.c <<do_swap_page>>
>              page =3D folio_file_page(folio, swp_offset(entry));
> mm/swap_state.c <<swapin_readahead>>
>              return folio_file_page(folio, swp_offset(entry));
> mm/swapfile.c <<unuse_pte>>
>              page =3D folio_file_page(folio, swp_offset(entry));
>
> Do you believe it's worthwhile to tidy up?
>

Hi Barry,

I'm not sure about this. Using folio_file_page doesn't look too bad,
and it will be gone once we convert them to always use folio, this
shouldn't take too long.