2023-10-07 11:36:02

by Zqiang

Subject: [PATCH] workqueue: Use the kmem_cache_free() instead of kfree() to release pwq

The pwq objects is allocated by kmem_cache_alloc(), this commit therefore
use kmem_cache_free() instead of kfree() to release pwq objects and also
make use the correct tracepoint("trace_kmem_cache_free") to trace the
release of pwq.

Signed-off-by: Zqiang <[email protected]>
---
kernel/workqueue.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index ebe24a5e1435..6f74cab2bd5a 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -4610,8 +4610,12 @@ static int alloc_and_link_pwqs(struct workqueue_struct *wq)

enomem:
if (wq->cpu_pwq) {
- for_each_possible_cpu(cpu)
- kfree(*per_cpu_ptr(wq->cpu_pwq, cpu));
+ for_each_possible_cpu(cpu) {
+ struct pool_workqueue *pwq = *per_cpu_ptr(wq->cpu_pwq, cpu);
+
+ if (pwq)
+ kmem_cache_free(pwq_cache, pwq);
+ }
free_percpu(wq->cpu_pwq);
wq->cpu_pwq = NULL;
}
--
2.17.1

2023-10-09 16:46:35

by Tejun Heo

Subject: Re: [PATCH] workqueue: Use the kmem_cache_free() instead of kfree() to release pwq

On Sat, Oct 07, 2023 at 07:35:41PM +0800, Zqiang wrote:
> The pwq objects is allocated by kmem_cache_alloc(), this commit therefore
> use kmem_cache_free() instead of kfree() to release pwq objects and also
> make use the correct tracepoint("trace_kmem_cache_free") to trace the
> release of pwq.

This isn't wrong. kfree() can be used for memory allocated with
kmem_cache_alloc().

Thanks.

--
tejun

2023-10-10 02:53:49

by Zqiang

Subject: Re: [PATCH] workqueue: Use the kmem_cache_free() instead of kfree() to release pwq

>
> On Sat, Oct 07, 2023 at 07:35:41PM +0800, Zqiang wrote:
> > The pwq objects is allocated by kmem_cache_alloc(), this commit therefore
> > use kmem_cache_free() instead of kfree() to release pwq objects and also
> > make use the correct tracepoint("trace_kmem_cache_free") to trace the
> > release of pwq.
>
> This isn't wrong. kfree() can be used for memory allocated with
> kmem_cache_alloc().
>

Yes, that's not wrong. but pwq is allocated by kmem_cache_alloc(),
usually should use kmem_cache_free() to release, correspondingly, we can
use 'trace_kmem_cache_alloc/trace_kmem_cache_free' to track, not using
'trace_kmem_cache_alloc/trace_kfree'.
And in rcu_free_pwq(), we use kmem_cache_free() to free pwq.

Thanks
Zqiang


>
> Thanks.
>
> --
> tejun

2023-10-10 18:47:14

by Tejun Heo

Subject: Re: [PATCH] workqueue: Use the kmem_cache_free() instead of kfree() to release pwq

On Tue, Oct 10, 2023 at 10:53:31AM +0800, Z qiang wrote:
> >
> > On Sat, Oct 07, 2023 at 07:35:41PM +0800, Zqiang wrote:
> > > The pwq objects is allocated by kmem_cache_alloc(), this commit therefore
> > > use kmem_cache_free() instead of kfree() to release pwq objects and also
> > > make use the correct tracepoint("trace_kmem_cache_free") to trace the
> > > release of pwq.
> >
> > This isn't wrong. kfree() can be used for memory allocated with
> > kmem_cache_alloc().
> >
>
> Yes, that's not wrong. but pwq is allocated by kmem_cache_alloc(),
> usually should use kmem_cache_free() to release, correspondingly, we can
> use 'trace_kmem_cache_alloc/trace_kmem_cache_free' to track, not using
> 'trace_kmem_cache_alloc/trace_kfree'.
> And in rcu_free_pwq(), we use kmem_cache_free() to free pwq.

Can you please update the patch description to clarify that the code is
currently not broken but it's trying to make it nicer by unifying how
they're freed?

Thanks.

--
tejun