linux-kernel - Re: [PATCH 7/8] mm: drop page_index/page_file_offset and convert swap helpers to use folio

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAGsJ_4zVH+MtB1X4J7X9Gk9c1bg_BNGKbg7viBXDKKKO8TO4EQ@mail.gmail.com>
Date: Thu, 18 Apr 2024 22:19:02 +1200
From: Barry Song <21cnbao@...il.com>
To: Kairui Song <ryncsn@...il.com>
Cc: linux-mm@...ck.org, Andrew Morton <akpm@...ux-foundation.org>, 
	"Huang, Ying" <ying.huang@...el.com>, Matthew Wilcox <willy@...radead.org>, Chris Li <chrisl@...nel.org>, 
	Barry Song <v-songbaohua@...o.com>, Ryan Roberts <ryan.roberts@....com>, Neil Brown <neilb@...e.de>, 
	Minchan Kim <minchan@...nel.org>, Hugh Dickins <hughd@...gle.com>, 
	David Hildenbrand <david@...hat.com>, Yosry Ahmed <yosryahmed@...gle.com>, linux-fsdevel@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 7/8] mm: drop page_index/page_file_offset and convert swap
 helpers to use folio

On Thu, Apr 18, 2024 at 2:42 PM Kairui Song <ryncsn@...il.com> wrote:
>
> On Thu, Apr 18, 2024 at 9:55 AM Barry Song <21cnbao@...il.com> wrote:
> >
> > On Thu, Apr 18, 2024 at 4:12 AM Kairui Song <ryncsn@...il.com> wrote:
> > >
> > > From: Kairui Song <kasong@...cent.com>
> > >
> > > When applied on swap cache pages, page_index / page_file_offset was used
> > > to retrieve the swap cache index or swap file offset of a page, and they
> > > have their folio equivalence version: folio_index / folio_file_pos.
> > >
> > > We have eliminated all users for page_index / page_file_offset, everything
> > > 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@...cent.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 = 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 = 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.

HI Kairui,
I am not quite sure this is going to be quite soon. our swap-in large
folios refault still have corner cases which can't map large folios even
we hit large folios in swapcache [1].
personally, i feel do_swap_page() isn't handling file, and those pages
are anon but not file.  so a separate helper taking folio and entry as
arguments seem more readable as Chris even wants to remove
the assumption large folios have been to swapped to contiguous
swap offsets. anyway, it is just me :-）

[1] https://lore.kernel.org/linux-mm/20240409082631.187483-1-21cnbao@gmail.com/