* [PATCH] workqueue: Fix double kfree for rescuer
@ 2020-05-25 7:59 qiang.zhang
2020-05-25 8:20 ` [PATCH v2] workqueue: Delete duplicate kfree() call for “rescuer” from rcu_free_wq() Markus Elfring
2020-05-25 9:50 ` [PATCH v2] workqueue: Fix double kfree for rescuer Markus Elfring
0 siblings, 2 replies; 7+ messages in thread
From: qiang.zhang @ 2020-05-25 7:59 UTC (permalink / raw)
To: tj; +Cc: jiangshanlai, markus.elfring, linux-kernel
From: Zhang Qiang <qiang.zhang@windriver.com>
The duplicate memory release should be deleted from the implementation
of the callback function "rcu_free_wq".
Fixes: 6ba94429c8e7 ("workqueue: Reorder sysfs code")
Signed-off-by: Zhang Qiang <qiang.zhang@windriver.com>
---
kernel/workqueue.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index 891ccad5f271..a2451cdcd503 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -3491,7 +3491,6 @@ static void rcu_free_wq(struct rcu_head *rcu)
else
free_workqueue_attrs(wq->unbound_attrs);
- kfree(wq->rescuer);
kfree(wq);
}
--
2.24.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2] workqueue: Delete duplicate kfree() call for “rescuer” from rcu_free_wq()
2020-05-25 7:59 [PATCH] workqueue: Fix double kfree for rescuer qiang.zhang
@ 2020-05-25 8:20 ` Markus Elfring
2020-05-25 9:50 ` [PATCH v2] workqueue: Fix double kfree for rescuer Markus Elfring
1 sibling, 0 replies; 7+ messages in thread
From: Markus Elfring @ 2020-05-25 8:20 UTC (permalink / raw)
To: Zhang Qiang, Tejun Heo, Lai Jiangshan; +Cc: linux-kernel, kernel-janitors
> The duplicate memory release should be deleted from the implementation
> of the callback function "rcu_free_wq".
I would find it nicer if you would have extended my simple wording suggestion.
1. Mention conditions:
The callback function “rcu_free_wq” could be called after memory was released
for “rescuer” already.
2. Specify the desired action:
Thus delete a misplaced call of the function “kfree”.
3. Please keep the patch versioning consistent.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?id=9cb1fd0efd195590b828b9b865421ad345a4a145#n709
Regards,
Markus
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] workqueue: Fix double kfree for rescuer
2020-05-25 7:59 [PATCH] workqueue: Fix double kfree for rescuer qiang.zhang
2020-05-25 8:20 ` [PATCH v2] workqueue: Delete duplicate kfree() call for “rescuer” from rcu_free_wq() Markus Elfring
@ 2020-05-25 9:50 ` Markus Elfring
2020-05-25 9:55 ` Markus Elfring
2020-05-25 10:19 ` qzhang2
1 sibling, 2 replies; 7+ messages in thread
From: Markus Elfring @ 2020-05-25 9:50 UTC (permalink / raw)
To: Zhang Qiang, Tejun Heo, Lai Jiangshan; +Cc: linux-kernel, kernel-janitors
> The duplicate memory release should be deleted from the implementation
> of the callback function "rcu_free_wq".
I tried to help with the selection of a better commit message.
I have taken another look also at the implementation of the function “destroy_workqueue”.
* The function call “destroy_workqueue” can be performed there in an if branch
after the statement “wq->rescuer = NULL” was executed.
* This data processing is independent from a possible call of the
function “call_rcu(&wq->rcu, rcu_free_wq)” in another if branch.
Thus it seems that a null pointer is intentionally passed by a data structure
member to this callback function on demand.
The corresponding call of the function “kfree” can tolerate this special case.
Now I find that the proposed change can be inappropriate.
Regards,
Markus
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] workqueue: Fix double kfree for rescuer
2020-05-25 9:50 ` [PATCH v2] workqueue: Fix double kfree for rescuer Markus Elfring
@ 2020-05-25 9:55 ` Markus Elfring
2020-05-25 10:19 ` qzhang2
1 sibling, 0 replies; 7+ messages in thread
From: Markus Elfring @ 2020-05-25 9:55 UTC (permalink / raw)
To: Zhang Qiang, Tejun Heo, Lai Jiangshan; +Cc: linux-kernel, kernel-janitors
> * The function call “destroy_workqueue” can be performed there in an if branch
> after the statement “wq->rescuer = NULL” was executed.
Another correction:
* The function call “kfree(rescuer)” can be performed there in an if branch
after the statement “wq->rescuer = NULL” was executed.
Regards,
Markus
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] workqueue: Fix double kfree for rescuer
2020-05-25 9:50 ` [PATCH v2] workqueue: Fix double kfree for rescuer Markus Elfring
2020-05-25 9:55 ` Markus Elfring
@ 2020-05-25 10:19 ` qzhang2
2020-05-25 10:40 ` [v2] " Markus Elfring
1 sibling, 1 reply; 7+ messages in thread
From: qzhang2 @ 2020-05-25 10:19 UTC (permalink / raw)
To: Markus Elfring, Tejun Heo, Lai Jiangshan; +Cc: linux-kernel, kernel-janitors
I see, kfree does nothing with null pointers and direct return.
but again kfree is not a good suggestion.
On 5/25/20 5:50 PM, Markus Elfring wrote:
>> The duplicate memory release should be deleted from the implementation
>> of the callback function "rcu_free_wq".
>
> I tried to help with the selection of a better commit message.
> I have taken another look also at the implementation of the function “destroy_workqueue”.
>
> * The function call “destroy_workqueue” can be performed there in an if branch
> after the statement “wq->rescuer = NULL” was executed.
>
> * This data processing is independent from a possible call of the
> function “call_rcu(&wq->rcu, rcu_free_wq)” in another if branch.
> Thus it seems that a null pointer is intentionally passed by a data structure
> member to this callback function on demand.
> The corresponding call of the function “kfree” can tolerate this special case.
>
>
> Now I find that the proposed change can be inappropriate.
>
> Regards,
> Markus
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [v2] workqueue: Fix double kfree for rescuer
2020-05-25 10:19 ` qzhang2
@ 2020-05-25 10:40 ` Markus Elfring
2020-05-26 2:05 ` qzhang2
0 siblings, 1 reply; 7+ messages in thread
From: Markus Elfring @ 2020-05-25 10:40 UTC (permalink / raw)
To: Zhang Qiang, Tejun Heo, Lai Jiangshan; +Cc: linux-kernel, kernel-janitors
> I see, kfree does nothing with null pointers and direct return.
> but again kfree is not a good suggestion.
I have got the impression that the implementation detail is important here
if non-null pointers (according to valid rescuer objects) are occasionally
passed by the corresponding data structure member for the callback
function “rcu_free_wq”.
Can another clarification attempt reduce unwanted confusion for this patch review?
Regards,
Markus
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [v2] workqueue: Fix double kfree for rescuer
2020-05-25 10:40 ` [v2] " Markus Elfring
@ 2020-05-26 2:05 ` qzhang2
0 siblings, 0 replies; 7+ messages in thread
From: qzhang2 @ 2020-05-26 2:05 UTC (permalink / raw)
To: Markus Elfring, Tejun Heo, Lai Jiangshan; +Cc: linux-kernel, kernel-janitors
Thanks for your advice.
The rescuer null pointer is intentionally passed by a data structure?
and also I read the code of workqueue again, when destroy_workqueue is
called, after "wq->rescuer = NULL" was executed, The scenario described
below does not happen
"if non-null pointers (according to valid rescuer objects) are
occasionally passed by the corresponding data structure member
for the callback function "rcu_free_wq"."
On 5/25/20 6:40 PM, Markus Elfring wrote:
>> I see, kfree does nothing with null pointers and direct return.
>> but again kfree is not a good suggestion.
>
> I have got the impression that the implementation detail is important here
> if non-null pointers (according to valid rescuer objects) are occasionally
> passed by the corresponding data structure member for the callback
> function “rcu_free_wq”.
> Can another clarification attempt reduce unwanted confusion for this patch review?
>
> Regards,
> Markus
>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-05-26 2:05 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-25 7:59 [PATCH] workqueue: Fix double kfree for rescuer qiang.zhang
2020-05-25 8:20 ` [PATCH v2] workqueue: Delete duplicate kfree() call for “rescuer” from rcu_free_wq() Markus Elfring
2020-05-25 9:50 ` [PATCH v2] workqueue: Fix double kfree for rescuer Markus Elfring
2020-05-25 9:55 ` Markus Elfring
2020-05-25 10:19 ` qzhang2
2020-05-25 10:40 ` [v2] " Markus Elfring
2020-05-26 2:05 ` qzhang2
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).