Skip to content

Commit fdaf0c8

Browse files
zhang-ruigroeck
authored andcommitted
hwmon: (coretemp) Fix bogus core_id to attr name mapping
Before commit 7108b80 ("hwmon/coretemp: Handle large core ID value"), there is a fixed mapping between 1. cpu_core_id 2. the index in pdata->core_data[] array 3. the sysfs attr name, aka "tempX_" The later two always equal cpu_core_id + 2. After the commit, pdata->core_data[] index is got from ida so that it can handle sparse core ids and support more cores within a package. However, the commit erroneously maps the sysfs attr name to pdata->core_data[] index instead of cpu_core_id + 2. As a result, the code is not aligned with the comments, and brings user visible changes in hwmon sysfs on systems with sparse core id. For example, before commit 7108b80 ("hwmon/coretemp: Handle large core ID value"), /sys/class/hwmon/hwmon2/temp2_label:Core 0 /sys/class/hwmon/hwmon2/temp3_label:Core 1 /sys/class/hwmon/hwmon2/temp4_label:Core 2 /sys/class/hwmon/hwmon2/temp5_label:Core 3 /sys/class/hwmon/hwmon2/temp6_label:Core 4 /sys/class/hwmon/hwmon3/temp10_label:Core 8 /sys/class/hwmon/hwmon3/temp11_label:Core 9 after commit, /sys/class/hwmon/hwmon2/temp2_label:Core 0 /sys/class/hwmon/hwmon2/temp3_label:Core 1 /sys/class/hwmon/hwmon2/temp4_label:Core 2 /sys/class/hwmon/hwmon2/temp5_label:Core 3 /sys/class/hwmon/hwmon2/temp6_label:Core 4 /sys/class/hwmon/hwmon2/temp7_label:Core 8 /sys/class/hwmon/hwmon2/temp8_label:Core 9 Restore the previous behavior and rework the code, comments and variable names to avoid future confusions. Fixes: 7108b80 ("hwmon/coretemp: Handle large core ID value") Signed-off-by: Zhang Rui <rui.zhang@intel.com> Link: https://lore.kernel.org/r/20240202092144.71180-3-rui.zhang@intel.com Signed-off-by: Guenter Roeck <linux@roeck-us.net>
1 parent 4e440ab commit fdaf0c8

File tree

1 file changed

+19
-13
lines changed

1 file changed

+19
-13
lines changed

drivers/hwmon/coretemp.c

Lines changed: 19 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -419,7 +419,7 @@ static ssize_t show_temp(struct device *dev,
419419
}
420420

421421
static int create_core_attrs(struct temp_data *tdata, struct device *dev,
422-
int attr_no)
422+
int index)
423423
{
424424
int i;
425425
static ssize_t (*const rd_ptr[TOTAL_ATTRS]) (struct device *dev,
@@ -431,13 +431,20 @@ static int create_core_attrs(struct temp_data *tdata, struct device *dev,
431431
};
432432

433433
for (i = 0; i < tdata->attr_size; i++) {
434+
/*
435+
* We map the attr number to core id of the CPU
436+
* The attr number is always core id + 2
437+
* The Pkgtemp will always show up as temp1_*, if available
438+
*/
439+
int attr_no = tdata->is_pkg_data ? 1 : tdata->cpu_core_id + 2;
440+
434441
snprintf(tdata->attr_name[i], CORETEMP_NAME_LENGTH,
435442
"temp%d_%s", attr_no, suffixes[i]);
436443
sysfs_attr_init(&tdata->sd_attrs[i].dev_attr.attr);
437444
tdata->sd_attrs[i].dev_attr.attr.name = tdata->attr_name[i];
438445
tdata->sd_attrs[i].dev_attr.attr.mode = 0444;
439446
tdata->sd_attrs[i].dev_attr.show = rd_ptr[i];
440-
tdata->sd_attrs[i].index = attr_no;
447+
tdata->sd_attrs[i].index = index;
441448
tdata->attrs[i] = &tdata->sd_attrs[i].dev_attr.attr;
442449
}
443450
tdata->attr_group.attrs = tdata->attrs;
@@ -495,26 +502,25 @@ static int create_core_data(struct platform_device *pdev, unsigned int cpu,
495502
struct platform_data *pdata = platform_get_drvdata(pdev);
496503
struct cpuinfo_x86 *c = &cpu_data(cpu);
497504
u32 eax, edx;
498-
int err, index, attr_no;
505+
int err, index;
499506

500507
if (!housekeeping_cpu(cpu, HK_TYPE_MISC))
501508
return 0;
502509

503510
/*
504-
* Find attr number for sysfs:
505-
* We map the attr number to core id of the CPU
506-
* The attr number is always core id + 2
507-
* The Pkgtemp will always show up as temp1_*, if available
511+
* Get the index of tdata in pdata->core_data[]
512+
* tdata for package: pdata->core_data[1]
513+
* tdata for core: pdata->core_data[2] .. pdata->core_data[NUM_REAL_CORES + 1]
508514
*/
509515
if (pkg_flag) {
510-
attr_no = PKG_SYSFS_ATTR_NO;
516+
index = PKG_SYSFS_ATTR_NO;
511517
} else {
512518
index = ida_alloc_max(&pdata->ida, NUM_REAL_CORES - 1, GFP_KERNEL);
513519
if (index < 0)
514520
return index;
515521

516522
pdata->cpu_map[index] = topology_core_id(cpu);
517-
attr_no = index + BASE_SYSFS_ATTR_NO;
523+
index += BASE_SYSFS_ATTR_NO;
518524
}
519525

520526
tdata = init_temp_data(cpu, pkg_flag);
@@ -540,20 +546,20 @@ static int create_core_data(struct platform_device *pdev, unsigned int cpu,
540546
if (get_ttarget(tdata, &pdev->dev) >= 0)
541547
tdata->attr_size++;
542548

543-
pdata->core_data[attr_no] = tdata;
549+
pdata->core_data[index] = tdata;
544550

545551
/* Create sysfs interfaces */
546-
err = create_core_attrs(tdata, pdata->hwmon_dev, attr_no);
552+
err = create_core_attrs(tdata, pdata->hwmon_dev, index);
547553
if (err)
548554
goto exit_free;
549555

550556
return 0;
551557
exit_free:
552-
pdata->core_data[attr_no] = NULL;
558+
pdata->core_data[index] = NULL;
553559
kfree(tdata);
554560
ida_free:
555561
if (!pkg_flag)
556-
ida_free(&pdata->ida, index);
562+
ida_free(&pdata->ida, index - BASE_SYSFS_ATTR_NO);
557563
return err;
558564
}
559565

0 commit comments

Comments
 (0)