lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Tue, 12 May 2020 17:06:21 +0900 From: Chanwoo Choi <cw00.choi@...sung.com> To: Krzysztof Kozlowski <krzk@...nel.org>, MyungJoo Ham <myungjoo.ham@...sung.com>, Kyungmin Park <kyungmin.park@...sung.com>, linux-pm@...r.kernel.org, linux-kernel@...r.kernel.org Subject: Re: [PATCH] devfreq: Use lockdep asserts instead of manual checks for locked mutex Hi Krzysztof, On 5/12/20 3:41 PM, Krzysztof Kozlowski wrote: > Instead of warning when mutex_is_locked(), just use the lockdep > framework. The code is smaller and checks could be disabled for > production environments (it is useful only during development). > > Put asserts at beginning of function, even before validating arguments. > > The behavior of update_devfreq() is now changed because lockdep assert > will only print a warning, not return with EINVAL. > > Signed-off-by: Krzysztof Kozlowski <krzk@...nel.org> > --- > drivers/devfreq/devfreq.c | 17 +++++++---------- > 1 file changed, 7 insertions(+), 10 deletions(-) > > diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c > index ef3d2bc3d1ac..52b9c3e141f3 100644 > --- a/drivers/devfreq/devfreq.c > +++ b/drivers/devfreq/devfreq.c > @@ -60,12 +60,12 @@ static struct devfreq *find_device_devfreq(struct device *dev) > { > struct devfreq *tmp_devfreq; > > + lockdep_assert_held(&devfreq_list_lock); > + > if (IS_ERR_OR_NULL(dev)) { > pr_err("DEVFREQ: %s: Invalid parameters\n", __func__); > return ERR_PTR(-EINVAL); > } > - WARN(!mutex_is_locked(&devfreq_list_lock), > - "devfreq_list_lock must be locked."); > > list_for_each_entry(tmp_devfreq, &devfreq_list, node) { > if (tmp_devfreq->dev.parent == dev) > @@ -258,12 +258,12 @@ static struct devfreq_governor *find_devfreq_governor(const char *name) > { > struct devfreq_governor *tmp_governor; > > + lockdep_assert_held(&devfreq_list_lock); > + > if (IS_ERR_OR_NULL(name)) { > pr_err("DEVFREQ: %s: Invalid parameters\n", __func__); > return ERR_PTR(-EINVAL); > } > - WARN(!mutex_is_locked(&devfreq_list_lock), > - "devfreq_list_lock must be locked."); > > list_for_each_entry(tmp_governor, &devfreq_governor_list, node) { > if (!strncmp(tmp_governor->name, name, DEVFREQ_NAME_LEN)) > @@ -289,12 +289,12 @@ static struct devfreq_governor *try_then_request_governor(const char *name) > struct devfreq_governor *governor; > int err = 0; > > + lockdep_assert_held(&devfreq_list_lock); > + > if (IS_ERR_OR_NULL(name)) { > pr_err("DEVFREQ: %s: Invalid parameters\n", __func__); > return ERR_PTR(-EINVAL); > } > - WARN(!mutex_is_locked(&devfreq_list_lock), > - "devfreq_list_lock must be locked."); > > governor = find_devfreq_governor(name); > if (IS_ERR(governor)) { > @@ -392,10 +392,7 @@ int update_devfreq(struct devfreq *devfreq) > int err = 0; > u32 flags = 0; > > - if (!mutex_is_locked(&devfreq->lock)) { > - WARN(true, "devfreq->lock must be locked by the caller.\n"); > - return -EINVAL; > - } > + lockdep_assert_held(&devfreq->lock); > > if (!devfreq->governor) > return -EINVAL; > It is reasonable. It looks good. Applied it. Thank -- Best Regards, Chanwoo Choi Samsung Electronics
Powered by blists - more mailing lists