[PATCH] refperf: Remove redundant kfree() after torture_stop_kthread()

[PATCH] refperf: Remove redundant kfree() after torture_stop_kthread()

[Kaushlendra Kumar]

Remove unnecessary kfree(main_task) call in ref_scale_cleanup() as
torture_stop_kthread() already handles the memory cleanup for the
task structure internally.

The torture_stop_kthread() function is designed to stop the kernel
thread and free the associated task structure as part of its cleanup
process. The additional kfree(main_task) call after torture_stop_kthread()
results in a double-free condition.

This pattern is consistent with other torture test modules where
torture_stop_kthread() is called without explicit kfree() of the
task pointer, as the torture framework manages the task lifecycle
internally.

Signed-off-by: Kaushlendra Kumar <kaushlendra.kumar@intel.com>
---
 kernel/rcu/refscale.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/kernel/rcu/refscale.c b/kernel/rcu/refscale.c
index 2c2648a3ad30..2bfa987f4ba9 100644
--- a/kernel/rcu/refscale.c
+++ b/kernel/rcu/refscale.c
@@ -1050,7 +1050,6 @@ ref_scale_cleanup(void)
 	kfree(reader_tasks);
 
 	torture_stop_kthread("main_task", main_task);
-	kfree(main_task);
 
 	// Do scale-type-specific cleanup operations.
 	if (cur_ops->cleanup != NULL)
-- 
2.34.1

Re: [PATCH] refperf: Remove redundant kfree() after torture_stop_kthread()

[Paul E. McKenney]

On Tue, Sep 09, 2025 at 11:11:48AM +0530, Kaushlendra Kumar wrote:
> Remove unnecessary kfree(main_task) call in ref_scale_cleanup() as
> torture_stop_kthread() already handles the memory cleanup for the
> task structure internally.

Again, thank you for your interest in Linux-kernel RCU!

And you are right that the kfree(main_task) is redundant, especially given
that the main_task pointer will be set to NULL in _torture_stop_kthread(),
so that this kfree() is just a confusing no-op.

> The torture_stop_kthread() function is designed to stop the kernel
> thread and free the associated task structure as part of its cleanup
> process. The additional kfree(main_task) call after torture_stop_kthread()
> results in a double-free condition.

But this last sentence needs adjustment.  Because _torture_stop_kthread()
NULLs the pointer, the second kfree() gets a NULL pointer, thus not doing
a duplicate free, right?

> This pattern is consistent with other torture test modules where
> torture_stop_kthread() is called without explicit kfree() of the
> task pointer, as the torture framework manages the task lifecycle
> internally.
> 
> Signed-off-by: Kaushlendra Kumar <kaushlendra.kumar@intel.com>
> ---
>  kernel/rcu/refscale.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/kernel/rcu/refscale.c b/kernel/rcu/refscale.c
> index 2c2648a3ad30..2bfa987f4ba9 100644
> --- a/kernel/rcu/refscale.c
> +++ b/kernel/rcu/refscale.c
> @@ -1050,7 +1050,6 @@ ref_scale_cleanup(void)
>  	kfree(reader_tasks);

For ease of debugging with kdgb, drgn, and other tools, it would also
be good to set reader_tasks to NULL.  Please feel free to do that,
either as part of this patch or as a separate patch.

							Thanx, Paul

>  	torture_stop_kthread("main_task", main_task);
> -	kfree(main_task);
>  
>  	// Do scale-type-specific cleanup operations.
>  	if (cur_ops->cleanup != NULL)
> -- 
> 2.34.1
> 

RE: [PATCH] refperf: Remove redundant kfree() after torture_stop_kthread()

[Kumar, Kaushlendra]

Hi Paul,

On Tue, Sep 09, 2025 at 11:11:48AM +0530, Paul E. McKenney <paulmck@kernel.org> wrote:
> On Tue, Sep 09, 2025 at 11:11:48AM +0530, Kaushlendra Kumar wrote:
> > Remove unnecessary kfree(main_task) call in ref_scale_cleanup() as
> > torture_stop_kthread() already handles the memory cleanup for the task 
> > structure internally.
> 
> Again, thank you for your interest in Linux-kernel RCU!
> 
> And you are right that the kfree(main_task) is redundant, especially given that the main_task pointer will be set to NULL in _torture_stop_kthread(), so that this kfree() is just a confusing no-op.
> 
> > The torture_stop_kthread() function is designed to stop the kernel 
> > thread and free the associated task structure as part of its cleanup 
> > process. The additional kfree(main_task) call after 
> > torture_stop_kthread() results in a double-free condition.
> 
> But this last sentence needs adjustment.  Because _torture_stop_kthread() NULLs the pointer, the second kfree() gets a NULL pointer, thus not doing a duplicate free, right?

You're absolutely correct! Thank you for the clarification.
.I have sent a v2 patch with the corrected commit message.

> For ease of debugging with kdgb, drgn, and other tools, it would also be good to set reader_tasks to NULL.  Please feel free to do that, either as part of this patch or as a separate patch.

I'll handle the reader_tasks = NULL assignment as a separate patch to
keep this one focused on removing the redundant kfree().

Thank you for the review!

Best regards,
Kaushlendra Kumar