On Fri, Jul 8, 2022 at 2:01 PM Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> wrote:
>
> On Wed, 6 Jul 2022 16:59:21 -0700 "Zach O'Keefe" <zokeefe@xxxxxxxxxx> wrote:
>
> > Modularize hugepage collapse by introducing struct collapse_control.
> > This structure serves to describe the properties of the requested
> > collapse, as well as serve as a local scratch pad to use during the
> > collapse itself.
> >
> > Start by moving global per-node khugepaged statistics into this
> > new structure. Note that this structure is still statically allocated
> > since CONFIG_NODES_SHIFT might be arbitrary large, and stack-allocating
> > a MAX_NUMNODES-sized array could cause -Wframe-large-than= errors.
> >
> > Signed-off-by: Zach O'Keefe <zokeefe@xxxxxxxxxx>
> > ---
> > mm/khugepaged.c | 87 ++++++++++++++++++++++++++++---------------------
> > 1 file changed, 50 insertions(+), 37 deletions(-)
> >
> > diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> > index 196eaadbf415..f1ef02d9fe07 100644
> > --- a/mm/khugepaged.c
> > +++ b/mm/khugepaged.c
> > @@ -85,6 +85,14 @@ static struct kmem_cache *mm_slot_cache __read_mostly;
> >
> > #define MAX_PTE_MAPPED_THP 8
> >
> > +struct collapse_control {
> > + /* Num pages scanned per node */
> > + int node_load[MAX_NUMNODES];
>
> Does this actually need to be 32-bit? Looking at the current code I'm
> suspecting that khugepaged_node_load[] could be a ushort?
>
> [And unsigned int would be more appropriate, but we always do that :(]
>
Hey Andrew,
Thanks for taking the time to review, and good catch - I don't think
we need 32 bits.
Minimally, we just need to be able to hold the maximum value of
HPAGE_PMD_NR = 1 << (PMD_SHIFT - PAGE_SHIFT).
I'm not sure what arch/config options (that also use THP) produce the
minimum/maximum value here. I looked through most of the archs that
define PMD_SHIFT, and couldn't find an example where we'd need > 16
bits, with most cases still requiring > 8 bits. All the various
configs do get complicated though.
Is it acceptable to use u16, with an #error if HPAGE_PMD_ORDER >= 16?
Thanks,
Zach
