mirror of https://lore.kernel.org/lkml/
 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: 10+ 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
2026-08-12 12:13   ` Youngjun Park
2026-08-12 20:56     ` 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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®