Re: [PATCH v3 1/3] mm: khugepaged: fix swap entry value to folio_pfn()

["Lorenzo Stoakes (ARM)" <ljs@kernel.org>]

On Wed, Aug 26, 2026 at 05:21:51PM +0800, Vernon Yang wrote:
> 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.

Thanks.

>
> > 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!

See David's reply, we'd like to go with this thanks.

--
Cheers, Lorenzo