From: Dan Carpenter <dan.carpenter@oracle.com> To: Lai Jiangshan <jiangshanlai@gmail.com> Cc: "Zhang, Qiang" <Qiang.Zhang@windriver.com>, Markus Elfring <markus.elfring@web.de>, Tejun Heo <tj@kernel.org>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, "kernel-janitors@vger.kernel.org" <kernel-janitors@vger.kernel.org> Subject: Re: 回复: [PATCH v5] workqueue: Remove unnecessary kfree() call in rcu_free_wq() Date: Thu, 28 May 2020 15:25:45 +0300 [thread overview] Message-ID: <20200528122545.GP22511@kadam> (raw) In-Reply-To: <CAJhGHyD1nV=M=ccycqCMt86GMuZGkO9trbJ=4ti4EzP9kta6iA@mail.gmail.com> On Thu, May 28, 2020 at 08:08:06PM +0800, Lai Jiangshan wrote: > On Thu, May 28, 2020 at 5:57 PM Dan Carpenter <dan.carpenter@oracle.com> wrote: > > > > Guys, the patch is wrong. The kfree is harmless when this is called > > from destroy_workqueue() and required when it's called from > > pwq_unbound_release_workfn(). Lai Jiangshan already explained this > > already. Why are we still discussing this? > > > > I'm also confused why they have been debating about the changelog > after the patch was queued. My statement was about "the patch is > a correct cleanup, but the changelog is totally misleading". > > destroy_workqueue(percpu_wq) -> rcu_free_wq() > or > destroy_workqueue(unbound_wq) -> put_pwq() -> > pwq_unbound_release_workfn() -> rcu_free_wq() > > So the patch is correct to me. Only can destroy_workqueue() > lead to rcu_free_wq(). It looks like there are lots of paths which call put_pwq() and put_pwq_unlocked(). 1168 static void pwq_dec_nr_in_flight(struct pool_workqueue *pwq, int color) 1169 { 1170 /* uncolored work items don't participate in flushing or nr_active */ 1171 if (color == WORK_NO_COLOR) 1172 goto out_put; 1173 We don't take an extra reference in this function. 1200 out_put: 1201 put_pwq(pwq); 1202 } I don't know this code well, so I will defer to your expertise if you say it is correct. > > Still, the kfree(NULL) is harmless. But it is cleaner > to have the patch. But the changelog is wrong, even after > the lengthened debating, and English is not my mother tongue, > so I just looked on. We have tried to tell Markus not to advise people about commit messages but he doesn't listen. He has discouraged some contributors. :/ regards, dan carpenter
WARNING: multiple messages have this Message-ID (diff)
From: Dan Carpenter <dan.carpenter@oracle.com> To: Lai Jiangshan <jiangshanlai@gmail.com> Cc: "Zhang, Qiang" <Qiang.Zhang@windriver.com>, Markus Elfring <markus.elfring@web.de>, Tejun Heo <tj@kernel.org>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, "kernel-janitors@vger.kernel.org" <kernel-janitors@vger.kernel.org> Subject: Re: 回复: [PATCH v5] workqueue: Remove unnecessary kfree() call Date: Thu, 28 May 2020 12:25:45 +0000 [thread overview] Message-ID: <20200528122545.GP22511@kadam> (raw) In-Reply-To: <CAJhGHyD1nV=M=ccycqCMt86GMuZGkO9trbJ=4ti4EzP9kta6iA@mail.gmail.com> On Thu, May 28, 2020 at 08:08:06PM +0800, Lai Jiangshan wrote: > On Thu, May 28, 2020 at 5:57 PM Dan Carpenter <dan.carpenter@oracle.com> wrote: > > > > Guys, the patch is wrong. The kfree is harmless when this is called > > from destroy_workqueue() and required when it's called from > > pwq_unbound_release_workfn(). Lai Jiangshan already explained this > > already. Why are we still discussing this? > > > > I'm also confused why they have been debating about the changelog > after the patch was queued. My statement was about "the patch is > a correct cleanup, but the changelog is totally misleading". > > destroy_workqueue(percpu_wq) -> rcu_free_wq() > or > destroy_workqueue(unbound_wq) -> put_pwq() -> > pwq_unbound_release_workfn() -> rcu_free_wq() > > So the patch is correct to me. Only can destroy_workqueue() > lead to rcu_free_wq(). It looks like there are lots of paths which call put_pwq() and put_pwq_unlocked(). 1168 static void pwq_dec_nr_in_flight(struct pool_workqueue *pwq, int color) 1169 { 1170 /* uncolored work items don't participate in flushing or nr_active */ 1171 if (color = WORK_NO_COLOR) 1172 goto out_put; 1173 We don't take an extra reference in this function. 1200 out_put: 1201 put_pwq(pwq); 1202 } I don't know this code well, so I will defer to your expertise if you say it is correct. > > Still, the kfree(NULL) is harmless. But it is cleaner > to have the patch. But the changelog is wrong, even after > the lengthened debating, and English is not my mother tongue, > so I just looked on. We have tried to tell Markus not to advise people about commit messages but he doesn't listen. He has discouraged some contributors. :/ regards, dan carpenter
next prev parent reply other threads:[~2020-05-28 12:28 UTC|newest] Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-05-27 7:57 [PATCH v5] workqueue: Remove unnecessary kfree() call in rcu_free_wq() qiang.zhang 2020-05-27 8:20 ` Markus Elfring 2020-05-27 8:20 ` Markus Elfring [not found] ` <DM6PR11MB32573F3884A864ECD586235EFF8E0@DM6PR11MB3257.namprd11.prod.outlook.com> 2020-05-28 1:44 ` 回复: " Zhang, Qiang 2020-05-28 1:44 ` 回复: [PATCH v5] workqueue: Remove unnecessar =?gb2312?B?eSBrZnJlZSgpIGN Zhang, Qiang 2020-05-28 9:57 ` 回复: [PATCH v5] workqueue: Remove unnecessary kfree() call in rcu_free_wq() Dan Carpenter 2020-05-28 9:57 ` 回复: [PATCH v5] workqueue: Remove unnecessary kfree() call Dan Carpenter 2020-05-28 10:38 ` 回复: [PATCH v5] workqueue: Remove unnecessary kfree() call in rcu_free_wq() Tejun Heo 2020-05-28 10:38 ` 回复: [PATCH v5] workqueue: Remove unnecessary kfree() call Tejun Heo 2020-05-28 11:00 ` [v5] workqueue: Remove unnecessary kfree() call in rcu_free_wq() Markus Elfring 2020-05-28 11:00 ` Markus Elfring 2020-05-28 14:40 ` Markus Elfring 2020-05-28 14:40 ` Markus Elfring 2020-05-28 12:08 ` 回复: [PATCH v5] " Lai Jiangshan 2020-05-28 12:08 ` 回复: [PATCH v5] workqueue: Remove unnecessary =?UTF-8?B?IGtmc Lai Jiangshan 2020-05-28 12:25 ` Dan Carpenter [this message] 2020-05-28 12:25 ` 回复: [PATCH v5] workqueue: Remove unnecessary kfree() call Dan Carpenter 2020-05-28 13:27 ` 回复: [PATCH v5] workqueue: Remove unnecessary kfree() call in rcu_free_wq() Lai Jiangshan 2020-05-28 13:27 ` 回复: [PATCH v5] workqueue: Remove unnecessary =?UTF-8?B?IGtmc Lai Jiangshan 2020-05-28 14:03 ` 回复: [PATCH v5] workqueue: Remove unnecessary kfree() call in rcu_free_wq() Tejun Heo 2020-05-28 14:03 ` 回复: [PATCH v5] workqueue: Remove unnecessary kfree() call Tejun Heo 2020-05-28 14:45 ` [v5] workqueue: Remove unnecessary kfree() call in rcu_free_wq() Markus Elfring 2020-05-28 14:45 ` Markus Elfring 2020-05-28 14:06 ` 回复: [PATCH v5] " Dan Carpenter 2020-05-28 14:06 ` 回复: [PATCH v5] workqueue: Remove unnecessary kfree() call Dan Carpenter 2020-05-28 15:25 ` [v5] workqueue: Remove unnecessary kfree() call in rcu_free_wq() Markus Elfring 2020-05-28 15:25 ` Markus Elfring 2020-05-28 15:02 ` Markus Elfring 2020-05-28 15:02 ` Markus Elfring 2020-05-28 15:02 ` Markus Elfring 2020-05-28 15:02 ` Markus Elfring 2020-05-27 13:52 ` [PATCH v5] " Tejun Heo
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20200528122545.GP22511@kadam \ --to=dan.carpenter@oracle.com \ --cc=Qiang.Zhang@windriver.com \ --cc=jiangshanlai@gmail.com \ --cc=kernel-janitors@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=markus.elfring@web.de \ --cc=tj@kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
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.