Re: [PATCH mm-new v8 2/4] mm: khugepaged: refine scan progress number

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

On Fri, Feb 27, 2026 at 01:15:24AM +0800, Vernon Yang wrote:
> On Thu, Feb 26, 2026 at 11:45 PM David Hildenbrand (Arm)
> <david@kernel.org> 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.

I will take a look at this (sorry for delay) but general point - while
fix-patches are convenient, they're incredibly anti-reviewer.

I hope at some point in the future we can move away from that so you can look at
a series on list and know that what's shown there is the actual patch.

As it stands, I can't go line-by-line correctly here without quite a bit of
additional effort.

(Not a criticism of you Vernon just a general point about mm process).

>
> --
> Cheers,
> Vernon

Thanks, Lorenzo