* Re: [PATCH v3] workqueue: Fix double kfree for rescuer
[not found] <20200525093019.2253-1-qiang.zhang@windriver.com>
@ 2020-05-25 10:01 ` Markus Elfring
2020-05-25 10:01 ` Markus Elfring
[not found] ` <CAJhGHyC4XcNL8yzWZKZ=73wZJej4JwCaAHGV8qjYn-AqcEAEjQ@mail.gmail.com>
2 siblings, 0 replies; 4+ messages in thread
From: Markus Elfring @ 2020-05-25 10:01 UTC (permalink / raw)
To: Zhang Qiang, Tejun Heo, Lai Jiangshan; +Cc: linux-kernel, kernel-janitors
> The callback function "rcu_free_wq" could be called after memory
> was released for "rescuer" already, Thus delete a misplaced call
> of the function "kfree".
I got into the mood to follow your interpretation of the
software situation for a moment.
I have taken another look also at the implementation of the function “destroy_workqueue”.
* The function call “kfree(rescuer)” 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.
I'm sorry for undesirable confusion because of this patch review.
Regards,
Markus
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v3] workqueue: Fix double kfree for rescuer
[not found] <20200525093019.2253-1-qiang.zhang@windriver.com>
2020-05-25 10:01 ` [PATCH v3] workqueue: Fix double kfree for rescuer Markus Elfring
@ 2020-05-25 10:01 ` Markus Elfring
[not found] ` <CAJhGHyC4XcNL8yzWZKZ=73wZJej4JwCaAHGV8qjYn-AqcEAEjQ@mail.gmail.com>
2 siblings, 0 replies; 4+ messages in thread
From: Markus Elfring @ 2020-05-25 10:01 UTC (permalink / raw)
To: Zhang Qiang, Tejun Heo, Lai Jiangshan; +Cc: linux-kernel, kernel-janitors
> The callback function "rcu_free_wq" could be called after memory
> was released for "rescuer" already, Thus delete a misplaced call
> of the function "kfree".
I got into the mood to follow your interpretation of the
software situation for a moment.
I have taken another look also at the implementation of the function “destroy_workqueue”.
* The function call “kfree(rescuer)” 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.
I'm sorry for undesirable confusion because of this patch review.
Regards,
Markus
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [v3] workqueue: Fix double kfree for rescuer
[not found] ` <CAJhGHyC4XcNL8yzWZKZ=73wZJej4JwCaAHGV8qjYn-AqcEAEjQ@mail.gmail.com>
@ 2020-05-26 9:11 ` Markus Elfring
[not found] ` <9e7c20bd-8161-0790-36de-108c6dae65df@windriver.com>
1 sibling, 0 replies; 4+ messages in thread
From: Markus Elfring @ 2020-05-26 9:11 UTC (permalink / raw)
To: Lai Jiangshan, Zhang Qiang; +Cc: Tejun Heo, LKML, kernel-janitors
> wq->rescuer is guaranteed to be NULL in rcu_free_wq()
I was unsure about this data processing detail.
> The patch is a cleanup to remove a "kfree(NULL);".
I would prefer also an improved commit message according to
the understanding of the software situation in this direction.
Regards,
Markus
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [v3] workqueue: Fix double kfree for rescuer
[not found] ` <9e7c20bd-8161-0790-36de-108c6dae65df@windriver.com>
@ 2020-05-26 10:34 ` Markus Elfring
0 siblings, 0 replies; 4+ messages in thread
From: Markus Elfring @ 2020-05-26 10:34 UTC (permalink / raw)
To: Zhang Qiang; +Cc: Lai Jiangshan, Tejun Heo, LKML, kernel-janitors
> There is something wrong with my description. is it feasible to describe as follows:
>
> The resucer is already free in "destroy_workqueue" and
> "wq->rescuer = NULL" was executed, but in "rcu_free_wq"
> it's release again (equivalent to kfree(NULL)), this is
> unnecessary, so should remove.
I find that this suggestion indicates further wording challenges.
Please try another variant.
Regards,
Markus
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-05-26 10:34 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <20200525093019.2253-1-qiang.zhang@windriver.com>
2020-05-25 10:01 ` [PATCH v3] workqueue: Fix double kfree for rescuer Markus Elfring
2020-05-25 10:01 ` Markus Elfring
[not found] ` <CAJhGHyC4XcNL8yzWZKZ=73wZJej4JwCaAHGV8qjYn-AqcEAEjQ@mail.gmail.com>
2020-05-26 9:11 ` [v3] " Markus Elfring
[not found] ` <9e7c20bd-8161-0790-36de-108c6dae65df@windriver.com>
2020-05-26 10:34 ` Markus Elfring
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).