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 <[email protected]> > --- > 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
