[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAF8kJuNLUE87G5=MK-dpR4tAXoEO8jQYE0A+a7Rtk+f3yBcvxw@mail.gmail.com>
Date: Sat, 6 Sep 2025 08:30:30 -0700
From: Chris Li <chrisl@...nel.org>
To: Kairui Song <kasong@...cent.com>
Cc: linux-mm@...ck.org, Andrew Morton <akpm@...ux-foundation.org>,
Matthew Wilcox <willy@...radead.org>, Hugh Dickins <hughd@...gle.com>, Barry Song <baohua@...nel.org>,
Baoquan He <bhe@...hat.com>, Nhat Pham <nphamcs@...il.com>,
Kemeng Shi <shikemeng@...weicloud.com>, Baolin Wang <baolin.wang@...ux.alibaba.com>,
Ying Huang <ying.huang@...ux.alibaba.com>, Johannes Weiner <hannes@...xchg.org>,
David Hildenbrand <david@...hat.com>, Yosry Ahmed <yosryahmed@...gle.com>,
Lorenzo Stoakes <lorenzo.stoakes@...cle.com>, Zi Yan <ziy@...dia.com>,
linux-kernel@...r.kernel.org, kernel test robot <oliver.sang@...el.com>
Subject: Re: [PATCH v2 13/15] mm, swap: remove contention workaround for swap cache
Acked-by: Chris Li <chrisl@...nel.org>
Chris
On Fri, Sep 5, 2025 at 12:15 PM Kairui Song <ryncsn@...il.com> wrote:
>
> From: Kairui Song <kasong@...cent.com>
>
> Swap cluster setup will try to shuffle the clusters on initialization.
> It was helpful to avoid contention for the swap cache space. The cluster
> size (2M) was much smaller than each swap cache space (64M), so
> shuffling the cluster means the allocator will try to allocate swap
> slots that are in different swap cache spaces for each CPU, reducing the
> chance of two CPUs using the same swap cache space, and hence reducing
> the contention.
>
> Now, swap cache is managed by swap clusters, this shuffle is pointless.
> Just remove it, and clean up related macros.
>
> This also improves the HDD swap performance as shuffling IO is a bad
> idea for HDD, and now the shuffling is gone. Test have shown a ~40%
> performance gain for HDD [1]:
>
> Doing sequential swap in of 8G data using 8 processes with usemem,
> average of 3 test runs:
>
> Before: 1270.91 KB/s per process
> After: 1849.54 KB/s per process
>
> Link: https://lore.kernel.org/linux-mm/CAMgjq7AdauQ8=X0zeih2r21QoV=-WWj1hyBxLWRzq74n-C=-Ng@mail.gmail.com/ [1]
> Reported-by: kernel test robot <oliver.sang@...el.com>
> Closes: https://lore.kernel.org/oe-lkp/202504241621.f27743ec-lkp@intel.com
> Signed-off-by: Kairui Song <kasong@...cent.com>
> Acked-by: Chris Li <chrisl@...nel.org>
> Reviewed-by: Barry Song <baohua@...nel.org>
> ---
> mm/swap.h | 4 ----
> mm/swapfile.c | 32 ++++++++------------------------
> mm/zswap.c | 7 +++++--
> 3 files changed, 13 insertions(+), 30 deletions(-)
>
> diff --git a/mm/swap.h b/mm/swap.h
> index e48431a26f89..c4fb28845d77 100644
> --- a/mm/swap.h
> +++ b/mm/swap.h
> @@ -197,10 +197,6 @@ int swap_writeout(struct folio *folio, struct swap_iocb **swap_plug);
> void __swap_writepage(struct folio *folio, struct swap_iocb **swap_plug);
>
> /* linux/mm/swap_state.c */
> -/* One swap address space for each 64M swap space */
> -#define SWAP_ADDRESS_SPACE_SHIFT 14
> -#define SWAP_ADDRESS_SPACE_PAGES (1 << SWAP_ADDRESS_SPACE_SHIFT)
> -#define SWAP_ADDRESS_SPACE_MASK (SWAP_ADDRESS_SPACE_PAGES - 1)
> extern struct address_space swap_space __ro_after_init;
> static inline struct address_space *swap_address_space(swp_entry_t entry)
> {
> diff --git a/mm/swapfile.c b/mm/swapfile.c
> index cbb7d4c0773d..6b3b35a7ddd9 100644
> --- a/mm/swapfile.c
> +++ b/mm/swapfile.c
> @@ -3202,21 +3202,14 @@ static int setup_swap_map(struct swap_info_struct *si,
> return 0;
> }
>
> -#define SWAP_CLUSTER_INFO_COLS \
> - DIV_ROUND_UP(L1_CACHE_BYTES, sizeof(struct swap_cluster_info))
> -#define SWAP_CLUSTER_SPACE_COLS \
> - DIV_ROUND_UP(SWAP_ADDRESS_SPACE_PAGES, SWAPFILE_CLUSTER)
> -#define SWAP_CLUSTER_COLS \
> - max_t(unsigned int, SWAP_CLUSTER_INFO_COLS, SWAP_CLUSTER_SPACE_COLS)
> -
> static struct swap_cluster_info *setup_clusters(struct swap_info_struct *si,
> union swap_header *swap_header,
> unsigned long maxpages)
> {
> unsigned long nr_clusters = DIV_ROUND_UP(maxpages, SWAPFILE_CLUSTER);
> struct swap_cluster_info *cluster_info;
> - unsigned long i, j, idx;
> int err = -ENOMEM;
> + unsigned long i;
>
> cluster_info = kvcalloc(nr_clusters, sizeof(*cluster_info), GFP_KERNEL);
> if (!cluster_info)
> @@ -3265,22 +3258,13 @@ static struct swap_cluster_info *setup_clusters(struct swap_info_struct *si,
> INIT_LIST_HEAD(&si->frag_clusters[i]);
> }
>
> - /*
> - * Reduce false cache line sharing between cluster_info and
> - * sharing same address space.
> - */
> - for (j = 0; j < SWAP_CLUSTER_COLS; j++) {
> - for (i = 0; i < DIV_ROUND_UP(nr_clusters, SWAP_CLUSTER_COLS); i++) {
> - struct swap_cluster_info *ci;
> - idx = i * SWAP_CLUSTER_COLS + j;
> - ci = cluster_info + idx;
> - if (idx >= nr_clusters)
> - continue;
> - if (ci->count) {
> - ci->flags = CLUSTER_FLAG_NONFULL;
> - list_add_tail(&ci->list, &si->nonfull_clusters[0]);
> - continue;
> - }
> + for (i = 0; i < nr_clusters; i++) {
> + struct swap_cluster_info *ci = &cluster_info[i];
> +
> + if (ci->count) {
> + ci->flags = CLUSTER_FLAG_NONFULL;
> + list_add_tail(&ci->list, &si->nonfull_clusters[0]);
> + } else {
> ci->flags = CLUSTER_FLAG_FREE;
> list_add_tail(&ci->list, &si->free_clusters);
> }
> diff --git a/mm/zswap.c b/mm/zswap.c
> index 3dda4310099e..cba7077fda40 100644
> --- a/mm/zswap.c
> +++ b/mm/zswap.c
> @@ -225,10 +225,13 @@ static bool zswap_has_pool;
> * helpers and fwd declarations
> **********************************/
>
> +/* One swap address space for each 64M swap space */
> +#define ZSWAP_ADDRESS_SPACE_SHIFT 14
> +#define ZSWAP_ADDRESS_SPACE_PAGES (1 << ZSWAP_ADDRESS_SPACE_SHIFT)
> static inline struct xarray *swap_zswap_tree(swp_entry_t swp)
> {
> return &zswap_trees[swp_type(swp)][swp_offset(swp)
> - >> SWAP_ADDRESS_SPACE_SHIFT];
> + >> ZSWAP_ADDRESS_SPACE_SHIFT];
> }
>
> #define zswap_pool_debug(msg, p) \
> @@ -1674,7 +1677,7 @@ int zswap_swapon(int type, unsigned long nr_pages)
> struct xarray *trees, *tree;
> unsigned int nr, i;
>
> - nr = DIV_ROUND_UP(nr_pages, SWAP_ADDRESS_SPACE_PAGES);
> + nr = DIV_ROUND_UP(nr_pages, ZSWAP_ADDRESS_SPACE_PAGES);
> trees = kvcalloc(nr, sizeof(*tree), GFP_KERNEL);
> if (!trees) {
> pr_err("alloc failed, zswap disabled for swap type %d\n", type);
> --
> 2.51.0
>
>
Powered by blists - more mailing lists