All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kairui Song <ryncsn@gmail.com>
To: Youngjun Park <youngjun.park@lge.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	 Chris Li <chrisl@kernel.org>, Kairui Song <kasong@tencent.com>,
	 Kemeng Shi <shikemeng@huaweicloud.com>,
	Nhat Pham <nphamcs@gmail.com>, Baoquan He <baoquan.he@linux.dev>,
	 Barry Song <baohua@kernel.org>,
	Jianyue Wu <wujianyue000@gmail.com>,
	her0gyugyu@gmail.com,  linux-mm@kvack.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3 3/4] mm, swap: give hibernation swap slots their own swap table entry type
Date: Wed, 12 Aug 2026 00:48:34 +0800	[thread overview]
Message-ID: <ans-nAQ7dSyfPczI@KASONG-MC4> (raw)
In-Reply-To: <20260811132209.2862708-4-youngjun.park@lge.com>

On Tue, Aug 11, 2026 at 10:22:08PM +0800, Youngjun Park wrote:
> swap_alloc_hibernation_slot() stores a fake shadow in the slot it hands
> out.  An anon slot swapped out with no workingset shadow looks exactly the
> same, so nothing in mm can tell the two apart.
> 
> Give hibernation slots their own type.  Bit 4 and every bit above it are
> set, except the count field, which stays 0.  Bits 0 to 3 are taken by the
> shadow, PFN, pointer and bad marks, so bit 4 is the first free one.  The
> type holds no data, so the value alone says what it is.
> 
> The entry has no swap count.  Hibernation only allocates and frees a slot,
> so a count would never change.  swap_free_hibernation_slot() frees the slot
> directly, there is no count to put first.
> 
> The slot is no longer a shadow, so the previous patch keeps it out of the
> swap cache.  The count field stays 0 as well, so code that reads the count
> without checking the type sees an unused slot instead of one at
> SWP_TB_COUNT_MAX, and a wrong put is caught by the existing underflow
> check.
> 
> Suggested-by: Kairui Song <kasong@tencent.com>
> Link: https://lore.kernel.org/linux-mm/abp7aDgYLrxF3Me8@KASONG-MC4/
> Signed-off-by: Youngjun Park <youngjun.park@lge.com>
> ---
>  mm/swap_table.h | 13 +++++++++++++
>  mm/swapfile.c   | 13 +++++++------
>  2 files changed, 20 insertions(+), 6 deletions(-)
> 
> diff --git a/mm/swap_table.h b/mm/swap_table.h
> index e6613e62f8d0..b916a6493521 100644
> --- a/mm/swap_table.h
> +++ b/mm/swap_table.h
> @@ -30,6 +30,7 @@ struct swap_memcg_table {
>   * PFN:      |SWAP_COUNT|Z|------ PFN -------|10| - Cached slot
>   * Pointer:  |----------- Pointer ----------|100| - (Unused)
>   * Bad:      |------------- 1 -------------|1000| - Bad slot
> + * Hibern:   |    0     |------- 1 -------|10000| - Hibernation slot

Hibern is a odd name, but I don't have better idea for this... Maybe
just HIB, match the macro name?

>   *
>   * COUNT is `SWP_TB_COUNT_BITS` long, Z is the `SWP_TB_ZERO_FLAG` bit,
>   * and together they form the `SWP_TB_FLAGS_BITS` wide flags field.
> @@ -54,6 +55,10 @@ struct swap_memcg_table {
>   *   aligned pointers.
>   *
>   * - Bad: Swap slot is reserved, protects swap header or holes on swap devices.
> + *
> + * - Hibern: Swap slot is reserved by hibernation for the suspend image, and
> + *   must never enter the swap cache. The count field is kept 0 so it never
> + *   reads as a slot in use.

Not sure if the comment is a bit misleading. The slot is treated as in use
by allocator since it is not a NULL slot, kind of like a special pin, just
like swap cache pin, which also has a count == 0, until folio_dup_swap
is called. We can never dup a hibernate slot though, which is expected.
And we never add it to swap cache.

> @@ -2201,7 +2203,6 @@ void swap_free_hibernation_slot(swp_entry_t entry)
>  	pgoff_t offset = swp_offset(entry);
>  
>  	ci = swap_cluster_lock(si, offset);
> -	__swap_cluster_put_entry(ci, offset % SWAPFILE_CLUSTER);

Nice! We can free it directly now.

>  	/*
>  	 * A slot with a folio in the swap cache is freed when the folio
>  	 * leaves the cache, the same rule swap_put_entries_cluster() follows.
> -- 
> 2.48.1
> 

Just two trivial nit picks on doc, feel free to ignore, code looks
good to me, Thanks!

Acked-by: Kairui Song <kasong@tencent.com>


  reply	other threads:[~2026-08-11 16:48 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-11 13:22 [PATCH v3 0/4] mm, swap: keep hibernation swap slots out of the swap cache Youngjun Park
2026-08-11 13:22 ` [PATCH v3 1/4] mm, swap: don't free a hibernation slot that is in " Youngjun Park
2026-08-11 13:22 ` [PATCH v3 2/4] mm, swap: only allow swapped-out slots into " Youngjun Park
2026-08-11 13:22 ` [PATCH v3 3/4] mm, swap: give hibernation swap slots their own swap table entry type Youngjun Park
2026-08-11 16:48   ` Kairui Song [this message]
2026-08-11 13:22 ` [PATCH v3 4/4] mm, swap: drop the swap cache guard and reclaim in swap_free_hibernation_slot() Youngjun Park
2026-08-11 17:09   ` Kairui Song
2026-08-11 18:46 ` [PATCH v3 0/4] mm, swap: keep hibernation swap slots out of the swap cache Andrew Morton

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ans-nAQ7dSyfPczI@KASONG-MC4 \
    --to=ryncsn@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=baohua@kernel.org \
    --cc=baoquan.he@linux.dev \
    --cc=chrisl@kernel.org \
    --cc=her0gyugyu@gmail.com \
    --cc=kasong@tencent.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=nphamcs@gmail.com \
    --cc=shikemeng@huaweicloud.com \
    --cc=wujianyue000@gmail.com \
    --cc=youngjun.park@lge.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.