Messages in this thread | | | Date | Thu, 12 Mar 2020 15:25:34 -0400 | From | Tejun Heo <> | Subject | Re: [GIT PULL] cgroup fixes for v5.6-rc5 |
| |
Hello, Linus.
On Tue, Mar 10, 2020 at 03:14:13PM -0700, Linus Torvalds wrote: > On Tue, Mar 10, 2020 at 7:41 AM Tejun Heo <tj@kernel.org> wrote: > > > > * Empty release_agent handling fix. > > Pulled. However, I gagged a bit when I saw the code: > > if (!pathbuf || !agentbuf || !strlen(agentbuf))
Hahaha, yeah, I can see that. I think it might have been copied from the commit it refers to - 64e90a8acb85 which contains the following snippet.
/* * If there is no binary for us to call, then just return and get out of * here. This allows us to set STATIC_USERMODEHELPER_PATH to "" and * disable all call_usermodehelper() calls. */ if (strlen(sub_info->path) == 0) goto out;
> Also, wouldn't it be nice to test for the empty string before you even > bother to kstrdup() it? Even before you
Let me restructure the code a bit.
> Finally, shouldn't we technically hold the release_agent_path_lock > while looking at it?
The release_agent_path is protected by both locks - cgroup_mutex and release_agent_path_lock, so readers can hold either cgroup_mutex or the path_lock. Here, it's holding the mutex, so it should be fine. IIRC, it used to be protected by cgroup_mutex (or whatever was equivalent) and the extra lock was added to break some cyclic dependency. Hmm... might as well drop the cgroup_mutex protection and always use the spinlock.
> Small details, and I've taken the pull, but the lack of locking does > seem to be an actual (if perhaps fairly theoretical) bug, no?
Will queue cleanup patches for the next window.
Thanks.
-- tejun
| |