| 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+HLZJRgmvca7K2@MiWiFi-R3L-srv>
Date: Mon, 15 Dec 2025 11:57:33 +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 09/19] mm, swap: swap entry of a bad slot should not
be considered as swapped out
On 12/05/25 at 03:29am, Kairui Song wrote:
> From: Kairui Song <kasong@...cent.com>
>
> When checking if a swap entry is swapped out, we simply check if the
> bitwise result of the count value is larger than 0. But SWAP_MAP_BAD
> will also be considered as a swao count value larger than 0.
>
> SWAP_MAP_BAD being considered as a count value larger than 0 is useful
> for the swap allocator: they will be seen as a used slot, so the
> allocator will skip them. But for the swapped out check, this
> isn't correct.
>
> There is currently no observable issue. The swapped out check is only
> useful for readahead and folio swapped-out status check. For readahead,
> the swap cache layer will abort upon checking and updating the swap map.
> For the folio swapped out status check, the swap allocator will never
> allocate an entry of bad slots to folio, so that part is fine too. The
> worst that could happen now is redundant allocation/freeing of folios
> and waste CPU time.
>
> This also makes it easier to get rid of swap map checking and update
> during folio insertion in the swap cache layer.
Will swap_entry_swapped() be called in other places in phase 3 of swap
table?
I checked the code of the current phase 2, in all three places you
convert it with swp_offset(entry), is it necessary? Why don't you keep
the swap_entry_swapped(si, entry)? Surely, this is trivial.
mm/swap_state.c <<swap_cache_alloc_folio>>
if (!swap_entry_swapped(si, swp_offset(entry)))
mm/swapfile.c <<folio_swapped>>
return swap_entry_swapped(si, swp_offset(entry));
mm/swapfile.c <<swap_free_hibernation_slot>>
WARN_ON(swap_entry_swapped(si, offset));
>
> Signed-off-by: Kairui Song <kasong@...cent.com>
> ---
> include/linux/swap.h | 6 ++++--
> mm/swap_state.c | 4 ++--
> mm/swapfile.c | 22 +++++++++++-----------
> 3 files changed, 17 insertions(+), 15 deletions(-)
>
> diff --git a/include/linux/swap.h b/include/linux/swap.h
> index bf72b548a96d..936fa8f9e5f3 100644
> --- a/include/linux/swap.h
> +++ b/include/linux/swap.h
> @@ -466,7 +466,8 @@ 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 bool swap_entry_swapped(struct swap_info_struct *si, swp_entry_t entry);
> +extern bool swap_entry_swapped(struct swap_info_struct *si,
> + unsigned long offset);
> extern int swp_swapcount(swp_entry_t entry);
> struct backing_dev_info;
> extern struct swap_info_struct *get_swap_device(swp_entry_t entry);
> @@ -535,7 +536,8 @@ static inline int __swap_count(swp_entry_t entry)
> return 0;
> }
>
> -static inline bool swap_entry_swapped(struct swap_info_struct *si, swp_entry_t entry)
> +static inline bool swap_entry_swapped(struct swap_info_struct *si,
> + unsigned long offset)
> {
> return false;
> }
> diff --git a/mm/swap_state.c b/mm/swap_state.c
> index 8c429dc33ca9..0c5aad537716 100644
> --- a/mm/swap_state.c
> +++ b/mm/swap_state.c
> @@ -527,8 +527,8 @@ struct folio *swap_cache_alloc_folio(swp_entry_t entry, gfp_t gfp_mask,
> if (folio)
> return folio;
>
> - /* Skip allocation for unused swap slot for readahead path. */
> - if (!swap_entry_swapped(si, entry))
> + /* Skip allocation for unused and bad swap slot for readahead. */
> + if (!swap_entry_swapped(si, swp_offset(entry)))
> return NULL;
>
> /* Allocate a new folio to be added into the swap cache. */
> diff --git a/mm/swapfile.c b/mm/swapfile.c
> index e23287c06f1c..5a766d4fcaa5 100644
> --- a/mm/swapfile.c
> +++ b/mm/swapfile.c
> @@ -1766,21 +1766,21 @@ int __swap_count(swp_entry_t entry)
> return swap_count(si->swap_map[offset]);
> }
>
> -/*
> - * How many references to @entry are currently swapped out?
> - * This does not give an exact answer when swap count is continued,
> - * but does include the high COUNT_CONTINUED flag to allow for that.
> +/**
> + * swap_entry_swapped - Check if the swap entry at @offset is swapped.
> + * @si: the swap device.
> + * @offset: offset of the swap entry.
> */
> -bool swap_entry_swapped(struct swap_info_struct *si, swp_entry_t entry)
> +bool swap_entry_swapped(struct swap_info_struct *si, unsigned long offset)
> {
> - pgoff_t offset = swp_offset(entry);
> struct swap_cluster_info *ci;
> int count;
>
> ci = swap_cluster_lock(si, offset);
> count = swap_count(si->swap_map[offset]);
> swap_cluster_unlock(ci);
> - return !!count;
> +
> + return count && count != SWAP_MAP_BAD;
> }
>
> /*
> @@ -1866,7 +1866,7 @@ static bool folio_swapped(struct folio *folio)
> return false;
>
> if (!IS_ENABLED(CONFIG_THP_SWAP) || likely(!folio_test_large(folio)))
> - return swap_entry_swapped(si, entry);
> + return swap_entry_swapped(si, swp_offset(entry));
>
> return swap_page_trans_huge_swapped(si, entry, folio_order(folio));
> }
> @@ -3677,10 +3677,10 @@ static int __swap_duplicate(swp_entry_t entry, unsigned char usage, int nr)
> count = si->swap_map[offset + i];
>
> /*
> - * swapin_readahead() doesn't check if a swap entry is valid, so the
> - * swap entry could be SWAP_MAP_BAD. Check here with lock held.
> + * Allocator never allocates bad slots, and readahead is guarded
> + * by swap_entry_swapped.
> */
> - if (unlikely(swap_count(count) == SWAP_MAP_BAD)) {
> + if (WARN_ON(swap_count(count) == SWAP_MAP_BAD)) {
> err = -ENOENT;
> goto unlock_out;
> }
>
> --
> 2.52.0
>
Powered by blists - more mailing lists