[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAKEwX=PSK-f0mK=Ffsvqs72qicPAoUWW-MdcNurj4PO0NMuJ3w@mail.gmail.com>
Date: Tue, 8 Apr 2025 08:11:21 -0700
From: Nhat Pham <nphamcs@...il.com>
To: Johannes Weiner <hannes@...xchg.org>
Cc: linux-mm@...ck.org, akpm@...ux-foundation.org, hughd@...gle.com,
yosry.ahmed@...ux.dev, 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 8, 2025 at 7:16 AM Johannes Weiner <hannes@...xchg.org> 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
Makes sense, yeah.
I will reorganize the series as follows:
1. Rearrange in the first patch (which I already did for
mm/swapfile.c, but now I'll also rearrange the functions in header
files as well).
2. One more patch to rename the function and add the new type.
3. The rest of the series (new API, new code, etc.).
Powered by blists - more mailing lists