Re: [PATCH v1 10/11] thermal/core: Alloc-copy-free the thermal zone parameters structure

From: kernel test robot
Date: Wed Mar 15 2023 - 08:54:56 EST


Hi Daniel,

I love your patch! Perhaps something to improve:

[auto build test WARNING on rafael-pm/linux-next]
[also build test WARNING on linus/master v6.3-rc2 next-20230315]
[cannot apply to rafael-pm/thermal]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url: https://github.com/intel-lab-lkp/linux/commits/Daniel-Lezcano/thermal-drivers-intel_pch_thermal-Use-thermal-driver-device-to-write-a-trace/20230307-223759
base: https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next
patch link: https://lore.kernel.org/r/20230307133735.90772-11-daniel.lezcano%40linaro.org
patch subject: [PATCH v1 10/11] thermal/core: Alloc-copy-free the thermal zone parameters structure
config: i386-randconfig-a011-20230313 (https://download.01.org/0day-ci/archive/20230315/202303152025.rVv9btko-lkp@xxxxxxxxx/config)
compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project f28c006a5895fc0e329fe15fead81e37457cb1d1)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/a9813bacf3531491cb02c13aafe358e6b5423aa0
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Daniel-Lezcano/thermal-drivers-intel_pch_thermal-Use-thermal-driver-device-to-write-a-trace/20230307-223759
git checkout a9813bacf3531491cb02c13aafe358e6b5423aa0
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 SHELL=/bin/bash drivers/thermal/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@xxxxxxxxx>
| Link: https://lore.kernel.org/oe-kbuild-all/202303152025.rVv9btko-lkp@xxxxxxxxx/

All warnings (new ones prefixed by >>):

>> drivers/thermal/thermal_core.c:1267:7: warning: variable 'result' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
if (!tz->tzp)
^~~~~~~~
drivers/thermal/thermal_core.c:1388:17: note: uninitialized use occurs here
return ERR_PTR(result);
^~~~~~
drivers/thermal/thermal_core.c:1267:3: note: remove the 'if' if its condition is always false
if (!tz->tzp)
^~~~~~~~~~~~~
drivers/thermal/thermal_core.c:1217:12: note: initialize the variable 'result' to silence this warning
int result;
^
= 0
1 warning generated.


vim +1267 drivers/thermal/thermal_core.c

1183
1184 /**
1185 * thermal_zone_device_register_with_trips() - register a new thermal zone device
1186 * @type: the thermal zone device type
1187 * @trips: a pointer to an array of thermal trips
1188 * @num_trips: the number of trip points the thermal zone support
1189 * @mask: a bit string indicating the writeablility of trip points
1190 * @devdata: private device data
1191 * @ops: standard thermal zone device callbacks
1192 * @tzp: thermal zone platform parameters
1193 * @passive_delay: number of milliseconds to wait between polls when
1194 * performing passive cooling
1195 * @polling_delay: number of milliseconds to wait between polls when checking
1196 * whether trip points have been crossed (0 for interrupt
1197 * driven systems)
1198 *
1199 * This interface function adds a new thermal zone device (sensor) to
1200 * /sys/class/thermal folder as thermal_zone[0-*]. It tries to bind all the
1201 * thermal cooling devices registered at the same time.
1202 * thermal_zone_device_unregister() must be called when the device is no
1203 * longer needed. The passive cooling depends on the .get_trend() return value.
1204 *
1205 * Return: a pointer to the created struct thermal_zone_device or an
1206 * in case of error, an ERR_PTR. Caller must check return value with
1207 * IS_ERR*() helpers.
1208 */
1209 struct thermal_zone_device *
1210 thermal_zone_device_register_with_trips(const char *type, struct thermal_trip *trips, int num_trips, int mask,
1211 void *devdata, struct thermal_zone_device_ops *ops,
1212 struct thermal_zone_params *tzp, int passive_delay,
1213 int polling_delay)
1214 {
1215 struct thermal_zone_device *tz;
1216 int id;
1217 int result;
1218 int count;
1219 struct thermal_governor *governor;
1220
1221 if (!type || strlen(type) == 0) {
1222 pr_err("No thermal zone type defined\n");
1223 return ERR_PTR(-EINVAL);
1224 }
1225
1226 if (strlen(type) >= THERMAL_NAME_LENGTH) {
1227 pr_err("Thermal zone name (%s) too long, should be under %d chars\n",
1228 type, THERMAL_NAME_LENGTH);
1229 return ERR_PTR(-EINVAL);
1230 }
1231
1232 /*
1233 * Max trip count can't exceed 31 as the "mask >> num_trips" condition.
1234 * For example, shifting by 32 will result in compiler warning:
1235 * warning: right shift count >= width of type [-Wshift-count- overflow]
1236 *
1237 * Also "mask >> num_trips" will always be true with 32 bit shift.
1238 * E.g. mask = 0x80000000 for trip id 31 to be RW. Then
1239 * mask >> 32 = 0x80000000
1240 * This will result in failure for the below condition.
1241 *
1242 * Check will be true when the bit 31 of the mask is set.
1243 * 32 bit shift will cause overflow of 4 byte integer.
1244 */
1245 if (num_trips > (BITS_PER_TYPE(int) - 1) || num_trips < 0 || mask >> num_trips) {
1246 pr_err("Incorrect number of thermal trips\n");
1247 return ERR_PTR(-EINVAL);
1248 }
1249
1250 if (!ops) {
1251 pr_err("Thermal zone device ops not defined\n");
1252 return ERR_PTR(-EINVAL);
1253 }
1254
1255 if (num_trips > 0 && (!ops->get_trip_type || !ops->get_trip_temp) && !trips)
1256 return ERR_PTR(-EINVAL);
1257
1258 if (!thermal_class)
1259 return ERR_PTR(-ENODEV);
1260
1261 tz = kzalloc(sizeof(*tz), GFP_KERNEL);
1262 if (!tz)
1263 return ERR_PTR(-ENOMEM);
1264
1265 if (tzp) {
1266 tz->tzp = kmemdup(tzp, sizeof(*tzp), GFP_KERNEL);
> 1267 if (!tz->tzp)
1268 goto free_tz;
1269 }
1270
1271 INIT_LIST_HEAD(&tz->thermal_instances);
1272 ida_init(&tz->ida);
1273 mutex_init(&tz->lock);
1274 id = ida_alloc(&thermal_tz_ida, GFP_KERNEL);
1275 if (id < 0) {
1276 result = id;
1277 goto free_tzp;
1278 }
1279
1280 tz->id = id;
1281 strscpy(tz->type, type, sizeof(tz->type));
1282
1283 if (!ops->critical)
1284 ops->critical = thermal_zone_device_critical;
1285
1286 tz->ops = ops;
1287 tz->device.class = thermal_class;
1288 tz->devdata = devdata;
1289 tz->trips = trips;
1290 tz->num_trips = num_trips;
1291
1292 thermal_set_delay_jiffies(&tz->passive_delay_jiffies, passive_delay);
1293 thermal_set_delay_jiffies(&tz->polling_delay_jiffies, polling_delay);
1294
1295 /* sys I/F */
1296 /* Add nodes that are always present via .groups */
1297 result = thermal_zone_create_device_groups(tz, mask);
1298 if (result)
1299 goto remove_id;
1300
1301 /* A new thermal zone needs to be updated anyway. */
1302 atomic_set(&tz->need_update, 1);
1303
1304 result = dev_set_name(&tz->device, "thermal_zone%d", tz->id);
1305 if (result) {
1306 thermal_zone_destroy_device_groups(tz);
1307 goto remove_id;
1308 }
1309 result = device_register(&tz->device);
1310 if (result)
1311 goto release_device;
1312
1313 for (count = 0; count < num_trips; count++) {
1314 struct thermal_trip trip;
1315
1316 result = thermal_zone_get_trip(tz, count, &trip);
1317 if (result)
1318 set_bit(count, &tz->trips_disabled);
1319 }
1320
1321 /* Update 'this' zone's governor information */
1322 mutex_lock(&thermal_governor_lock);
1323
1324 if (tz->tzp)
1325 governor = __find_governor(tz->tzp->governor_name);
1326 else
1327 governor = def_governor;
1328
1329 result = thermal_set_governor(tz, governor);
1330 if (result) {
1331 mutex_unlock(&thermal_governor_lock);
1332 goto unregister;
1333 }
1334
1335 mutex_unlock(&thermal_governor_lock);
1336
1337 if (!tz->tzp || !tz->tzp->no_hwmon) {
1338 result = thermal_add_hwmon_sysfs(tz);
1339 if (result)
1340 goto unregister;
1341 }
1342
1343 mutex_lock(&thermal_list_lock);
1344 list_add_tail(&tz->node, &thermal_tz_list);
1345 mutex_unlock(&thermal_list_lock);
1346
1347 if (tzp && tzp->linked_dev) {
1348 result = sysfs_create_link(&tzp->linked_dev->kobj,
1349 &tz->device.kobj, "thermal_zone");
1350 if (result)
1351 goto out_list_del;
1352
1353 result = sysfs_create_link(&tz->device.kobj,
1354 &tzp->linked_dev->kobj, "device");
1355 if (result)
1356 goto out_del_link;
1357 }
1358
1359 /* Bind cooling devices for this zone */
1360 bind_tz(tz);
1361
1362 INIT_DELAYED_WORK(&tz->poll_queue, thermal_zone_device_check);
1363
1364 thermal_zone_device_init(tz);
1365 /* Update the new thermal zone and mark it as already updated. */
1366 if (atomic_cmpxchg(&tz->need_update, 1, 0))
1367 thermal_zone_device_update(tz, THERMAL_EVENT_UNSPECIFIED);
1368
1369 thermal_notify_tz_create(tz->id, tz->type);
1370
1371 return tz;
1372
1373 out_del_link:
1374 sysfs_remove_link(&tz->device.kobj, "thermal_zone");
1375 out_list_del:
1376 list_del(&tz->node);
1377 unregister:
1378 device_del(&tz->device);
1379 release_device:
1380 put_device(&tz->device);
1381 tz = NULL;
1382 remove_id:
1383 ida_free(&thermal_tz_ida, id);
1384 free_tzp:
1385 kfree(tz->tzp);
1386 free_tz:
1387 kfree(tz);
1388 return ERR_PTR(result);
1389 }
1390 EXPORT_SYMBOL_GPL(thermal_zone_device_register_with_trips);
1391

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests