| 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
| ||
|
Message-ID: <aT+KvZplFPimgStY@MiWiFi-R3L-srv>
Date: Mon, 15 Dec 2025 12:12:45 +0800
From: Baoquan He <bhe@...hat.com>
To: Kairui Song <ryncsn@...il.com>
Cc: linux-mm@...ck.org, Andrew Morton <akpm@...ux-foundation.org>,
Barry Song <baohua@...nel.org>, Chris Li <chrisl@...nel.org>,
Nhat Pham <nphamcs@...il.com>, Yosry Ahmed <yosry.ahmed@...ux.dev>,
David Hildenbrand <david@...nel.org>,
Johannes Weiner <hannes@...xchg.org>,
Youngjun Park <youngjun.park@....com>,
Hugh Dickins <hughd@...gle.com>,
Baolin Wang <baolin.wang@...ux.alibaba.com>,
Ying Huang <ying.huang@...ux.alibaba.com>,
Kemeng Shi <shikemeng@...weicloud.com>,
Lorenzo Stoakes <lorenzo.stoakes@...cle.com>,
"Matthew Wilcox (Oracle)" <willy@...radead.org>,
linux-kernel@...r.kernel.org, Kairui Song <kasong@...cent.com>
Subject: Re: [PATCH v4 10/19] mm, swap: consolidate cluster reclaim and
usability check
On 12/05/25 at 03:29am, Kairui Song wrote:
> From: Kairui Song <kasong@...cent.com>
>
> Swap cluster cache reclaim requires releasing the lock, so the cluster
> may become unusable after the reclaim. To prepare for checking swap
> cache using the swap table directly, consolidate the swap cluster
> reclaim and the check logic.
>
> We will want to avoid touching the cluster's data completely with the
~~~~~~~~
'want to' means 'will'?
> swap table, to avoid RCU overhead here. And by moving the cluster usable
> check into the reclaim helper, it will also help avoid a redundant scan of
> the slots if the cluster is no longer usable, and we will want to avoid
~~~~~~~~~~~~
this place too.
> touching the cluster.
>
> Also, adjust it very slightly while at it: always scan the whole region
> during reclaim, don't skip slots covered by a reclaimed folio. Because
> the reclaim is lockless, it's possible that new cache lands at any time.
> And for allocation, we want all caches to be reclaimed to avoid
> fragmentation. Besides, if the scan offset is not aligned with the size
> of the reclaimed folio, we might skip some existing cache and fail the
> reclaim unexpectedly.
>
> There should be no observable behavior change. It might slightly improve
> the fragmentation issue or performance.
>
> Signed-off-by: Kairui Song <kasong@...cent.com>
> ---
> mm/swapfile.c | 45 +++++++++++++++++++++++++++++----------------
> 1 file changed, 29 insertions(+), 16 deletions(-)
>
> diff --git a/mm/swapfile.c b/mm/swapfile.c
> index 5a766d4fcaa5..2703dfafc632 100644
> --- a/mm/swapfile.c
> +++ b/mm/swapfile.c
> @@ -777,33 +777,51 @@ static int swap_cluster_setup_bad_slot(struct swap_cluster_info *cluster_info,
> return 0;
> }
>
> +/*
> + * Reclaim drops the ci lock, so the cluster may become unusable (freed or
> + * stolen by a lower order). @usable will be set to false if that happens.
> + */
> static bool cluster_reclaim_range(struct swap_info_struct *si,
> struct swap_cluster_info *ci,
> - unsigned long start, unsigned long end)
> + unsigned long start, unsigned int order,
> + bool *usable)
> {
> + unsigned int nr_pages = 1 << order;
> + unsigned long offset = start, end = start + nr_pages;
> unsigned char *map = si->swap_map;
> - unsigned long offset = start;
> int nr_reclaim;
>
> spin_unlock(&ci->lock);
> do {
> switch (READ_ONCE(map[offset])) {
> case 0:
> - offset++;
> break;
> case SWAP_HAS_CACHE:
> nr_reclaim = __try_to_reclaim_swap(si, offset, TTRS_ANYWAY);
> - if (nr_reclaim > 0)
> - offset += nr_reclaim;
> - else
> + if (nr_reclaim < 0)
> goto out;
> break;
> default:
> goto out;
> }
> - } while (offset < end);
> + } while (++offset < end);
~~~~~ '++offset' is conflicting with nr_reclaim
returned from __try_to_reclaim_swap(). can you explain?
> out:
> spin_lock(&ci->lock);
> +
> + /*
> + * We just dropped ci->lock so cluster could be used by another
> + * order or got freed, check if it's still usable or empty.
> + */
> + if (!cluster_is_usable(ci, order)) {
> + *usable = false;
> + return false;
> + }
> + *usable = true;
> +
> + /* Fast path, no need to scan if the whole cluster is empty */
> + if (cluster_is_empty(ci))
> + return true;
> +
> /*
> * Recheck the range no matter reclaim succeeded or not, the slot
> * could have been be freed while we are not holding the lock.
> @@ -900,9 +918,10 @@ static unsigned int alloc_swap_scan_cluster(struct swap_info_struct *si,
> unsigned long start = ALIGN_DOWN(offset, SWAPFILE_CLUSTER);
> unsigned long end = min(start + SWAPFILE_CLUSTER, si->max);
> unsigned int nr_pages = 1 << order;
> - bool need_reclaim, ret;
> + bool need_reclaim, ret, usable;
>
> lockdep_assert_held(&ci->lock);
> + VM_WARN_ON(!cluster_is_usable(ci, order));
>
> if (end < nr_pages || ci->count + nr_pages > SWAPFILE_CLUSTER)
> goto out;
> @@ -912,14 +931,8 @@ static unsigned int alloc_swap_scan_cluster(struct swap_info_struct *si,
> if (!cluster_scan_range(si, ci, offset, nr_pages, &need_reclaim))
> continue;
> if (need_reclaim) {
> - ret = cluster_reclaim_range(si, ci, offset, offset + nr_pages);
> - /*
> - * Reclaim drops ci->lock and cluster could be used
> - * by another order. Not checking flag as off-list
> - * cluster has no flag set, and change of list
> - * won't cause fragmentation.
> - */
> - if (!cluster_is_usable(ci, order))
> + ret = cluster_reclaim_range(si, ci, offset, order, &usable);
> + if (!usable)
> goto out;
> if (cluster_is_empty(ci))
> offset = start;
>
> --
> 2.52.0
>
Powered by blists - more mailing lists