The global variable 'khugepaged_collapse_control' is not used outside of
mm/khugepaged.c. Make it static to limit its scope.

Reviewed-by: Wei Yang <richard.weiyang@gmail.com>
Reviewed-by: Zi Yan <ziy@nvidia.com>
Acked-by: David Hildenbrand (Red Hat) <david@kernel.org>
Reviewed-by: Anshuman Khandual <anshuman.khandual@arm.com>
Signed-off-by: Shivank Garg <shivankg@amd.com>
---
 mm/khugepaged.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/khugepaged.c b/mm/khugepaged.c
index 1667abae6d8d..fba6aea5bea6 100644
--- a/mm/khugepaged.c
+++ b/mm/khugepaged.c
@@ -827,7 +827,7 @@ static void khugepaged_alloc_sleep(void)
 	remove_wait_queue(&khugepaged_wait, &wait);
 }
 
-struct collapse_control khugepaged_collapse_control = {
+static struct collapse_control khugepaged_collapse_control = {
 	.is_khugepaged = true,
 };
 
-- 
2.43.0

On 19/01/26 12:53 am, Shivank Garg wrote:
> The global variable 'khugepaged_collapse_control' is not used outside of
> mm/khugepaged.c. Make it static to limit its scope.
>
> Reviewed-by: Wei Yang <richard.weiyang@gmail.com>
> Reviewed-by: Zi Yan <ziy@nvidia.com>
> Acked-by: David Hildenbrand (Red Hat) <david@kernel.org>
> Reviewed-by: Anshuman Khandual <anshuman.khandual@arm.com>
> Signed-off-by: Shivank Garg <shivankg@amd.com>
> ---
>  mm/khugepaged.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> index 1667abae6d8d..fba6aea5bea6 100644
> --- a/mm/khugepaged.c
> +++ b/mm/khugepaged.c
> @@ -827,7 +827,7 @@ static void khugepaged_alloc_sleep(void)
>  	remove_wait_queue(&khugepaged_wait, &wait);
>  }
>  
> -struct collapse_control khugepaged_collapse_control = {
> +static struct collapse_control khugepaged_collapse_control = {
>  	.is_khugepaged = true,
>  };
>  

Will it not be better to just remove this variable? In madvise_collapse,
we defined cc as a local variable and set .is_khugepaged = false. The
same can be done in int khugepaged() - define a local variable and set
.is_khugepaged = true.

On 22/01/26 2:58 pm, Dev Jain wrote:
> On 19/01/26 12:53 am, Shivank Garg wrote:
>> The global variable 'khugepaged_collapse_control' is not used outside of
>> mm/khugepaged.c. Make it static to limit its scope.
>>
>> Reviewed-by: Wei Yang <richard.weiyang@gmail.com>
>> Reviewed-by: Zi Yan <ziy@nvidia.com>
>> Acked-by: David Hildenbrand (Red Hat) <david@kernel.org>
>> Reviewed-by: Anshuman Khandual <anshuman.khandual@arm.com>
>> Signed-off-by: Shivank Garg <shivankg@amd.com>
>> ---
>>  mm/khugepaged.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
>> index 1667abae6d8d..fba6aea5bea6 100644
>> --- a/mm/khugepaged.c
>> +++ b/mm/khugepaged.c
>> @@ -827,7 +827,7 @@ static void khugepaged_alloc_sleep(void)
>>  	remove_wait_queue(&khugepaged_wait, &wait);
>>  }
>>  
>> -struct collapse_control khugepaged_collapse_control = {
>> +static struct collapse_control khugepaged_collapse_control = {
>>  	.is_khugepaged = true,
>>  };
>>  
> Will it not be better to just remove this variable? In madvise_collapse,
> we defined cc as a local variable and set .is_khugepaged = false. The
> same can be done in int khugepaged() - define a local variable and set
> .is_khugepaged = true.

Since this patch has been stabilized already by 4 R-bs, it may be a headache
to now remove this, we can do my suggestion later.

Reviewed-by: Dev Jain <dev.jain@arm.com>

>
>

On 1/23/2026 1:18 PM, Dev Jain wrote:
> 
> On 22/01/26 2:58 pm, Dev Jain wrote:
>> On 19/01/26 12:53 am, Shivank Garg wrote:
>>> The global variable 'khugepaged_collapse_control' is not used outside of
>>> mm/khugepaged.c. Make it static to limit its scope.
>>>
>>> Reviewed-by: Wei Yang <richard.weiyang@gmail.com>
>>> Reviewed-by: Zi Yan <ziy@nvidia.com>
>>> Acked-by: David Hildenbrand (Red Hat) <david@kernel.org>
>>> Reviewed-by: Anshuman Khandual <anshuman.khandual@arm.com>
>>> Signed-off-by: Shivank Garg <shivankg@amd.com>
>>> ---
>>>  mm/khugepaged.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
>>> index 1667abae6d8d..fba6aea5bea6 100644
>>> --- a/mm/khugepaged.c
>>> +++ b/mm/khugepaged.c
>>> @@ -827,7 +827,7 @@ static void khugepaged_alloc_sleep(void)
>>>  	remove_wait_queue(&khugepaged_wait, &wait);
>>>  }
>>>  
>>> -struct collapse_control khugepaged_collapse_control = {
>>> +static struct collapse_control khugepaged_collapse_control = {
>>>  	.is_khugepaged = true,
>>>  };
>>>  
>> Will it not be better to just remove this variable? In madvise_collapse,
>> we defined cc as a local variable and set .is_khugepaged = false. The
>> same can be done in int khugepaged() - define a local variable and set
>> .is_khugepaged = true.
> 
> Since this patch has been stabilized already by 4 R-bs, it may be a headache
> to now remove this, we can do my suggestion later.
> 
> Reviewed-by: Dev Jain <dev.jain@arm.com>
> 
>>
>>

Thank you Dev for the feedback and review.

I've attached the patch implementing your suggestion and sending this as a separate
follow-up to avoid disrupting the current series.

I’m happy to queue it for next cycle or if it’s acceptable now, please take it.

Thanks for the suggestion!

Regards,
Shivank

---
From: Shivank Garg <shivankg@amd.com>
Date: Thu, 22 Jan 2026 12:36:28 +0000
Subject: [PATCH] mm/khugepaged: convert khugepaged_collapse_control to local
 variable in khugepaged()

Make khugepaged_collapse_control a local variable in khugepaged() instead
of static global, consistent with how madvise_collapse() handles its
collapse_control. Static storage is unnecessary here as node_load and
alloc_nmask are reset per-VMA during scanning.

No functional change.

Suggested-by: Dev Jain <dev.jain@arm.com>
Signed-off-by: Shivank Garg <shivankg@amd.com>
---
 mm/khugepaged.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/mm/khugepaged.c b/mm/khugepaged.c
index 9f790ec34400..c18d2ce639b1 100644
--- a/mm/khugepaged.c
+++ b/mm/khugepaged.c
@@ -829,10 +829,6 @@ static void khugepaged_alloc_sleep(void)
 	remove_wait_queue(&khugepaged_wait, &wait);
 }
 
-static struct collapse_control khugepaged_collapse_control = {
-	.is_khugepaged = true,
-};
-
 static bool hpage_collapse_scan_abort(int nid, struct collapse_control *cc)
 {
 	int i;
@@ -2629,13 +2625,16 @@ static void khugepaged_wait_work(void)
 
 static int khugepaged(void *none)
 {
+	struct collapse_control cc = {
+		.is_khugepaged = true,
+	};
 	struct mm_slot *slot;
 
 	set_freezable();
 	set_user_nice(current, MAX_NICE);
 
 	while (!kthread_should_stop()) {
-		khugepaged_do_scan(&khugepaged_collapse_control);
+		khugepaged_do_scan(&cc);
 		khugepaged_wait_work();
 	}
 
-- 
2.43.0

NAK to this change....

On Fri, Jan 23, 2026 at 03:03:58PM +0530, Garg, Shivank wrote:
>
>
> On 1/23/2026 1:18 PM, Dev Jain wrote:
> >
> > On 22/01/26 2:58 pm, Dev Jain wrote:
> >> On 19/01/26 12:53 am, Shivank Garg wrote:
> >>> The global variable 'khugepaged_collapse_control' is not used outside of
> >>> mm/khugepaged.c. Make it static to limit its scope.
> >>>
> >>> Reviewed-by: Wei Yang <richard.weiyang@gmail.com>
> >>> Reviewed-by: Zi Yan <ziy@nvidia.com>
> >>> Acked-by: David Hildenbrand (Red Hat) <david@kernel.org>
> >>> Reviewed-by: Anshuman Khandual <anshuman.khandual@arm.com>
> >>> Signed-off-by: Shivank Garg <shivankg@amd.com>
> >>> ---
> >>>  mm/khugepaged.c | 2 +-
> >>>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>>
> >>> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> >>> index 1667abae6d8d..fba6aea5bea6 100644
> >>> --- a/mm/khugepaged.c
> >>> +++ b/mm/khugepaged.c
> >>> @@ -827,7 +827,7 @@ static void khugepaged_alloc_sleep(void)
> >>>  	remove_wait_queue(&khugepaged_wait, &wait);
> >>>  }
> >>>
> >>> -struct collapse_control khugepaged_collapse_control = {
> >>> +static struct collapse_control khugepaged_collapse_control = {
> >>>  	.is_khugepaged = true,
> >>>  };
> >>>
> >> Will it not be better to just remove this variable? In madvise_collapse,
> >> we defined cc as a local variable and set .is_khugepaged = false. The
> >> same can be done in int khugepaged() - define a local variable and set
> >> .is_khugepaged = true.
> >
> > Since this patch has been stabilized already by 4 R-bs, it may be a headache
> > to now remove this, we can do my suggestion later.
> >
> > Reviewed-by: Dev Jain <dev.jain@arm.com>
> >
> >>
> >>
>
> Thank you Dev for the feedback and review.
>
> I've attached the patch implementing your suggestion and sending this as a separate
> follow-up to avoid disrupting the current series.
>
> I’m happy to queue it for next cycle or if it’s acceptable now, please take it.
>
> Thanks for the suggestion!
>
> Regards,
> Shivank
>
> ---
> From: Shivank Garg <shivankg@amd.com>
> Date: Thu, 22 Jan 2026 12:36:28 +0000
> Subject: [PATCH] mm/khugepaged: convert khugepaged_collapse_control to local
>  variable in khugepaged()
>
> Make khugepaged_collapse_control a local variable in khugepaged() instead
> of static global, consistent with how madvise_collapse() handles its
> collapse_control. Static storage is unnecessary here as node_load and
> alloc_nmask are reset per-VMA during scanning.
>
> No functional change.
>
> Suggested-by: Dev Jain <dev.jain@arm.com>
> Signed-off-by: Shivank Garg <shivankg@amd.com>
> ---
>  mm/khugepaged.c | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> index 9f790ec34400..c18d2ce639b1 100644
> --- a/mm/khugepaged.c
> +++ b/mm/khugepaged.c
> @@ -829,10 +829,6 @@ static void khugepaged_alloc_sleep(void)
>  	remove_wait_queue(&khugepaged_wait, &wait);
>  }
>
> -static struct collapse_control khugepaged_collapse_control = {
> -	.is_khugepaged = true,
> -};
> -
>  static bool hpage_collapse_scan_abort(int nid, struct collapse_control *cc)
>  {
>  	int i;
> @@ -2629,13 +2625,16 @@ static void khugepaged_wait_work(void)
>
>  static int khugepaged(void *none)
>  {
> +	struct collapse_control cc = {
> +		.is_khugepaged = true,
> +	};
>  	struct mm_slot *slot;
>
>  	set_freezable();
>  	set_user_nice(current, MAX_NICE);
>
>  	while (!kthread_should_stop()) {
> -		khugepaged_do_scan(&khugepaged_collapse_control);
> +		khugepaged_do_scan(&cc);
>  		khugepaged_wait_work();
>  	}
>
> --
> 2.43.0
>
>
>
>

Andrew's already commented but this is terribly mistaken.

The argument against it (why did nobody check...) is that this struct is HUGE
and there's really no benefit to doing this.

Nico's series makes this struct even bigger (...!)

Dev - PLEASE use pahole or sizeof(...) or something before suggesting moving
things like this on to the stack, in future e.g.:

$ pahole collapse_control
struct collapse_control {
	bool                       is_khugepaged;        /*     0     1 */

	/* XXX 3 bytes hole, try to pack */

	u32                        node_load[1024];      /*     4  4096 */

	/* XXX 4 bytes hole, try to pack */

	/* --- cacheline 64 boundary (4096 bytes) was 8 bytes ago --- */
	nodemask_t                 alloc_nmask;          /*  4104   128 */

	/* size: 4232, cachelines: 67, members: 3 */
	/* sum members: 4225, holes: 2, sum holes: 7 */
	/* last cacheline: 8 bytes */
};

Making this static was fine. Leave it as-is.

Thanks, Lorenzo

On 24/01/26 2:31 pm, Lorenzo Stoakes wrote:
> NAK to this change....
>
> On Fri, Jan 23, 2026 at 03:03:58PM +0530, Garg, Shivank wrote:
>>
>> On 1/23/2026 1:18 PM, Dev Jain wrote:
>>> On 22/01/26 2:58 pm, Dev Jain wrote:
>>>> On 19/01/26 12:53 am, Shivank Garg wrote:
>>>>> The global variable 'khugepaged_collapse_control' is not used outside of
>>>>> mm/khugepaged.c. Make it static to limit its scope.
>>>>>
>>>>> Reviewed-by: Wei Yang <richard.weiyang@gmail.com>
>>>>> Reviewed-by: Zi Yan <ziy@nvidia.com>
>>>>> Acked-by: David Hildenbrand (Red Hat) <david@kernel.org>
>>>>> Reviewed-by: Anshuman Khandual <anshuman.khandual@arm.com>
>>>>> Signed-off-by: Shivank Garg <shivankg@amd.com>
>>>>> ---
>>>>>  mm/khugepaged.c | 2 +-
>>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
>>>>> index 1667abae6d8d..fba6aea5bea6 100644
>>>>> --- a/mm/khugepaged.c
>>>>> +++ b/mm/khugepaged.c
>>>>> @@ -827,7 +827,7 @@ static void khugepaged_alloc_sleep(void)
>>>>>  	remove_wait_queue(&khugepaged_wait, &wait);
>>>>>  }
>>>>>
>>>>> -struct collapse_control khugepaged_collapse_control = {
>>>>> +static struct collapse_control khugepaged_collapse_control = {
>>>>>  	.is_khugepaged = true,
>>>>>  };
>>>>>
>>>> Will it not be better to just remove this variable? In madvise_collapse,
>>>> we defined cc as a local variable and set .is_khugepaged = false. The
>>>> same can be done in int khugepaged() - define a local variable and set
>>>> .is_khugepaged = true.
>>> Since this patch has been stabilized already by 4 R-bs, it may be a headache
>>> to now remove this, we can do my suggestion later.
>>>
>>> Reviewed-by: Dev Jain <dev.jain@arm.com>
>>>
>>>>
>> Thank you Dev for the feedback and review.
>>
>> I've attached the patch implementing your suggestion and sending this as a separate
>> follow-up to avoid disrupting the current series.
>>
>> I’m happy to queue it for next cycle or if it’s acceptable now, please take it.
>>
>> Thanks for the suggestion!
>>
>> Regards,
>> Shivank
>>
>> ---
>> From: Shivank Garg <shivankg@amd.com>
>> Date: Thu, 22 Jan 2026 12:36:28 +0000
>> Subject: [PATCH] mm/khugepaged: convert khugepaged_collapse_control to local
>>  variable in khugepaged()
>>
>> Make khugepaged_collapse_control a local variable in khugepaged() instead
>> of static global, consistent with how madvise_collapse() handles its
>> collapse_control. Static storage is unnecessary here as node_load and
>> alloc_nmask are reset per-VMA during scanning.
>>
>> No functional change.
>>
>> Suggested-by: Dev Jain <dev.jain@arm.com>
>> Signed-off-by: Shivank Garg <shivankg@amd.com>
>> ---
>>  mm/khugepaged.c | 9 ++++-----
>>  1 file changed, 4 insertions(+), 5 deletions(-)
>>
>> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
>> index 9f790ec34400..c18d2ce639b1 100644
>> --- a/mm/khugepaged.c
>> +++ b/mm/khugepaged.c
>> @@ -829,10 +829,6 @@ static void khugepaged_alloc_sleep(void)
>>  	remove_wait_queue(&khugepaged_wait, &wait);
>>  }
>>
>> -static struct collapse_control khugepaged_collapse_control = {
>> -	.is_khugepaged = true,
>> -};
>> -
>>  static bool hpage_collapse_scan_abort(int nid, struct collapse_control *cc)
>>  {
>>  	int i;
>> @@ -2629,13 +2625,16 @@ static void khugepaged_wait_work(void)
>>
>>  static int khugepaged(void *none)
>>  {
>> +	struct collapse_control cc = {
>> +		.is_khugepaged = true,
>> +	};
>>  	struct mm_slot *slot;
>>
>>  	set_freezable();
>>  	set_user_nice(current, MAX_NICE);
>>
>>  	while (!kthread_should_stop()) {
>> -		khugepaged_do_scan(&khugepaged_collapse_control);
>> +		khugepaged_do_scan(&cc);
>>  		khugepaged_wait_work();
>>  	}
>>
>> --
>> 2.43.0
>>
>>
>>
>>
> Andrew's already commented but this is terribly mistaken.
>
> The argument against it (why did nobody check...) is that this struct is HUGE
> and there's really no benefit to doing this.
>
> Nico's series makes this struct even bigger (...!)
>
> Dev - PLEASE use pahole or sizeof(...) or something before suggesting moving
> things like this on to the stack, in future e.g.:
>
> $ pahole collapse_control
> struct collapse_control {
> 	bool                       is_khugepaged;        /*     0     1 */
>
> 	/* XXX 3 bytes hole, try to pack */
>
> 	u32                        node_load[1024];      /*     4  4096 */
>
> 	/* XXX 4 bytes hole, try to pack */
>
> 	/* --- cacheline 64 boundary (4096 bytes) was 8 bytes ago --- */
> 	nodemask_t                 alloc_nmask;          /*  4104   128 */
>
> 	/* size: 4232, cachelines: 67, members: 3 */
> 	/* sum members: 4225, holes: 2, sum holes: 7 */
> 	/* last cacheline: 8 bytes */
> };
>
> Making this static was fine. Leave it as-is.

I wasn't suggesting that! When I said

"In madvise_collapse, we defined cc as a local variable and set .is_khugepaged = false. The
same can be done in int khugepaged() - define a local variable and set .is_khugepaged = true."

madvise_collapse does kmalloc() to allocate this large struct. I was suggesting to do the
same for khugepaged, to enforce consistency.

>
> Thanks, Lorenzo

On Sat, Jan 24, 2026 at 04:24:24PM +0530, Dev Jain wrote:
>
> On 24/01/26 2:31 pm, Lorenzo Stoakes wrote:
> > NAK to this change....
> >
> > On Fri, Jan 23, 2026 at 03:03:58PM +0530, Garg, Shivank wrote:
> >>
> >> On 1/23/2026 1:18 PM, Dev Jain wrote:
> >>> On 22/01/26 2:58 pm, Dev Jain wrote:
> >>>> On 19/01/26 12:53 am, Shivank Garg wrote:
> >>>>> The global variable 'khugepaged_collapse_control' is not used outside of
> >>>>> mm/khugepaged.c. Make it static to limit its scope.
> >>>>>
> >>>>> Reviewed-by: Wei Yang <richard.weiyang@gmail.com>
> >>>>> Reviewed-by: Zi Yan <ziy@nvidia.com>
> >>>>> Acked-by: David Hildenbrand (Red Hat) <david@kernel.org>
> >>>>> Reviewed-by: Anshuman Khandual <anshuman.khandual@arm.com>
> >>>>> Signed-off-by: Shivank Garg <shivankg@amd.com>
> >>>>> ---
> >>>>>  mm/khugepaged.c | 2 +-
> >>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>>>>
> >>>>> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> >>>>> index 1667abae6d8d..fba6aea5bea6 100644
> >>>>> --- a/mm/khugepaged.c
> >>>>> +++ b/mm/khugepaged.c
> >>>>> @@ -827,7 +827,7 @@ static void khugepaged_alloc_sleep(void)
> >>>>>  	remove_wait_queue(&khugepaged_wait, &wait);
> >>>>>  }
> >>>>>
> >>>>> -struct collapse_control khugepaged_collapse_control = {
> >>>>> +static struct collapse_control khugepaged_collapse_control = {
> >>>>>  	.is_khugepaged = true,
> >>>>>  };
> >>>>>
> >>>> Will it not be better to just remove this variable? In madvise_collapse,
> >>>> we defined cc as a local variable and set .is_khugepaged = false. The
> >>>> same can be done in int khugepaged() - define a local variable and set
> >>>> .is_khugepaged = true.
> >>> Since this patch has been stabilized already by 4 R-bs, it may be a headache
> >>> to now remove this, we can do my suggestion later.
> >>>
> >>> Reviewed-by: Dev Jain <dev.jain@arm.com>
> >>>
> >>>>
> >> Thank you Dev for the feedback and review.
> >>
> >> I've attached the patch implementing your suggestion and sending this as a separate
> >> follow-up to avoid disrupting the current series.
> >>
> >> I’m happy to queue it for next cycle or if it’s acceptable now, please take it.
> >>
> >> Thanks for the suggestion!
> >>
> >> Regards,
> >> Shivank
> >>
> >> ---
> >> From: Shivank Garg <shivankg@amd.com>
> >> Date: Thu, 22 Jan 2026 12:36:28 +0000
> >> Subject: [PATCH] mm/khugepaged: convert khugepaged_collapse_control to local
> >>  variable in khugepaged()
> >>
> >> Make khugepaged_collapse_control a local variable in khugepaged() instead
> >> of static global, consistent with how madvise_collapse() handles its
> >> collapse_control. Static storage is unnecessary here as node_load and
> >> alloc_nmask are reset per-VMA during scanning.
> >>
> >> No functional change.
> >>
> >> Suggested-by: Dev Jain <dev.jain@arm.com>
> >> Signed-off-by: Shivank Garg <shivankg@amd.com>
> >> ---
> >>  mm/khugepaged.c | 9 ++++-----
> >>  1 file changed, 4 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> >> index 9f790ec34400..c18d2ce639b1 100644
> >> --- a/mm/khugepaged.c
> >> +++ b/mm/khugepaged.c
> >> @@ -829,10 +829,6 @@ static void khugepaged_alloc_sleep(void)
> >>  	remove_wait_queue(&khugepaged_wait, &wait);
> >>  }
> >>
> >> -static struct collapse_control khugepaged_collapse_control = {
> >> -	.is_khugepaged = true,
> >> -};
> >> -
> >>  static bool hpage_collapse_scan_abort(int nid, struct collapse_control *cc)
> >>  {
> >>  	int i;
> >> @@ -2629,13 +2625,16 @@ static void khugepaged_wait_work(void)
> >>
> >>  static int khugepaged(void *none)
> >>  {
> >> +	struct collapse_control cc = {
> >> +		.is_khugepaged = true,
> >> +	};
> >>  	struct mm_slot *slot;
> >>
> >>  	set_freezable();
> >>  	set_user_nice(current, MAX_NICE);
> >>
> >>  	while (!kthread_should_stop()) {
> >> -		khugepaged_do_scan(&khugepaged_collapse_control);
> >> +		khugepaged_do_scan(&cc);
> >>  		khugepaged_wait_work();
> >>  	}
> >>
> >> --
> >> 2.43.0
> >>
> >>
> >>
> >>
> > Andrew's already commented but this is terribly mistaken.
> >
> > The argument against it (why did nobody check...) is that this struct is HUGE
> > and there's really no benefit to doing this.
> >
> > Nico's series makes this struct even bigger (...!)
> >
> > Dev - PLEASE use pahole or sizeof(...) or something before suggesting moving
> > things like this on to the stack, in future e.g.:
> >
> > $ pahole collapse_control
> > struct collapse_control {
> > 	bool                       is_khugepaged;        /*     0     1 */
> >
> > 	/* XXX 3 bytes hole, try to pack */
> >
> > 	u32                        node_load[1024];      /*     4  4096 */
> >
> > 	/* XXX 4 bytes hole, try to pack */
> >
> > 	/* --- cacheline 64 boundary (4096 bytes) was 8 bytes ago --- */
> > 	nodemask_t                 alloc_nmask;          /*  4104   128 */
> >
> > 	/* size: 4232, cachelines: 67, members: 3 */
> > 	/* sum members: 4225, holes: 2, sum holes: 7 */
> > 	/* last cacheline: 8 bytes */
> > };
> >
> > Making this static was fine. Leave it as-is.
>
> I wasn't suggesting that! When I said
>
> "In madvise_collapse, we defined cc as a local variable and set .is_khugepaged = false. The
> same can be done in int khugepaged() - define a local variable and set .is_khugepaged = true."

Yeah I would suggest more precision in your language in future :) 'as a local
variable' and set . not -> is_khugepaged true... I can see why Shivank
interpreted it at as a stack variable.

>
> madvise_collapse does kmalloc() to allocate this large struct. I was suggesting to do the
> same for khugepaged, to enforce consistency.

As I said in reply to Andrew, NAK to the kmalloc idea too.

This consistency argument is nonsense, madvise_collapse() does that because
_there can be multiple instances_ of the cc around for different processes, you
literally _have_ to kmalloc there.

For khugepaged this isn't the case. We're good as we are.

Thanks, Lorenzo

On 1/24/2026 5:10 PM, Lorenzo Stoakes wrote:
> On Sat, Jan 24, 2026 at 04:24:24PM +0530, Dev Jain wrote:
>>
>> On 24/01/26 2:31 pm, Lorenzo Stoakes wrote:
>>> NAK to this change....
>>>
>>> On Fri, Jan 23, 2026 at 03:03:58PM +0530, Garg, Shivank wrote:
>>>>
>>>> On 1/23/2026 1:18 PM, Dev Jain wrote:
>>>>> On 22/01/26 2:58 pm, Dev Jain wrote:
>>>>>> On 19/01/26 12:53 am, Shivank Garg wrote:
>>>>>>> The global variable 'khugepaged_collapse_control' is not used outside of
>>>>>>> mm/khugepaged.c. Make it static to limit its scope.
>>>>>>>
>>>>>>> Reviewed-by: Wei Yang <richard.weiyang@gmail.com>
>>>>>>> Reviewed-by: Zi Yan <ziy@nvidia.com>
>>>>>>> Acked-by: David Hildenbrand (Red Hat) <david@kernel.org>
>>>>>>> Reviewed-by: Anshuman Khandual <anshuman.khandual@arm.com>
>>>>>>> Signed-off-by: Shivank Garg <shivankg@amd.com>
>>>>>>> ---
>>>>>>>  mm/khugepaged.c | 2 +-
>>>>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>>>
>>>>>>> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
>>>>>>> index 1667abae6d8d..fba6aea5bea6 100644
>>>>>>> --- a/mm/khugepaged.c
>>>>>>> +++ b/mm/khugepaged.c
>>>>>>> @@ -827,7 +827,7 @@ static void khugepaged_alloc_sleep(void)
>>>>>>>  	remove_wait_queue(&khugepaged_wait, &wait);
>>>>>>>  }
>>>>>>>
>>>>>>> -struct collapse_control khugepaged_collapse_control = {
>>>>>>> +static struct collapse_control khugepaged_collapse_control = {
>>>>>>>  	.is_khugepaged = true,
>>>>>>>  };
>>>>>>>
>>>>>> Will it not be better to just remove this variable? In madvise_collapse,
>>>>>> we defined cc as a local variable and set .is_khugepaged = false. The
>>>>>> same can be done in int khugepaged() - define a local variable and set
>>>>>> .is_khugepaged = true.
>>>>> Since this patch has been stabilized already by 4 R-bs, it may be a headache
>>>>> to now remove this, we can do my suggestion later.
>>>>>
>>>>> Reviewed-by: Dev Jain <dev.jain@arm.com>
>>>>>
>>>>>>
>>>> Thank you Dev for the feedback and review.
>>>>
>>>> I've attached the patch implementing your suggestion and sending this as a separate
>>>> follow-up to avoid disrupting the current series.
>>>>
>>>> I’m happy to queue it for next cycle or if it’s acceptable now, please take it.
>>>>
>>>> Thanks for the suggestion!
>>>>
>>>> Regards,
>>>> Shivank
>>>>
>>>> ---
>>>> From: Shivank Garg <shivankg@amd.com>
>>>> Date: Thu, 22 Jan 2026 12:36:28 +0000
>>>> Subject: [PATCH] mm/khugepaged: convert khugepaged_collapse_control to local
>>>>  variable in khugepaged()
>>>>
>>>> Make khugepaged_collapse_control a local variable in khugepaged() instead
>>>> of static global, consistent with how madvise_collapse() handles its
>>>> collapse_control. Static storage is unnecessary here as node_load and
>>>> alloc_nmask are reset per-VMA during scanning.
>>>>
>>>> No functional change.
>>>>
>>>> Suggested-by: Dev Jain <dev.jain@arm.com>
>>>> Signed-off-by: Shivank Garg <shivankg@amd.com>
>>>> ---
>>>>  mm/khugepaged.c | 9 ++++-----
>>>>  1 file changed, 4 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
>>>> index 9f790ec34400..c18d2ce639b1 100644
>>>> --- a/mm/khugepaged.c
>>>> +++ b/mm/khugepaged.c
>>>> @@ -829,10 +829,6 @@ static void khugepaged_alloc_sleep(void)
>>>>  	remove_wait_queue(&khugepaged_wait, &wait);
>>>>  }
>>>>
>>>> -static struct collapse_control khugepaged_collapse_control = {
>>>> -	.is_khugepaged = true,
>>>> -};
>>>> -
>>>>  static bool hpage_collapse_scan_abort(int nid, struct collapse_control *cc)
>>>>  {
>>>>  	int i;
>>>> @@ -2629,13 +2625,16 @@ static void khugepaged_wait_work(void)
>>>>
>>>>  static int khugepaged(void *none)
>>>>  {
>>>> +	struct collapse_control cc = {
>>>> +		.is_khugepaged = true,
>>>> +	};
>>>>  	struct mm_slot *slot;
>>>>
>>>>  	set_freezable();
>>>>  	set_user_nice(current, MAX_NICE);
>>>>
>>>>  	while (!kthread_should_stop()) {
>>>> -		khugepaged_do_scan(&khugepaged_collapse_control);
>>>> +		khugepaged_do_scan(&cc);
>>>>  		khugepaged_wait_work();
>>>>  	}
>>>>
>>>> --
>>>> 2.43.0
>>>>
>>>>
>>>>
>>>>
>>> Andrew's already commented but this is terribly mistaken.
>>>
>>> The argument against it (why did nobody check...) is that this struct is HUGE
>>> and there's really no benefit to doing this.
>>>
>>> Nico's series makes this struct even bigger (...!)
>>>
>>> Dev - PLEASE use pahole or sizeof(...) or something before suggesting moving
>>> things like this on to the stack, in future e.g.:
>>>
>>> $ pahole collapse_control
>>> struct collapse_control {
>>> 	bool                       is_khugepaged;        /*     0     1 */
>>>
>>> 	/* XXX 3 bytes hole, try to pack */
>>>
>>> 	u32                        node_load[1024];      /*     4  4096 */
>>>
>>> 	/* XXX 4 bytes hole, try to pack */
>>>
>>> 	/* --- cacheline 64 boundary (4096 bytes) was 8 bytes ago --- */
>>> 	nodemask_t                 alloc_nmask;          /*  4104   128 */
>>>
>>> 	/* size: 4232, cachelines: 67, members: 3 */
>>> 	/* sum members: 4225, holes: 2, sum holes: 7 */
>>> 	/* last cacheline: 8 bytes */
>>> };
>>>
>>> Making this static was fine. Leave it as-is.
>>
>> I wasn't suggesting that! When I said
>>
>> "In madvise_collapse, we defined cc as a local variable and set .is_khugepaged = false. The
>> same can be done in int khugepaged() - define a local variable and set .is_khugepaged = true."
> 
> Yeah I would suggest more precision in your language in future :) 'as a local
> variable' and set . not -> is_khugepaged true... I can see why Shivank
> interpreted it at as a stack variable.
> 
>>
>> madvise_collapse does kmalloc() to allocate this large struct. I was suggesting to do the
>> same for khugepaged, to enforce consistency.
> 
> As I said in reply to Andrew, NAK to the kmalloc idea too.
> 
> This consistency argument is nonsense, madvise_collapse() does that because
> _there can be multiple instances_ of the cc around for different processes, you
> literally _have_ to kmalloc there.
> 
> For khugepaged this isn't the case. We're good as we are.
> 

Thank you Lorenzo for explanation on frame-size/pahole and single-instance vs multi-instance pattern.

This was a valuable discussion and I learned a lot from it :)

Best Regards,
Shivank

On 24/01/26 5:10 pm, Lorenzo Stoakes wrote:
> On Sat, Jan 24, 2026 at 04:24:24PM +0530, Dev Jain wrote:
>> On 24/01/26 2:31 pm, Lorenzo Stoakes wrote:
>>> NAK to this change....
>>>
>>> On Fri, Jan 23, 2026 at 03:03:58PM +0530, Garg, Shivank wrote:
>>>> On 1/23/2026 1:18 PM, Dev Jain wrote:
>>>>> On 22/01/26 2:58 pm, Dev Jain wrote:
>>>>>> On 19/01/26 12:53 am, Shivank Garg wrote:
>>>>>>> The global variable 'khugepaged_collapse_control' is not used outside of
>>>>>>> mm/khugepaged.c. Make it static to limit its scope.
>>>>>>>
>>>>>>> Reviewed-by: Wei Yang <richard.weiyang@gmail.com>
>>>>>>> Reviewed-by: Zi Yan <ziy@nvidia.com>
>>>>>>> Acked-by: David Hildenbrand (Red Hat) <david@kernel.org>
>>>>>>> Reviewed-by: Anshuman Khandual <anshuman.khandual@arm.com>
>>>>>>> Signed-off-by: Shivank Garg <shivankg@amd.com>
>>>>>>> ---
>>>>>>>  mm/khugepaged.c | 2 +-
>>>>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>>>
>>>>>>> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
>>>>>>> index 1667abae6d8d..fba6aea5bea6 100644
>>>>>>> --- a/mm/khugepaged.c
>>>>>>> +++ b/mm/khugepaged.c
>>>>>>> @@ -827,7 +827,7 @@ static void khugepaged_alloc_sleep(void)
>>>>>>>  	remove_wait_queue(&khugepaged_wait, &wait);
>>>>>>>  }
>>>>>>>
>>>>>>> -struct collapse_control khugepaged_collapse_control = {
>>>>>>> +static struct collapse_control khugepaged_collapse_control = {
>>>>>>>  	.is_khugepaged = true,
>>>>>>>  };
>>>>>>>
>>>>>> Will it not be better to just remove this variable? In madvise_collapse,
>>>>>> we defined cc as a local variable and set .is_khugepaged = false. The
>>>>>> same can be done in int khugepaged() - define a local variable and set
>>>>>> .is_khugepaged = true.
>>>>> Since this patch has been stabilized already by 4 R-bs, it may be a headache
>>>>> to now remove this, we can do my suggestion later.
>>>>>
>>>>> Reviewed-by: Dev Jain <dev.jain@arm.com>
>>>>>
>>>> Thank you Dev for the feedback and review.
>>>>
>>>> I've attached the patch implementing your suggestion and sending this as a separate
>>>> follow-up to avoid disrupting the current series.
>>>>
>>>> I’m happy to queue it for next cycle or if it’s acceptable now, please take it.
>>>>
>>>> Thanks for the suggestion!
>>>>
>>>> Regards,
>>>> Shivank
>>>>
>>>> ---
>>>> From: Shivank Garg <shivankg@amd.com>
>>>> Date: Thu, 22 Jan 2026 12:36:28 +0000
>>>> Subject: [PATCH] mm/khugepaged: convert khugepaged_collapse_control to local
>>>>  variable in khugepaged()
>>>>
>>>> Make khugepaged_collapse_control a local variable in khugepaged() instead
>>>> of static global, consistent with how madvise_collapse() handles its
>>>> collapse_control. Static storage is unnecessary here as node_load and
>>>> alloc_nmask are reset per-VMA during scanning.
>>>>
>>>> No functional change.
>>>>
>>>> Suggested-by: Dev Jain <dev.jain@arm.com>
>>>> Signed-off-by: Shivank Garg <shivankg@amd.com>
>>>> ---
>>>>  mm/khugepaged.c | 9 ++++-----
>>>>  1 file changed, 4 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
>>>> index 9f790ec34400..c18d2ce639b1 100644
>>>> --- a/mm/khugepaged.c
>>>> +++ b/mm/khugepaged.c
>>>> @@ -829,10 +829,6 @@ static void khugepaged_alloc_sleep(void)
>>>>  	remove_wait_queue(&khugepaged_wait, &wait);
>>>>  }
>>>>
>>>> -static struct collapse_control khugepaged_collapse_control = {
>>>> -	.is_khugepaged = true,
>>>> -};
>>>> -
>>>>  static bool hpage_collapse_scan_abort(int nid, struct collapse_control *cc)
>>>>  {
>>>>  	int i;
>>>> @@ -2629,13 +2625,16 @@ static void khugepaged_wait_work(void)
>>>>
>>>>  static int khugepaged(void *none)
>>>>  {
>>>> +	struct collapse_control cc = {
>>>> +		.is_khugepaged = true,
>>>> +	};
>>>>  	struct mm_slot *slot;
>>>>
>>>>  	set_freezable();
>>>>  	set_user_nice(current, MAX_NICE);
>>>>
>>>>  	while (!kthread_should_stop()) {
>>>> -		khugepaged_do_scan(&khugepaged_collapse_control);
>>>> +		khugepaged_do_scan(&cc);
>>>>  		khugepaged_wait_work();
>>>>  	}
>>>>
>>>> --
>>>> 2.43.0
>>>>
>>>>
>>>>
>>>>
>>> Andrew's already commented but this is terribly mistaken.
>>>
>>> The argument against it (why did nobody check...) is that this struct is HUGE
>>> and there's really no benefit to doing this.
>>>
>>> Nico's series makes this struct even bigger (...!)
>>>
>>> Dev - PLEASE use pahole or sizeof(...) or something before suggesting moving
>>> things like this on to the stack, in future e.g.:
>>>
>>> $ pahole collapse_control
>>> struct collapse_control {
>>> 	bool                       is_khugepaged;        /*     0     1 */
>>>
>>> 	/* XXX 3 bytes hole, try to pack */
>>>
>>> 	u32                        node_load[1024];      /*     4  4096 */
>>>
>>> 	/* XXX 4 bytes hole, try to pack */
>>>
>>> 	/* --- cacheline 64 boundary (4096 bytes) was 8 bytes ago --- */
>>> 	nodemask_t                 alloc_nmask;          /*  4104   128 */
>>>
>>> 	/* size: 4232, cachelines: 67, members: 3 */
>>> 	/* sum members: 4225, holes: 2, sum holes: 7 */
>>> 	/* last cacheline: 8 bytes */
>>> };
>>>
>>> Making this static was fine. Leave it as-is.
>> I wasn't suggesting that! When I said
>>
>> "In madvise_collapse, we defined cc as a local variable and set .is_khugepaged = false. The
>> same can be done in int khugepaged() - define a local variable and set .is_khugepaged = true."
> Yeah I would suggest more precision in your language in future :) 'as a local
> variable' and set . not -> is_khugepaged true... I can see why Shivank
> interpreted it at as a stack variable.
>
>> madvise_collapse does kmalloc() to allocate this large struct. I was suggesting to do the
>> same for khugepaged, to enforce consistency.
> As I said in reply to Andrew, NAK to the kmalloc idea too.
>
> This consistency argument is nonsense, madvise_collapse() does that because
> _there can be multiple instances_ of the cc around for different processes, you
> literally _have_ to kmalloc there.

Ah yes nice observation : ) Thanks.

>
> For khugepaged this isn't the case. We're good as we are.
>
> Thanks, Lorenzo

On Fri, 23 Jan 2026 15:03:58 +0530 "Garg, Shivank" <shivankg@amd.com> wrote:

> --- a/mm/khugepaged.c
> +++ b/mm/khugepaged.c
> @@ -829,10 +829,6 @@ static void khugepaged_alloc_sleep(void)
>  	remove_wait_queue(&khugepaged_wait, &wait);
>  }
>  
> -static struct collapse_control khugepaged_collapse_control = {
> -	.is_khugepaged = true,
> -};
> -
>  static bool hpage_collapse_scan_abort(int nid, struct collapse_control *cc)
>  {
>  	int i;
> @@ -2629,13 +2625,16 @@ static void khugepaged_wait_work(void)
>  
>  static int khugepaged(void *none)
>  {
> +	struct collapse_control cc = {
> +		.is_khugepaged = true,
> +	};
>  	struct mm_slot *slot;
>  
>  	set_freezable();
>  	set_user_nice(current, MAX_NICE);
>  
>  	while (!kthread_should_stop()) {
> -		khugepaged_do_scan(&khugepaged_collapse_control);
> +		khugepaged_do_scan(&cc);
>  		khugepaged_wait_work();
>  	}
>  

lol, OK, I droppped $Subject and did this:

From: "Garg, Shivank" <shivankg@amd.com>
Subject: mm/khugepaged: make khugepaged_collapse_control a local
Date: Fri, 23 Jan 2026 15:03:58 +0530

Make this collapse_control instance local to the only function which uses
it.

Link: https://lkml.kernel.org/r/ba4502f7-0c35-460e-a42c-d32dea9ab9eb@amd.com
Signed-off-by: Shivank Garg <shivankg@amd.com>
Cc: Anshuman Khandual <anshuman.khandual@arm.com>
Cc: Baolin Wang <baolin.wang@linux.alibaba.com>
Cc: Barry Song <baohua@kernel.org>
Cc: David Hildenbrand (Red Hat) <david@kernel.org>
Cc: Dev Jain <dev.jain@arm.com>
Cc: Lance Yang <lance.yang@linux.dev>
Cc: Liam R. Howlett <Liam.Howlett@oracle.com>
Cc: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
Cc: Nico Pache <npache@redhat.com>
Cc: Ryan Roberts <ryan.roberts@arm.com>
Cc: Wei Yang <richard.weiyang@gmail.com>
Cc: Zi Yan <ziy@nvidia.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 mm/khugepaged.c |    9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

--- a/mm/khugepaged.c~mm-khugepaged-make-khugepaged_collapse_control-a-local
+++ a/mm/khugepaged.c
@@ -827,10 +827,6 @@ static void khugepaged_alloc_sleep(void)
 	remove_wait_queue(&khugepaged_wait, &wait);
 }
 
-struct collapse_control khugepaged_collapse_control = {
-	.is_khugepaged = true,
-};
-
 static bool hpage_collapse_scan_abort(int nid, struct collapse_control *cc)
 {
 	int i;
@@ -2618,13 +2614,16 @@ static void khugepaged_wait_work(void)
 
 static int khugepaged(void *none)
 {
+	struct collapse_control cc = {
+		.is_khugepaged = true,
+	};
 	struct mm_slot *slot;
 
 	set_freezable();
 	set_user_nice(current, MAX_NICE);
 
 	while (!kthread_should_stop()) {
-		khugepaged_do_scan(&khugepaged_collapse_control);
+		khugepaged_do_scan(&cc);
 		khugepaged_wait_work();
 	}
 
_

On Fri, 23 Jan 2026 17:21:42 -0800 Andrew Morton <akpm@linux-foundation.org> wrote:

> 
> lol, OK, I droppped $Subject and did this:
> 
> --- a/mm/khugepaged.c~mm-khugepaged-make-khugepaged_collapse_control-a-local
> +++ a/mm/khugepaged.c
> @@ -827,10 +827,6 @@ static void khugepaged_alloc_sleep(void)
>  	remove_wait_queue(&khugepaged_wait, &wait);
>  }
>  
> -struct collapse_control khugepaged_collapse_control = {
> -	.is_khugepaged = true,
> -};
> -
>  static bool hpage_collapse_scan_abort(int nid, struct collapse_control *cc)
>  {
>  	int i;
> @@ -2618,13 +2614,16 @@ static void khugepaged_wait_work(void)
>  
>  static int khugepaged(void *none)
>  {
> +	struct collapse_control cc = {
> +		.is_khugepaged = true,
> +	};

nope, that thing's really big!

mm/khugepaged.c: In function 'khugepaged':
mm/khugepaged.c:2637:1: error: the frame size of 4696 bytes is larger than 2048 

We could kzalloc() it but I think I'll just restore the make-it-static
patch.  Someone please add to the todo list?

On Fri, Jan 23, 2026 at 07:02:13PM -0800, Andrew Morton wrote:
> On Fri, 23 Jan 2026 17:21:42 -0800 Andrew Morton <akpm@linux-foundation.org> wrote:
>
> >
> > lol, OK, I droppped $Subject and did this:
> >
> > --- a/mm/khugepaged.c~mm-khugepaged-make-khugepaged_collapse_control-a-local
> > +++ a/mm/khugepaged.c
> > @@ -827,10 +827,6 @@ static void khugepaged_alloc_sleep(void)
> >  	remove_wait_queue(&khugepaged_wait, &wait);
> >  }
> >
> > -struct collapse_control khugepaged_collapse_control = {
> > -	.is_khugepaged = true,
> > -};
> > -
> >  static bool hpage_collapse_scan_abort(int nid, struct collapse_control *cc)
> >  {
> >  	int i;
> > @@ -2618,13 +2614,16 @@ static void khugepaged_wait_work(void)
> >
> >  static int khugepaged(void *none)
> >  {
> > +	struct collapse_control cc = {
> > +		.is_khugepaged = true,
> > +	};
>
> nope, that thing's really big!
>
> mm/khugepaged.c: In function 'khugepaged':
> mm/khugepaged.c:2637:1: error: the frame size of 4696 bytes is larger than 2048

Yup, thanks Andrew, this change is completely crazy.

>
> We could kzalloc() it but I think I'll just restore the make-it-static
> patch.  Someone please add to the todo list?

And no to kzalloc(), let's just keep this static for now. It works fine as it is!

Thanks, Lorenzo