From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-qv1-f45.google.com (mail-qv1-f45.google.com [209.85.219.45]) (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 EEB5733FE33 for ; Mon, 3 Aug 2026 16:53:55 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.219.45 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1785776039; cv=none; b=VcadOMk31mK7v0q4C6oqlFLTvTzMzZijJKXc7Dx14ipCC/khJl6aR5zeolls98/sQ+VsqG4ONKuYCftXgvlpcnpK1B1LWnBk2EXN7jiQF8fno4AcYPz/Co8Q3RiHB9+fVQlhXA0+nbph3cs1L3YeObUhXbrlP7jy8ngbGVLfuRA= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1785776039; c=relaxed/simple; bh=bGSWbB7N9+IH16P5VueNUxxexIUchaQsuXWTkkrAfr0=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=RQ5B46+9aVxZtN5ykebiwCQfsGTdJT4qlpPzRPvqQS9U+AK7zcSNFlt4jgyhNNb4GaPf+1OXKTDBM6im67VZ1ReVfkFDFvDEllmmR72PkcxRNp/xkiTGJDsr8NgStbIPqd66PsUOWMiqNP1JImDqP2uCTFwGDXDvIn3Hr/cmmAk= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=cmpxchg.org; spf=pass smtp.mailfrom=cmpxchg.org; dkim=pass (2048-bit key) header.d=cmpxchg.org header.i=@cmpxchg.org header.b=rpi5UBLK; arc=none smtp.client-ip=209.85.219.45 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=cmpxchg.org Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=cmpxchg.org Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=cmpxchg.org header.i=@cmpxchg.org header.b="rpi5UBLK" Received: by mail-qv1-f45.google.com with SMTP id 6a1803df08f44-8f29ec73064so23822966d6.1 for ; Mon, 03 Aug 2026 09:53:55 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=cmpxchg.org; s=google; t=1785776034; x=1786380834; 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=z9IfHCJ55TIpuuGVduSHBR4SUqDeJcna8GIDAX/jjGU=; b=rpi5UBLK3QBZww8++IjoeT0M4Z4J6fwGPv+eWQrvCV9oyc2FLLPIy2gJ4scZ4bLsVQ kpecxMa2LiPcDsYhMSQtLHzalbEZjgf3hRUTOqIXojvPPg+pxj1QytYa3S0bKmIYoA2u Dacfps+8h6L+Dhiw1LT/fnUah8BXar76hXMFFIYZdCKigUpr9jAQ22kQHaDp3iwzayZA meWx+mF8WeXCKoqcQgWgGbNUt7vWe7RFkCcqGfOkSGAid1kqPRCMPhVH1T4KUzkw/g+A tg8n7TVp11QcfFR14c8gAiwn0duWmP+xslpFQzOA9ZjWvHjeLB6QtUH0I8Ys7MtfZ42E GvJw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1785776034; x=1786380834; 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=z9IfHCJ55TIpuuGVduSHBR4SUqDeJcna8GIDAX/jjGU=; b=aW08zvW2ARuiMpoD0F9o+lpF+tf2fPqV2rbqLw7emTJIzoRGf9ipF+r4ULn32qeQNe wEANKT8B7zUbfojYzd3j/xZEZLpSuXB9cjm2W6qpz5paDH6UmhAc/Cw4rFdJj5Aibq6s FOy3PzVRhUjaEco2KesHRKOjIbtuGczAJ7DecjBFDF6h9CwWezVLn6Ntce8+DXPRPDNk zxzCA1++G4DSTzAZHnEAX+V2UdHUo/qPYABTR+0R081zwG4sPg82SBSclaYCUq5ETAXm CKUC+5orYPWYM0Zr6XlRZj9RXMlZc1oXO6jpYm3QNr5GUdAaDHO0gC2B0WOYp6lkiZlo cD4Q== X-Forwarded-Encrypted: i=1; AHgh+RqKOiV19a7BXRskB4Ezi2wztmJKjwkrpNMokR9AKafzrZu+PfGRuyfovHKsPPlMoJ56UMimzbhsALWIOPk=@vger.kernel.org X-Gm-Message-State: AOJu0YwbLhFg3IrZdAl59eFirZARgjksmKgJ4+rCjFwSaMTOLy1sroBL T68IyJasR7ZHiaWbEarQPBepXCsyA2NJJL6mA1hB+vDZZsIIqc7LtXVwhCUviJ3/s9M= X-Gm-Gg: AR+sD11b2dKNuQ0EETBfIXKa30eoZ6zxnnsT35wbQM7aX60RDTogSBLK3qYLx28sNhv ZAHH308o3i/EEDIGlmKKF2pEnQ9Vziu/8te5H5o4VsRF8JBMt5oOz9v0SHgVCkEXgYujOEjROj7 xW+9fNIYnuKU6wSjoeUkzVKy9mOj70lYhZ/tJN7FkE7JG757nMcXfJoRxN8bliy3+hxo0BNP1Yv 8oJ/iq7itVj+J1rg6n1tFCt4A+S2PjSN26b2l0xWOTBKMvsoeEVSHGcj6qxUsCLsIFqfkeOxYhg iERJqJqPBU3ghKwtP46XsQ1MgCnXdqDUwVxJSMsF0cgEBbe8NraQ0sh0XMDQ5z2qvWa15EaIsi6 MwnJfHUqwsq611FRhVqX78g+Il4JxEO3YBvG8FyBLN/7L5/TYhInRVvRVYFr/wgTJ74nUfZr+Xg L4B5IZnLnM9pCiT62BdRRfLDlUa6y71wy/6Xexim/aPLnjTjx+rluRw2v3kRY= X-Received: by 2002:a05:6214:5548:b0:8f0:d59d:78d2 with SMTP id 6a1803df08f44-9084956313dmr242376206d6.5.1785776034484; Mon, 03 Aug 2026 09:53:54 -0700 (PDT) Received: from localhost ([2603:7001:f100:500:365a:60ff:fe62:ff29]) by smtp.gmail.com with ESMTPSA id 6a1803df08f44-908435b6948sm79412756d6.27.2026.08.03.09.53.53 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 03 Aug 2026 09:53:53 -0700 (PDT) Date: Mon, 3 Aug 2026 12:53:53 -0400 From: Johannes Weiner To: Zi Yan Cc: David Hildenbrand , "Matthew Wilcox (Oracle)" , Andrew Morton , Muchun Song , Lorenzo Stoakes , "Liam R. Howlett" , Vlastimil Babka , Mike Rapoport , Suren Baghdasaryan , Michal Hocko , Baolin Wang , Nico Pache , Ryan Roberts , Dev Jain , Barry Song , Lance Yang , Usama Arif , Gregory Price , Ying Huang , Alistair Popple , Qi Zheng , Shakeel Butt , Kairui Song , linux-mm@kvack.org, linux-kernel@vger.kernel.org, Minchan Kim , Sergey Senozhatsky Subject: Re: [PATCH RFC 01/14] mm/zsmalloc: replace PG_private with pointer comparison Message-ID: References: <20260731-remove-pg_private-v1-0-142c97ba3562@nvidia.com> <20260731-remove-pg_private-v1-1-142c97ba3562@nvidia.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: On Mon, Aug 03, 2026 at 11:34:35AM -0400, Zi Yan wrote: > On Mon Aug 3, 2026 at 11:04 AM EDT, Johannes Weiner wrote: > > On Fri, Jul 31, 2026 at 10:13:24PM -0400, Zi Yan wrote: > >> zsmalloc uses PG_private to indicate first zpdesc in the zspage chain. > >> Replace it with zpdesc->zspage->first_zpdesc == zpdesc. The check, > >> is_first_zpdesc(), is only used in VM_BUG_ON(), so performance impact > >> should be negligible. > >> > >> It prepares for a future commit that remove PG_private. > >> > >> No functional change intended. > >> > >> Assisted-by: Claude:claude-opus-4-8 > >> Assisted-by: Codex:gpt-5 > >> Signed-off-by: Zi Yan > >> To: Minchan Kim > >> To: Sergey Senozhatsky > >> To: Andrew Morton > >> Cc: linux-mm@kvack.org > >> Cc: linux-kernel@vger.kernel.org > >> --- > >> mm/zpdesc.h | 2 +- > >> mm/zsmalloc.c | 15 +++------------ > >> 2 files changed, 4 insertions(+), 13 deletions(-) > >> > >> diff --git a/mm/zpdesc.h b/mm/zpdesc.h > >> index b8258dc78548d..4fd81c2e80769 100644 > >> --- a/mm/zpdesc.h > >> +++ b/mm/zpdesc.h > >> @@ -26,8 +26,8 @@ > >> * with memcg_data. > >> * > >> * Page flags used: > >> - * * PG_private identifies the first component page. > >> * * PG_locked is used by page migration code. > >> + * The first component page has zpdesc->zspage->first_zpdesc == zpdesc > >> */ > >> struct zpdesc { > >> unsigned long flags; > >> diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c > >> index 8204b76f78308..e8ef227624efa 100644 > >> --- a/mm/zsmalloc.c > >> +++ b/mm/zsmalloc.c > >> @@ -290,11 +290,6 @@ struct zs_pool { > >> atomic_t compaction_in_progress; > >> }; > >> > >> -static inline void zpdesc_set_first(struct zpdesc *zpdesc) > >> -{ > >> - SetPagePrivate(zpdesc_page(zpdesc)); > >> -} > >> - > >> static inline void zpdesc_inc_zone_page_state(struct zpdesc *zpdesc) > >> { > >> inc_zone_page_state(zpdesc_page(zpdesc), NR_ZSPAGES); > >> @@ -478,7 +473,7 @@ static void record_obj(unsigned long handle, unsigned long obj) > >> > >> static inline bool __maybe_unused is_first_zpdesc(struct zpdesc *zpdesc) > >> { > >> - return PagePrivate(zpdesc_page(zpdesc)); > >> + return zpdesc->zspage->first_zpdesc == zpdesc; > >> } > > > > There are two checks: get_first_zpdesc() and obj_allocated(). > > > > static struct zpdesc *get_first_zpdesc(struct zspage *zspage) > > { > > struct zpdesc *first_zpdesc = zspage->first_zpdesc; > > > > VM_BUG_ON_PAGE(is_first_zpdesc(first_zpdesc), zpdesc_page(first_zpdesc)); > > return first_zpdesc; > > } > > > > If you expand the helper, this seems kind of pointless now: > > > > first_zpdesc = zspage->first_zpdesc; > > VM_BUG_ON_PAGE(first_zpdesc != first_zpdesc->zspage->first_zpdesc, ...); > > > > Mayyybe it could make sense to assert first_zpdesc->zspage != > > zspage. But that's a separate issue that the previous check didn't > > Usama has the same comment about this. > > > necessarily catch. And might not be worth checking, considering how > > trivial create_page_chain() is. > > > > In any case, it doesn't seem worth keeping the check as-is. > > > > And with one caller remaining, you could delete the helper and inline > > that expression into the check in obj_allocated(). What it does now is > > self-explanatory; it doesn't need another name like that PagePrivate() > > check before did. > > How about the version below? Basically, I made is_first_zpdesc() more > straightforward for backpointer checking and first_zpdesc checking. > > 1. get_first_zpdesc() needs the backpointer check; the first_zpdesc check is > meaningless, since the assignment is done above. > > 2. obj_allocated() needs the first_zpdesc check; the backpointer check > is meaningless, since the zspage is from get_zspage(). Personally, I'm not a fan of "super predicates" where individual conditions are only useful for only some of the callsites. They tend to become obstacles to understanding the code and lead to subtle bugs when developers misunderstand context requirements. IMO it's better to just precisely express what each callsite needs. Only factor a common helper if it's actually the same.