Re: [PATCH v3 3/4] mm, swap: give hibernation swap slots their own swap table entry type

[Kairui Song]

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@xxxxxxxxxxx>
> Link: https://lore.kernel.org/linux-mm/abp7aDgYLrxF3Me8@KASONG-MC4/
> Signed-off-by: Youngjun Park <youngjun.park@xxxxxxx>
> ---
>  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@xxxxxxxxxxx>