[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20200608115155.GY30374@kadam>
Date: Mon, 8 Jun 2020 14:51:55 +0300
From: Dan Carpenter <dan.carpenter@...cle.com>
To: kbuild@...ts.01.org, Lukasz Luba <lukasz.luba@....com>,
linux-kernel@...r.kernel.org, linux-pm@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org,
dri-devel@...ts.freedesktop.org, linux-omap@...r.kernel.org,
linux-mediatek@...ts.infradead.org, linux-arm-msm@...r.kernel.org,
linux-imx@....com
Cc: lkp@...el.com, Dan Carpenter <error27@...il.com>,
kbuild-all@...ts.01.org, Dietmar.Eggemann@....com,
cw00.choi@...sung.com
Subject: Re: [PATCH v8 4/8] PM / EM: add support for other devices than CPUs
in Energy Model
Hi Lukasz,
I love your patch! Perhaps something to improve:
url: https://github.com/0day-ci/linux/commits/Lukasz-Luba/Add-support-for-devices-in-the-Energy-Model/20200527-180614
base: https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next
config: i386-randconfig-m021-20200605 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-13) 9.3.0
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@...el.com>
Reported-by: Dan Carpenter <dan.carpenter@...cle.com>
smatch warnings:
kernel/power/energy_model.c:316 em_dev_register_perf_domain() error: we previously assumed 'dev->em_pd' could be null (see line 277)
# https://github.com/0day-ci/linux/commit/110d050cb7ba1c96e63ada498979d1fd99529be2
git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout 110d050cb7ba1c96e63ada498979d1fd99529be2
vim +316 kernel/power/energy_model.c
0e294e607adaf3 Lukasz Luba 2020-05-27 262 int em_dev_register_perf_domain(struct device *dev, unsigned int nr_states,
110d050cb7ba1c Lukasz Luba 2020-05-27 263 struct em_data_callback *cb, cpumask_t *cpus)
27871f7a8a341e Quentin Perret 2018-12-03 264 {
27871f7a8a341e Quentin Perret 2018-12-03 265 unsigned long cap, prev_cap = 0;
110d050cb7ba1c Lukasz Luba 2020-05-27 266 int cpu, ret;
27871f7a8a341e Quentin Perret 2018-12-03 267
110d050cb7ba1c Lukasz Luba 2020-05-27 268 if (!dev || !nr_states || !cb)
27871f7a8a341e Quentin Perret 2018-12-03 269 return -EINVAL;
27871f7a8a341e Quentin Perret 2018-12-03 270
27871f7a8a341e Quentin Perret 2018-12-03 271 /*
27871f7a8a341e Quentin Perret 2018-12-03 272 * Use a mutex to serialize the registration of performance domains and
27871f7a8a341e Quentin Perret 2018-12-03 273 * let the driver-defined callback functions sleep.
27871f7a8a341e Quentin Perret 2018-12-03 274 */
27871f7a8a341e Quentin Perret 2018-12-03 275 mutex_lock(&em_pd_mutex);
27871f7a8a341e Quentin Perret 2018-12-03 276
110d050cb7ba1c Lukasz Luba 2020-05-27 @277 if (dev->em_pd) {
^^^^^^^^^^
Check for NULL.
27871f7a8a341e Quentin Perret 2018-12-03 278 ret = -EEXIST;
27871f7a8a341e Quentin Perret 2018-12-03 279 goto unlock;
27871f7a8a341e Quentin Perret 2018-12-03 280 }
27871f7a8a341e Quentin Perret 2018-12-03 281
110d050cb7ba1c Lukasz Luba 2020-05-27 282 if (_is_cpu_device(dev)) {
110d050cb7ba1c Lukasz Luba 2020-05-27 283 if (!cpus) {
110d050cb7ba1c Lukasz Luba 2020-05-27 284 dev_err(dev, "EM: invalid CPU mask\n");
110d050cb7ba1c Lukasz Luba 2020-05-27 285 ret = -EINVAL;
110d050cb7ba1c Lukasz Luba 2020-05-27 286 goto unlock;
110d050cb7ba1c Lukasz Luba 2020-05-27 287 }
110d050cb7ba1c Lukasz Luba 2020-05-27 288
110d050cb7ba1c Lukasz Luba 2020-05-27 289 for_each_cpu(cpu, cpus) {
110d050cb7ba1c Lukasz Luba 2020-05-27 290 if (em_cpu_get(cpu)) {
110d050cb7ba1c Lukasz Luba 2020-05-27 291 dev_err(dev, "EM: exists for CPU%d\n", cpu);
110d050cb7ba1c Lukasz Luba 2020-05-27 292 ret = -EEXIST;
110d050cb7ba1c Lukasz Luba 2020-05-27 293 goto unlock;
110d050cb7ba1c Lukasz Luba 2020-05-27 294 }
27871f7a8a341e Quentin Perret 2018-12-03 295 /*
110d050cb7ba1c Lukasz Luba 2020-05-27 296 * All CPUs of a domain must have the same
110d050cb7ba1c Lukasz Luba 2020-05-27 297 * micro-architecture since they all share the same
110d050cb7ba1c Lukasz Luba 2020-05-27 298 * table.
27871f7a8a341e Quentin Perret 2018-12-03 299 */
8ec59c0f5f4966 Vincent Guittot 2019-06-17 300 cap = arch_scale_cpu_capacity(cpu);
27871f7a8a341e Quentin Perret 2018-12-03 301 if (prev_cap && prev_cap != cap) {
110d050cb7ba1c Lukasz Luba 2020-05-27 302 dev_err(dev, "EM: CPUs of %*pbl must have the same capacity\n",
110d050cb7ba1c Lukasz Luba 2020-05-27 303 cpumask_pr_args(cpus));
110d050cb7ba1c Lukasz Luba 2020-05-27 304
27871f7a8a341e Quentin Perret 2018-12-03 305 ret = -EINVAL;
27871f7a8a341e Quentin Perret 2018-12-03 306 goto unlock;
27871f7a8a341e Quentin Perret 2018-12-03 307 }
27871f7a8a341e Quentin Perret 2018-12-03 308 prev_cap = cap;
27871f7a8a341e Quentin Perret 2018-12-03 309 }
110d050cb7ba1c Lukasz Luba 2020-05-27 310 }
27871f7a8a341e Quentin Perret 2018-12-03 311
110d050cb7ba1c Lukasz Luba 2020-05-27 312 ret = em_create_pd(dev, nr_states, cb, cpus);
If it's a _is_cpu_device() then it iterates through a bunch of devices
and sets up cpu_dev->em_pd for each. Presumably one of the devices is
"dev" or this would crash pretty early on in testing?
110d050cb7ba1c Lukasz Luba 2020-05-27 313 if (ret)
27871f7a8a341e Quentin Perret 2018-12-03 314 goto unlock;
27871f7a8a341e Quentin Perret 2018-12-03 315
110d050cb7ba1c Lukasz Luba 2020-05-27 @316 em_debug_create_pd(dev);
^^^
Dereferences dev->em_pd.
110d050cb7ba1c Lukasz Luba 2020-05-27 317 dev_info(dev, "EM: created perf domain\n");
27871f7a8a341e Quentin Perret 2018-12-03 318
27871f7a8a341e Quentin Perret 2018-12-03 319 unlock:
27871f7a8a341e Quentin Perret 2018-12-03 320 mutex_unlock(&em_pd_mutex);
27871f7a8a341e Quentin Perret 2018-12-03 321 return ret;
27871f7a8a341e Quentin Perret 2018-12-03 322 }
0e294e607adaf3 Lukasz Luba 2020-05-27 323 EXPORT_SYMBOL_GPL(em_dev_register_perf_domain);
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Download attachment ".config.gz" of type "application/gzip" (28764 bytes)
Powered by blists - more mailing lists