Re: [PATCH mm-new v8 2/4] mm: khugepaged: refine scan progress number
From: Vernon Yang
Date: Thu Feb 26 2026 - 12:59:41 EST
On Thu, Feb 26, 2026 at 11:45 PM David Hildenbrand (Arm)
<david@xxxxxxxxxx> wrote:
>
> On 2/26/26 15:31, Vernon Yang wrote:
> > On Wed, Feb 25, 2026 at 03:29:05PM +0100, David Hildenbrand (Arm) wrote:
> >> On 2/25/26 15:25, Vernon Yang wrote:
> >>>
> >>> Thank you for suggestion.
> >>>
> >>> Placing it inside "struct collapse_control" makes the overall code
> >>> simpler, there also coincidentally has a 4-bytes hole, as shown below:
> >>>
> >>> struct collapse_control {
> >>> bool is_khugepaged; /* 0 1 */
> >>>
> >>> /* XXX 3 bytes hole, try to pack */
> >>>
> >>> u32 node_load[64]; /* 4 256 */
> >>>
> >>> /* XXX 4 bytes hole, try to pack */
> >>>
> >>> /* --- cacheline 4 boundary (256 bytes) was 8 bytes ago --- */
> >>> nodemask_t alloc_nmask; /* 264 8 */
> >>>
> >>> /* size: 272, cachelines: 5, members: 3 */
> >>> /* sum members: 265, holes: 2, sum holes: 7 */
> >>> /* last cacheline: 16 bytes */
> >>> };
> >>>
> >>> But regardless of khugepaged or madvise(MADV_COLLAPSE), "cur_progress"
> >>> will be counted, while madvise(MADV_COLLAPSE) actually does not need to
> >>> be counted.
> >>>
> >>> David, do we want to place "cur_progress" inside the "struct collapse_control"?
> >>
> >> Might end up looking nicer code-wise. But the reset semantics (within a
> >> pmd) are a bit weird.
> >>
> >>> If Yes, it would be better to rename "cur_progress" to "pmd_progress",
> >>> as show below:
> >>>
> >>
> >> "pmd_progress" is misleading. "progress_in_pmd" might be clearer.
> >>
> >> Play with it to see if it looks better :)
> >
> > Hi Andrew, David,
> >
> > Based on previous discussions [1], v2 as follow, and testing shows the
> > same performance benefits. Just make code cleaner, no function changes.
> >
> > If David has no further revisions, Andrew, could you please squash the
> > following clean into this patch? If you prefer a new version, please let
> > me know. Thanks.
>
> Do we also have to update the resulting patch description? Patch itself
> LGTM.
No need to update the patch description.
--
Cheers,
Vernon