From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pj1-f41.google.com (mail-pj1-f41.google.com [209.85.216.41]) (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 999033BFE2F for ; Wed, 26 Aug 2026 09:22:01 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.216.41 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1787736123; cv=none; b=YPVRI2s97S6a2eoPg58WtUmieTWq5JOgIwgMVJaA8fJcSGlYh30kU038UAOwGiFAT1M8Ir/oGKhc01STO85FFJvecZ2SEZBiunetATbC9sI+4dvDpDwdKP7aBQxXsTb0ezK7lJwREmpVvRExF8bHSf95PXrgel0BqOZeE9UdO+4= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1787736123; c=relaxed/simple; bh=IBPxf6OZGvf01iPqu934qlUWO5hXy1HyOFafFD1QGEc=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=IroDJFJUH98pdpkJwicYW5A3kNXFs5Ma+V5jV8JARJkKAnYMVwRazuEiLBsCrtiJhQGPqVoi+h4JZ5gSYBMYAitzZKrKWy9rdeHhhDI7u9AtorfmcJoq/TPAXIia4L46yh6P+4FUlGkVhz+7U92IQGmy6rnCBDm5WlbQop37Wwg= 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=DgPfnqeY; arc=none smtp.client-ip=209.85.216.41 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="DgPfnqeY" Received: by mail-pj1-f41.google.com with SMTP id 98e67ed59e1d1-3964e76d0f4so981946a91.3 for ; Wed, 26 Aug 2026 02:22:01 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20251104; t=1787736121; x=1788340921; 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=jALS777Z+qg1ejYQPjNAfxupaXWG07m9ugFq1DHHkyY=; b=DgPfnqeY9ST2DfoCpZSuuhMkzN3hcT1LSbcOcgqkTgMBoqOiezb95YN+js9dFfx6oT TYIwmTscNASIx0hE3HxvglnLOL04ulGhZT7zKbuzYbFvNRrYSAbo5OpA/laoWRNIJNcL d+UT76Jr0m65l8s3QAs7qhuQQTGgPSduQV+mJMWhmnp6jWrFtHHrpZxmuv/job0iSdiq OGYX8jzo5nW8mDms4Busk091Tbmzp94D2xAwhvdqlIbabt2lwBZ0umekXwJoxvBzLECD APQniHc0QPaLhFooHSeVWbj2p6MFBjJ7xtTYeUpudfOnkIg+2zsbRUbHztIsc0yGQFa6 xHZQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1787736121; x=1788340921; 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=jALS777Z+qg1ejYQPjNAfxupaXWG07m9ugFq1DHHkyY=; b=nOyP0IqkRXoes0vKzBzX9EV9tigkyuMeULcvEeLwnqvBIxTVhlMezEC6muJAHEx832 u4cMGp/+/NIFsCEb3bcm34Rm38/KVxzdmX7tPzeWA5TGz0N1OwvalHolgh/qCosRSolo 1OUwLHJGX/uxu9HBw9E6RVjZ2o+pq8n4HXMn6oVNOtNRS1v3YYpA7vOD0jcekSHGwZAL O8qZ+WhWDKgU+SfD2dFJlFl//4mYuS5hUkZw0MpjnqquFFTX+/RpESNNNzVzi5jQyAPw 6Um4dYMQCy4NReop+puFQB6ozGXwwPzuz9PrfifDnf1qIJR17yfi6vRgrwUTEKoTLiXA p92w== X-Forwarded-Encrypted: i=1; AHgh+Ro9fwMdNVz/Kg2YAvhMOiFjqGuV0nTU6Rh0F1aw5AOZBHGPH+e7tcdV8kd4JjDdoyCYPN3O151/JQogQVc=@vger.kernel.org X-Gm-Message-State: AFuF++mdmbdsfFydkn4jrtrGj25XVOejqfz1Ge8woFW/VNUDOmybQ4FU DjI48zEUaRaUJro61DWFgV6Zvv1YmFuayBBUOBqDvjsV/hpmJ3t0CzGf X-Gm-Gg: AR+sD12aA8bDV0hDyU6awbWZLEbp25lDet7sfUhMeAByHQkghv0joZ1RnHmJvgLCt1P iM5Eg4U5gGhyHPSRCwwOLIZqwSXWKOMk48orsKi51+DgwUdkuwMwLxHQaivZeXsUqcebQPtgsOB UmsA289zzmF6EUxll90DikxwaZ8YRQXQfJjVgFwCcBlhX3f/7psXv9NBH7Qn7+RuNxTWzhh0ih8 lfK5B3ptlH0J1dgblIqJ8r6i1kJnBF6rJ/n4yeX6kOa40BgSr297ujONrTWTR+ronncpICs4Oem PyCYJt5z/dImM/IkY0QytwWaOC73e7oBRTj9uD+3SvFzezMiJ6aG3fy2RRAuRAznAc0Exd+C2O0 TGCETFZoEd7UxiMKcNHvVKAhcKnNK8HCmLoOZeyJ+HRc2Bv/y92Py57XUBg1HT4C1dbjWTUamz1 TkZv/15OCTWJgUL6GBiixLk3PVIhm9TNMy1UmJafLYrioVz2CbHBOIImXRYgRzcfTiLKRBcKa8O fhk9pkDZ9zsVg== X-Received: by 2002:a17:90b:1804:b0:36b:bec8:94c5 with SMTP id 98e67ed59e1d1-3966d400328mr10605164a91.10.1787736120866; Wed, 26 Aug 2026 02:22:00 -0700 (PDT) Received: from localhost.localdomain ([240e:b8f:1df9:a600:c693:b19f:ada0:748]) by smtp.gmail.com with ESMTPSA id 98e67ed59e1d1-39645da08desm7002492a91.17.2026.08.26.02.21.56 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 26 Aug 2026 02:22:00 -0700 (PDT) Date: Wed, 26 Aug 2026 17:21:51 +0800 From: Vernon Yang To: "Lorenzo Stoakes (ARM)" Cc: "David Hildenbrand (Arm)" , akpm@linux-foundation.org, nico.pache@linux.dev, ryan.roberts@arm.com, dev.jain@arm.com, baohua@kernel.org, lance.yang@linux.dev, usama.arif@linux.dev, zokeefe@google.com, linux-kernel@vger.kernel.org, linux-mm@kvack.org, stable@vger.kernel.org, Vernon Yang Subject: Re: [PATCH v3 1/3] mm: khugepaged: fix swap entry value to folio_pfn() Message-ID: <330789dd-94e2-4827-abdd-23671263608f@gmail.com> References: <20260824092935.73892-2-vernon2gm@gmail.com> <6a9c2369-5589-4f2a-bcfe-c6e3b46a1ccd@gmail.com> <42695df0-3962-4126-9afb-9a1905d096a0@kernel.org> <49aa013f-f613-42be-bc58-35956ef189de@kernel.org> 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 Wed, Aug 26, 2026 at 09:35:38AM +0100, Lorenzo Stoakes (ARM) wrote: > On Wed, Aug 26, 2026 at 10:24:56AM +0200, David Hildenbrand (Arm) wrote: > > On 8/26/26 10:16, David Hildenbrand (Arm) wrote: > > > On 8/26/26 10:11, Lorenzo Stoakes (ARM) wrote: > > >> On Wed, Aug 26, 2026 at 10:08:58AM +0200, David Hildenbrand (Arm) wrote: > > >>> > > >>> Elaborate. > > >> > > >> It's overly long, I read it and am confused as to what is 'problematic' or not, > > >> it reads weirdly in English and pfn_xxx is the usual convention for naming of > > >> pfn's anyway. > > > > > > Excuse me, what? Are you now just making up arguments? > > To clarify, we have various users of "xxx_pfn" in the tree and I fail to see how > > "this is a problematic pfn" -> "problematic_pfn" is odd and why > > "pfn_problematic" would be any clearer. > > > > I do agree with the "problematic" aspect. "failed" might indeed be nicer. > > Right yeah. Mostly the push back is on the word being a bit confusing. Fair > enough on the pfn thing, failed_pfn is actually the nicest name suggested so far > :) failed_pfn is good to me. > I still think: > > if (result == SCAN_SUCCEED) { > ... > trace_mm_khugepaged_scan_file(mm, -1, file, present, swap, result); > } else { > trace_mm_khugepaged_scan_file(mm, failed_pfn, file, present, > swap, result); > } > > Is a little neater as then it's only on the failure path that we trace the > failed pfn, and otherwise we explicitly -1. I understand what you're trying to say, but personally it isn't necessary, because failed_pfn defaults to -1, and one trace_mm_khugepaged_scan_file() already covers it. If everyone clearly expresses that they want two trace_mm_khugepaged_scan_file(), please let me know explicitly. Thanks! > But it's not exactly a show stopper this :) > > Very rough edit of your patch - if you're happy then let's go with this, if not > then edit it + post so Vernon has a clear direction. I'm not feeling super > strongly on this so don't want to block anything: > > ----8<---- > diff --git a/mm/khugepaged.c b/mm/khugepaged.c > index 75639298efc27..371ee0b16d10c 100644 > --- a/mm/khugepaged.c > +++ b/mm/khugepaged.c > @@ -2683,6 +2683,7 @@ static enum scan_result collapse_scan_file(struct > mm_struct *mm, > int present, swap; > int node = NUMA_NO_NODE; > enum scan_result result = SCAN_SUCCEED; > + unsigned long failed_pfn = -1; > > present = 0; > swap = 0; > @@ -2714,6 +2715,7 @@ static enum scan_result collapse_scan_file(struct > mm_struct *mm, > } > > if (is_pmd_order(folio_order(folio))) { > + failed_pfn = folio_pfn(folio); > result = SCAN_PTE_MAPPED_HUGEPAGE; > /* > * PMD-sized THP implies that we can only try > @@ -2725,6 +2727,7 @@ static enum scan_result collapse_scan_file(struct > mm_struct *mm, > > node = folio_nid(folio); > if (collapse_scan_abort(node, cc)) { > + failed_pfn = folio_pfn(folio); > result = SCAN_SCAN_ABORT; > folio_put(folio); > break; > @@ -2732,12 +2735,14 @@ static enum scan_result collapse_scan_file(struct > mm_struct *mm, > cc->node_load[node]++; > > if (!folio_test_lru(folio)) { > + failed_pfn = folio_pfn(folio); > result = SCAN_PAGE_LRU; > folio_put(folio); > break; > } > > if (folio_expected_ref_count(folio) + 1 != folio_ref_count(folio)) { > + failed_pfn = folio_pfn(folio); > result = SCAN_PAGE_COUNT; > folio_put(folio); > break; > @@ -2773,7 +2778,7 @@ static enum scan_result collapse_scan_file(struct > mm_struct *mm, > } > - } > - trace_mm_khugepaged_scan_file(mm, folio, file, present, swap, result); > + trace_mm_khugepaged_scan_file(mm, -1, file, present, swap, > + SCAN_SUCCEED); > + } else { > + trace_mm_khugepaged_scan_file(mm, failed_pfn, file, present, > + swap, result); > + } > + > return result; > } > > -- > Cheers, Lorenzo >