From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pj1-f44.google.com (mail-pj1-f44.google.com [209.85.216.44]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 1B3313CF21D for ; Sat, 8 Aug 2026 13:25:53 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.216.44 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1786195556; cv=none; b=R2NeXMtujwkQMBpOqZemZ2TJywil329IDa47S17kJoxHL6uXBgLnHxqUkLMwnHYp4nJRk2U2mrx8K2Myudlv2h5H5Be9hECuDrgJ++PpwB6J84JKU8wNnlfGmPFcb7kKadZQ/QQ+I5G1Wyrj8ASJmaHfE8UjZ5hV1kQipkOCdhM= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1786195556; c=relaxed/simple; bh=cyEjymEI58PV8cxhY91bQ67EoUDFHQyLnJz0wlSHmiY=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=GPiB7N0sUsBoSJXUA5wi9RCCKYKu0X7pEBna8WIl0cB5P+QaYo44g/Bc6BxwoY9XMAGHRsBHmDKtimmOwHtC2oa6SBlfP6bsZRyMlpVnUiW6C/mFVD4CMf9VcZppjI3MrEcRWuC6+XoqUiTnB77M0Vo2d5vjEZYgnSJPCB6Wz7w= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=qDBIetEa; arc=none smtp.client-ip=209.85.216.44 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="qDBIetEa" Received: by mail-pj1-f44.google.com with SMTP id 98e67ed59e1d1-381c51fde6bso495749a91.2 for ; Sat, 08 Aug 2026 06:25:53 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20251104; t=1786195553; x=1786800353; darn=vger.kernel.org; h=in-reply-to:content-disposition:content-type:mime-version :references:message-id:subject:cc:to:from:date:from:to:cc:subject :date:message-id:reply-to:content-type; bh=aEPuJi9dOtjcd1sSrhmRN/FNlJj3jv4zUiWWrq20D8g=; b=qDBIetEajCDE2yT/EHcDd/RqTJXa7l5vHz74cJf8Y3mCq44yron6UhNB7ky9Y8HTR0 c+UkYTKVMinVRjAFW4xJze1KoF0aTGS/jnz8yQVRR0ZacWjeXXJ30bcYqVbwM88gF+9X PjY3nYXmN8oFVGqk363wCcOH/QyQSwA0UqSzhLMQJm+xNGAqEeRGuC4ab4RUogBu6CKH +nD7bDSu6Fd2ohcravMkQf7K8k91xnO0qdGCA5Lqp4NrH/nCN6sN2CnG13/FnRvQcGA4 JfXZLQPKsLe5iRJmZcPOVq8tPS2Y0HHKCsBuNtT+iMFv8fn+lEmITuX/oeYXH+pJZbLa CpkQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1786195553; x=1786800353; h=in-reply-to:content-disposition:content-type:mime-version :references:message-id:subject:cc:to:from:date:x-gm-gg :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to :content-type; bh=aEPuJi9dOtjcd1sSrhmRN/FNlJj3jv4zUiWWrq20D8g=; b=qRf/xDcLxTZ1EoF2dQl9Hw2KlQPXN4GLZLK9ER5n78bXWyHZq9y8wGdxXRl5aYvdnW sMY3dKAiEk+t1fM0PxTOu9q+OI8y8+yr/GsOCb4v8nE6WUBBWMou5PtWPuGaAta8qHvq Qohth/pZFkcoVhrX0bRZFe2NhSvCSE4UWk91shLeBsE4L57A4g/HDCCuUFUGwGkGvfG/ FulhiwmVflrW+bAhFRGognj20KBKo6LdtrerGDOIUMLAD5IxCJzfmQ0fF/wShdD2teE0 lMlwH4t0M/CVoZ9BBpkIuuTSX1ent/D4LR0803jYCVbdnQ8YSaMgwgwTN2pKEQ73OCIz 73OA== X-Forwarded-Encrypted: i=1; AHgh+RpcrkBryAm9FCiXq4biO2NH+svftfuAxUQSH+zak/HNRIHFu0uky+ZTzSJzDGoOt+RdkA5Gc0YapIb9LIM=@vger.kernel.org X-Gm-Message-State: AOJu0YxNwFHPkZs806KnDPwvvKY0wagj9hwPIPdtmMtO/yLk/l6qpdd1 j+bfM14NHR9EjmEP5C3Cd26vhJ7g2X+IImy2GTRlrO9TzR2FRT6OKM1C X-Gm-Gg: AR+sD13caJqq/z2+UxZzPxvKlv56A/fyZO9FlICmSCWxsN1rEqnPANl4ICUy0GlYTJS FuAyFYBoRxNWt9h56Z9BMhXbbfg3VxqpLM7LrNEu82rwNPsc4FL8U0oNfb5WExhFXyLYWNno5WK vqPjkJHIKssSOHEbOQzr+mkQ0XHFVbC6LsKbLFO1276ESJFNhCU2gRSJzYDi2iMna1AvbcfRhn5 1BezgWbuUDrKGfPLNSixhwdKt1twCRpuNyIIbIRapNVfrujv7fA5znZC3Vjb3WRBpGdb1O/hPnD Wd3NLYZYWNsRNdVRGA+NjDjl+gdO/UrbccZ5a2vFciURrUjsG4++c/3S+VG1X9S7UtMdp+JVVNG 5N4BCS8FfbU2uO/m3JTIRlPPnK6Tv/+tCwukaQ6Ojun/y6E+CwaoxgHrqBpvQMlSPAmr3fhmJV8 DdlPNTB0YJrv+jMWO/xNAzqSzkd7HxTX35Og6sTAUsMaF6PadcQluK1z3Fa3I8C6uszg9x9mmkR PfxMtpIIyEwi24XlK/o5k/SjiZGmTGL7n0= X-Received: by 2002:a17:90b:2748:b0:380:7688:fbe9 with SMTP id 98e67ed59e1d1-392823a4a2bmr7598863a91.8.1786195552899; Sat, 08 Aug 2026 06:25:52 -0700 (PDT) Received: from KASONG-MC4 ([101.32.222.185]) by smtp.gmail.com with ESMTPSA id 98e67ed59e1d1-39085f5c9a5sm8330201a91.15.2026.08.08.06.25.49 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 08 Aug 2026 06:25:52 -0700 (PDT) Date: Sat, 8 Aug 2026 21:25:47 +0800 From: Kairui Song To: Youngjun Park Cc: Andrew Morton , Chris Li , Kairui Song , Kemeng Shi , Nhat Pham , Baoquan He , Barry Song , Jianyue Wu , her0gyugyu@gmail.com, linux-mm@kvack.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 2/4] mm, swap: give hibernation swap slots their own swap table entry type Message-ID: References: <20260806190636.446205-1-youngjun.park@lge.com> <20260806190636.446205-3-youngjun.park@lge.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20260806190636.446205-3-youngjun.park@lge.com> On Fri, Aug 07, 2026 at 04:06:34AM +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, the same shape as SWP_TB_BAD. Bits 0 to 3 are taken by the shadow, > PFN, pointer and bad marks, so bit 4 is the first free one. Neither type > holds 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 next patch needs these slots to stop looking like shadows. > > Suggested-by: Kairui Song > Link: https://lore.kernel.org/linux-mm/abp7aDgYLrxF3Me8@KASONG-MC4/ > Signed-off-by: Youngjun Park > --- > mm/swap_table.h | 12 ++++++++++++ > mm/swapfile.c | 13 +++++++------ > 2 files changed, 19 insertions(+), 6 deletions(-) > > diff --git a/mm/swap_table.h b/mm/swap_table.h > index e6613e62f8d0..c1c516bcc17e 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: |------------ 1 -------------|10000| - Hibernation slot Nice! Just one idea, would it be nicer if we have: * Hibern: | 0 |------- 1 -------------|10000| - Hibernation slot Or: * Hibern: |0..001|------- 1 -------------|10000| - Hibernation slot That way if we accidentally used __swp_tb_get_count, it return a actual meaningful value instead of MAX. Either 0 - the slot is not used as a countable ordinary slot, or 1 - the slot has one user: hibernation. Maybe 0 is better at least for the intermediate commit, see below. > > +static inline bool swp_tb_is_hibernation(unsigned long swp_tb) > +{ > + return swp_tb == SWP_TB_HIB; > +} > + > static inline bool swp_tb_is_countable(unsigned long swp_tb) > { > return (swp_tb_is_shadow(swp_tb) || swp_tb_is_folio(swp_tb) || > diff --git a/mm/swapfile.c b/mm/swapfile.c > index f5dfc7e59191..a337387f7431 100644 > --- a/mm/swapfile.c > +++ b/mm/swapfile.c > @@ -928,7 +928,7 @@ static bool __swap_cluster_alloc_entries(struct swap_info_struct *si, > * upon folio unmap. > * > * Else, it's a exclusive order 0 allocation for hibernation. > - * The slot starts with count == 1 and never increases. > + * The slot carries no swap count and is freed by offset. > */ > if (likely(folio)) { > order = folio_order(folio); > @@ -940,8 +940,8 @@ static bool __swap_cluster_alloc_entries(struct swap_info_struct *si, > order = 0; > nr_pages = 1; > swap_cluster_assert_empty(ci, ci_off, 1, false); > - /* Fake shadow placeholder with no flag, hibernation does not use the zeromap */ > - __swap_table_set(ci, ci_off, __swp_tb_mk_count(shadow_to_swp_tb(NULL, 0), 1)); > + /* Exclusively owned by hibernation, must never enter the swap cache */ > + __swap_table_set(ci, ci_off, SWP_TB_HIB); > } else { > /* Allocation without folio is only possible with hibernation */ > WARN_ON_ONCE(1); > @@ -1929,9 +1929,11 @@ void __swap_cluster_free_entries(struct swap_info_struct *si, > old_tb = __swap_table_get(ci, ci_off); > /* > * Freeing is done after release of the last swap count > - * ref, or after swap cache is dropped > + * ref, or after swap cache is dropped. A hibernation slot > + * has no count and is freed directly by its owner. > */ > - VM_WARN_ON(!swp_tb_is_shadow(old_tb) || __swp_tb_get_count(old_tb) > 1); > + VM_WARN_ON(!swp_tb_is_hibernation(old_tb) && > + (!swp_tb_is_shadow(old_tb) || __swp_tb_get_count(old_tb) > 1)); > > /* Resetting the slot to NULL also clears the inline flags. */ > __swap_table_set(ci, ci_off, null_to_swp_tb()); > @@ -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); > /* > * 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. This idea is right, but is the patch in the right order? If readahead tried to add a folio to a hibernate slot by accident, seems nothing blocks that in the current patch, and that PFN slot will have a (MAX) count value, and considered countable? If the that folio is somehow reclaimed, we got a corrupted shadow (hib type is gone)? If we have the count part of a hibernation slot be 0, __swap_cache_add_check will fail natively, seems there will be no such risk. A few existing helpers can also help catch potential wrong freeing of hibernation slot. (underflow check). The layout can be changed again afterwards. How do you think?