* [PATCH] kernel/workqueue.c: kfree issue, need check flag 'WQ_UNBOUND' when processing failure.
@ 2013-05-14 13:06 Chen Gang
2013-05-14 15:20 ` Tejun Heo
0 siblings, 1 reply; 5+ messages in thread
From: Chen Gang @ 2013-05-14 13:06 UTC (permalink / raw)
To: tj; +Cc: linux-kernel
When failure occurs in __alloc_workqueue_key(), if 'flags' has
'WQ_UNBOUND', the destroy_workqueue() will not release 'wq'.
In this situation, we need release it, or will cause memory leak.
And also simplify the related code.
Signed-off-by: Chen Gang <gang.chen@asianux.com>
---
kernel/workqueue.c | 9 +++++----
1 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index 1ae6028..bf644d4 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -4167,12 +4167,13 @@ struct workqueue_struct *__alloc_workqueue_key(const char *fmt,
return wq;
-err_free_wq:
- free_workqueue_attrs(wq->unbound_attrs);
- kfree(wq);
- return NULL;
err_destroy:
destroy_workqueue(wq);
+ if (flags & WQ_UNBOUND) {
+err_free_wq:
+ free_workqueue_attrs(wq->unbound_attrs);
+ kfree(wq);
+ }
return NULL;
}
EXPORT_SYMBOL_GPL(__alloc_workqueue_key);
--
1.7.7.6
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] kernel/workqueue.c: kfree issue, need check flag 'WQ_UNBOUND' when processing failure.
2013-05-14 13:06 [PATCH] kernel/workqueue.c: kfree issue, need check flag 'WQ_UNBOUND' when processing failure Chen Gang
@ 2013-05-14 15:20 ` Tejun Heo
2013-05-15 1:34 ` Chen Gang
0 siblings, 1 reply; 5+ messages in thread
From: Tejun Heo @ 2013-05-14 15:20 UTC (permalink / raw)
To: Chen Gang; +Cc: linux-kernel
On Tue, May 14, 2013 at 09:06:40PM +0800, Chen Gang wrote:
> err_destroy:
> destroy_workqueue(wq);
> + if (flags & WQ_UNBOUND) {
> +err_free_wq:
> + free_workqueue_attrs(wq->unbound_attrs);
> + kfree(wq);
> + }
Doesn't the above make the code free wq twice on after err_destroy?
--
tejun
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] kernel/workqueue.c: kfree issue, need check flag 'WQ_UNBOUND' when processing failure.
2013-05-14 15:20 ` Tejun Heo
@ 2013-05-15 1:34 ` Chen Gang
2013-05-15 21:11 ` Tejun Heo
0 siblings, 1 reply; 5+ messages in thread
From: Chen Gang @ 2013-05-15 1:34 UTC (permalink / raw)
To: Tejun Heo; +Cc: linux-kernel
On 05/14/2013 11:20 PM, Tejun Heo wrote:
> On Tue, May 14, 2013 at 09:06:40PM +0800, Chen Gang wrote:
>> err_destroy:
>> destroy_workqueue(wq);
>> + if (flags & WQ_UNBOUND) {
>> +err_free_wq:
>> + free_workqueue_attrs(wq->unbound_attrs);
>> + kfree(wq);
>> + }
>
> Doesn't the above make the code free wq twice on after err_destroy?
>
Oh, it is my fault. I did not see the put_pwq_unlocked() in details,
next I should read the code carefully.
Thanks.
--
Chen Gang
Asianux Corporation
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] kernel/workqueue.c: kfree issue, need check flag 'WQ_UNBOUND' when processing failure.
2013-05-15 1:34 ` Chen Gang
@ 2013-05-15 21:11 ` Tejun Heo
2013-05-16 2:04 ` Chen Gang
0 siblings, 1 reply; 5+ messages in thread
From: Tejun Heo @ 2013-05-15 21:11 UTC (permalink / raw)
To: Chen Gang; +Cc: linux-kernel
On Wed, May 15, 2013 at 09:34:36AM +0800, Chen Gang wrote:
> On 05/14/2013 11:20 PM, Tejun Heo wrote:
> > On Tue, May 14, 2013 at 09:06:40PM +0800, Chen Gang wrote:
> >> err_destroy:
> >> destroy_workqueue(wq);
> >> + if (flags & WQ_UNBOUND) {
> >> +err_free_wq:
> >> + free_workqueue_attrs(wq->unbound_attrs);
> >> + kfree(wq);
> >> + }
> >
> > Doesn't the above make the code free wq twice on after err_destroy?
> >
>
> Oh, it is my fault. I did not see the put_pwq_unlocked() in details,
> next I should read the code carefully.
We're still leaking unbound_attrs in the failure path, right? We can
probably just add unconditional free_workqueue_attrs() in err_free_wq?
Thanks.
--
tejun
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] kernel/workqueue.c: kfree issue, need check flag 'WQ_UNBOUND' when processing failure.
2013-05-15 21:11 ` Tejun Heo
@ 2013-05-16 2:04 ` Chen Gang
0 siblings, 0 replies; 5+ messages in thread
From: Chen Gang @ 2013-05-16 2:04 UTC (permalink / raw)
To: Tejun Heo; +Cc: linux-kernel
On 05/16/2013 05:11 AM, Tejun Heo wrote:
> On Wed, May 15, 2013 at 09:34:36AM +0800, Chen Gang wrote:
>> On 05/14/2013 11:20 PM, Tejun Heo wrote:
>>> On Tue, May 14, 2013 at 09:06:40PM +0800, Chen Gang wrote:
>>>> err_destroy:
>>>> destroy_workqueue(wq);
>>>> + if (flags & WQ_UNBOUND) {
>>>> +err_free_wq:
>>>> + free_workqueue_attrs(wq->unbound_attrs);
>>>> + kfree(wq);
>>>> + }
>>>
>>> Doesn't the above make the code free wq twice on after err_destroy?
>>>
>>
>> Oh, it is my fault. I did not see the put_pwq_unlocked() in details,
>> next I should read the code carefully.
>
> We're still leaking unbound_attrs in the failure path, right? We can
> probably just add unconditional free_workqueue_attrs() in err_free_wq?
>
It seems that it already done, the related patch is provided by you :
"6029a91 workqueue: add workqueue->unbound_attrs".
Please check.
Thanks.
--
Chen Gang
Asianux Corporation
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2013-05-16 2:05 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-05-14 13:06 [PATCH] kernel/workqueue.c: kfree issue, need check flag 'WQ_UNBOUND' when processing failure Chen Gang
2013-05-14 15:20 ` Tejun Heo
2013-05-15 1:34 ` Chen Gang
2013-05-15 21:11 ` Tejun Heo
2013-05-16 2:04 ` Chen Gang
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.