linux-kernel - Re: [RFC PATCH 03/14] mm: swap: add a separate type for physical swap slots

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-next>] [day] [month] [year] [list]

Message-ID: <CAKEwX=Mjx4LYe60ErJasFofkq-uH_R9R0TZD9ROdN1vn4V=Yjw@mail.gmail.com>
Date: Tue, 22 Apr 2025 08:50:46 -0700
From: Nhat Pham <nphamcs@...il.com>
To: Yosry Ahmed <yosry.ahmed@...ux.dev>
Cc: Johannes Weiner <hannes@...xchg.org>, linux-mm@...ck.org, akpm@...ux-foundation.org, 
	hughd@...gle.com, mhocko@...nel.org, roman.gushchin@...ux.dev, 
	shakeel.butt@...ux.dev, muchun.song@...ux.dev, len.brown@...el.com, 
	chengming.zhou@...ux.dev, kasong@...cent.com, chrisl@...nel.org, 
	huang.ying.caritas@...il.com, ryan.roberts@....com, viro@...iv.linux.org.uk, 
	baohua@...nel.org, osalvador@...e.de, lorenzo.stoakes@...cle.com, 
	christophe.leroy@...roup.eu, pavel@...nel.org, kernel-team@...a.com, 
	linux-kernel@...r.kernel.org, cgroups@...r.kernel.org, 
	linux-pm@...r.kernel.org
Subject: Re: [RFC PATCH 03/14] mm: swap: add a separate type for physical swap slots

On Tue, Apr 22, 2025 at 7:43 AM Yosry Ahmed <yosry.ahmed@...ux.dev> wrote:
>
> On Tue, Apr 08, 2025 at 10:15:55AM -0400, Johannes Weiner wrote:
> > On Mon, Apr 07, 2025 at 04:42:04PM -0700, Nhat Pham wrote:
> > > In preparation for swap virtualization, add a new type to represent the
> > > physical swap slots of swapfile. This allows us to separates:
> > >
> > > 1. The logical view of the swap entry (i.e what is stored in page table
> > >    entries and used to index into the swap cache), represented by the
> > >    old swp_entry_t type.
> > >
> > > from:
> > >
> > > 2. Its physical backing state (i.e the actual backing slot on the swap
> > >    device), represented by the new swp_slot_t type.
> > >
> > > The functions that operate at the physical level (i.e on the swp_slot_t
> > > types) are also renamed where appropriate (prefixed with swp_slot_* for
> > > e.g). We also take this opportunity to re-arrange the header files
> > > (include/linux/swap.h and swapops.h), grouping the swap API into the
> > > following categories:
> > >
> > > 1. Virtual swap API (i.e functions on swp_entry_t type).
> > >
> > > 2. Swap cache API (mm/swap_state.c)
> > >
> > > 3. Swap slot cache API (mm/swap_slots.c)
> > >
> > > 4. Physical swap slots and device API (mm/swapfile.c).
> >
> > This all makes sense.
> >
> > However,
> >
> > > @@ -483,50 +503,37 @@ static inline long get_nr_swap_pages(void)
> > >     return atomic_long_read(&nr_swap_pages);
> > >  }
> > >
> > > -extern void si_swapinfo(struct sysinfo *);
> > > -swp_entry_t folio_alloc_swap(struct folio *folio);
> > > -bool folio_free_swap(struct folio *folio);
> > > -void put_swap_folio(struct folio *folio, swp_entry_t entry);
> > > -extern swp_entry_t get_swap_page_of_type(int);
> > > -extern int get_swap_pages(int n, swp_entry_t swp_entries[], int order);
> > > -extern int add_swap_count_continuation(swp_entry_t, gfp_t);
> > > -extern void swap_shmem_alloc(swp_entry_t, int);
> > > -extern int swap_duplicate(swp_entry_t);
> > > -extern int swapcache_prepare(swp_entry_t entry, int nr);
> > > -extern void swap_free_nr(swp_entry_t entry, int nr_pages);
> > > -extern void swapcache_free_entries(swp_entry_t *entries, int n);
> > > -extern void free_swap_and_cache_nr(swp_entry_t entry, int nr);
> > > +void si_swapinfo(struct sysinfo *);
> > > +swp_slot_t swap_slot_alloc_of_type(int);
> > > +int swap_slot_alloc(int n, swp_slot_t swp_slots[], int order);
> > > +void swap_slot_free_nr(swp_slot_t slot, int nr_pages);
> > > +void swap_slot_cache_free_slots(swp_slot_t *slots, int n);
> > >  int swap_type_of(dev_t device, sector_t offset);
> > > +sector_t swapdev_block(int, pgoff_t);
> > >  int find_first_swap(dev_t *device);
> > > -extern unsigned int count_swap_pages(int, int);
> > > -extern sector_t swapdev_block(int, pgoff_t);
> > > -extern int __swap_count(swp_entry_t entry);
> > > -extern int swap_swapcount(struct swap_info_struct *si, swp_entry_t entry);
> > > -extern int swp_swapcount(swp_entry_t entry);
> > > -struct swap_info_struct *swp_swap_info(swp_entry_t entry);
> > > +unsigned int count_swap_pages(int, int);
> > > +struct swap_info_struct *swap_slot_swap_info(swp_slot_t slot);
> > >  struct backing_dev_info;
> > > -extern int init_swap_address_space(unsigned int type, unsigned long nr_pages);
> > > -extern void exit_swap_address_space(unsigned int type);
> > > -extern struct swap_info_struct *get_swap_device(swp_entry_t entry);
> > > +struct swap_info_struct *swap_slot_tryget_swap_info(swp_slot_t slot);
> > >  sector_t swap_folio_sector(struct folio *folio);
> >
> > this is difficult to review.
> >
> > Can you please split out:
> >
> > 1. Code moves / cut-and-paste
> >
> > 2. Renames
> >
> > 3. New code
> >
> > into three separate steps
>
> +1, I agree with the fundamental change (and is something that I
> attempted before), but it's really difficult to parse :)
>
> Also, weren't the swap slots scheduled for removal or is my brain making
> stuff up again?

You mean the swap slot cache? That's the one Kairui wants to remove (I
think he removed a huge chunk of it already).

This "swap slot" is basically just a new type I introduced to separate
the physical and virtual swap types.

>