* [PATCH 0/2] hwmon: (coretemp) replace hardcoded core count and fix style issues @ 2026-05-16 11:42 bakshansky.lists 2026-05-16 11:42 ` [PATCH 1/2] hwmon: (coretemp) replace hardcoded core count with dynamic value bakshansky.lists 2026-05-16 11:42 ` [PATCH 2/2] hwmon: (coretemp) fix coding style issues bakshansky.lists 0 siblings, 2 replies; 9+ messages in thread From: bakshansky.lists @ 2026-05-16 11:42 UTC (permalink / raw) To: linux-hwmon; +Cc: linux, Roman Bakshansky From: Roman Bakshansky <bakshansky.lists@gmail.com> This series addresses a long-standing TODO in the coretemp driver by replacing the hardcoded NUM_REAL_CORES with a dynamic per-package core count. The second patch cleans up a few coding style warnings. Roman Bakshansky (2): hwmon: (coretemp) replace hardcoded core count with dynamic value hwmon: (coretemp) fix coding style issues drivers/hwmon/coretemp.c | 21 +++++++-------------- 1 file changed, 7 insertions(+), 14 deletions(-) -- 2.53.0 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/2] hwmon: (coretemp) replace hardcoded core count with dynamic value 2026-05-16 11:42 [PATCH 0/2] hwmon: (coretemp) replace hardcoded core count and fix style issues bakshansky.lists @ 2026-05-16 11:42 ` bakshansky.lists 2026-05-16 12:16 ` sashiko-bot 2026-05-16 12:48 ` Guenter Roeck 2026-05-16 11:42 ` [PATCH 2/2] hwmon: (coretemp) fix coding style issues bakshansky.lists 1 sibling, 2 replies; 9+ messages in thread From: bakshansky.lists @ 2026-05-16 11:42 UTC (permalink / raw) To: linux-hwmon; +Cc: linux, Roman Bakshansky From: Roman Bakshansky <bakshansky.lists@gmail.com> The hardcoded maximum of 512 cores per package was first defined by commit 34cf8c657cf0 ("hwmon: (coretemp) Enlarge per package core count limit") and later kept as a fallback with a TODO in commit 1a793caf6f69 ("hwmon: (coretemp) Use dynamic allocated memory for core temp_data") because the actual per-package core count was not reliably available at the time. Now that topology_num_cores_per_package() is stable and suitable for use, it's time to complete the TODO and allocate only the needed amount of memory for core_data. Signed-off-by: Roman Bakshansky <bakshansky.lists@gmail.com> --- drivers/hwmon/coretemp.c | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/drivers/hwmon/coretemp.c b/drivers/hwmon/coretemp.c index 6a0d94711ead..c722b1d8e480 100644 --- a/drivers/hwmon/coretemp.c +++ b/drivers/hwmon/coretemp.c @@ -39,7 +39,6 @@ static int force_tjmax; module_param_named(tjmax, force_tjmax, int, 0444); MODULE_PARM_DESC(tjmax, "TjMax value in degrees Celsius"); -#define NUM_REAL_CORES 512 /* Number of Real cores per cpu */ #define CORETEMP_NAME_LENGTH 28 /* String Length of attrs */ enum coretemp_attr_index { @@ -485,13 +484,7 @@ init_temp_data(struct platform_data *pdata, unsigned int cpu, int pkg_flag) struct temp_data *tdata; if (!pdata->core_data) { - /* - * TODO: - * The information of actual possible cores in a package is broken for now. - * Will replace hardcoded NUM_REAL_CORES with actual per package core count - * when this information becomes available. - */ - pdata->nr_cores = NUM_REAL_CORES; + pdata->nr_cores = topology_num_cores_per_package(); pdata->core_data = kzalloc_objs(struct temp_data *, pdata->nr_cores); if (!pdata->core_data) -- 2.53.0 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] hwmon: (coretemp) replace hardcoded core count with dynamic value 2026-05-16 11:42 ` [PATCH 1/2] hwmon: (coretemp) replace hardcoded core count with dynamic value bakshansky.lists @ 2026-05-16 12:16 ` sashiko-bot 2026-05-16 12:47 ` Guenter Roeck 2026-05-16 12:48 ` Guenter Roeck 1 sibling, 1 reply; 9+ messages in thread From: sashiko-bot @ 2026-05-16 12:16 UTC (permalink / raw) To: bakshansky.lists; +Cc: linux-hwmon Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] If `topology_num_cores_per_package()` evaluates to 0, an integer underflow occurs leading to an out-of-bounds write and kernel panic. - [High] Using the global `topology_num_cores_per_package()` limits the per-package array size, preventing sensor initialization on heterogeneous packages or during CPU hot-add. -- commit d686b3577dd2107e7452adf6a949cc0720d2213a Author: Roman Bakshansky <bakshansky.lists@gmail.com> hwmon: (coretemp) replace hardcoded core count with dynamic value This commit replaces the hardcoded limit of 512 cores per package with a dynamic value. It uses topology_num_cores_per_package() to allocate only the needed amount of memory for core_data. > diff --git a/drivers/hwmon/coretemp.c b/drivers/hwmon/coretemp.c > index 6a0d94711eadb..c722b1d8e4804 100644 > --- a/drivers/hwmon/coretemp.c > +++ b/drivers/hwmon/coretemp.c [ ... ] > @@ -485,13 +484,7 @@ init_temp_data(struct platform_data *pdata, unsigned int cpu, int pkg_flag) > struct temp_data *tdata; > > if (!pdata->core_data) { > - /* > - * TODO: > - * The information of actual possible cores in a package is broken for now. > - * Will replace hardcoded NUM_REAL_CORES with actual per package core count > - * when this information becomes available. > - */ > - pdata->nr_cores = NUM_REAL_CORES; > + pdata->nr_cores = topology_num_cores_per_package(); Since topology_num_cores_per_package() returns a global value that is set once during boot, could this prevent sensor initialization on heterogeneous packages or during CPU hot-add? In asymmetric multi-package architectures where a secondary package has more cores than the primary, or when virtual CPUs are hot-added, pdata->nr_cores would be capped at the primary package's boot core count. When initializing extra cores, ida_alloc_max() would enforce this bound and return -ENOSPC, silently dropping temperature sensors for valid cores. Additionally, could topology_num_cores_per_package() return 0 if the CPU topology is broken or misconfigured by a hypervisor? > + pdata->core_data = kzalloc_objs(struct temp_data *, > + pdata->nr_cores); > if (!pdata->core_data) > return NULL; If pdata->nr_cores is 0, kzalloc_objs() will allocate 0 bytes and return the kernel's ZERO_SIZE_PTR instead of NULL, bypassing this error check. Later in this function, the index is allocated: } else { tdata->index = ida_alloc_max(&pdata->ida, pdata->nr_cores - 1, GFP_KERNEL); If pdata->nr_cores is 0, the maximum bound (0 - 1) evaluates to -1, which implicitly casts to UINT_MAX. ida_alloc_max() would accept this large bound and return a valid index. if (tdata->index < 0) { kfree(tdata); return NULL; } pdata->core_data[tdata->index] = tdata; Could this result in an attempt to write to the ZERO_SIZE_PTR address, causing an unhandled page fault and kernel panic? -- Sashiko AI review · https://sashiko.dev/#/patchset/20260516114253.5466-1-bakshansky.lists@gmail.com?part=1 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] hwmon: (coretemp) replace hardcoded core count with dynamic value 2026-05-16 12:16 ` sashiko-bot @ 2026-05-16 12:47 ` Guenter Roeck 0 siblings, 0 replies; 9+ messages in thread From: Guenter Roeck @ 2026-05-16 12:47 UTC (permalink / raw) To: sashiko-reviews, bakshansky.lists; +Cc: linux-hwmon On 5/16/26 05:16, sashiko-bot@kernel.org wrote: > Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: > - [High] If `topology_num_cores_per_package()` evaluates to 0, an integer underflow occurs leading to an out-of-bounds write and kernel panic. > - [High] Using the global `topology_num_cores_per_package()` limits the per-package array size, preventing sensor initialization on heterogeneous packages or during CPU hot-add. > -- > > commit d686b3577dd2107e7452adf6a949cc0720d2213a > Author: Roman Bakshansky <bakshansky.lists@gmail.com> > > hwmon: (coretemp) replace hardcoded core count with dynamic value > > This commit replaces the hardcoded limit of 512 cores per package with > a dynamic value. It uses topology_num_cores_per_package() to allocate > only the needed amount of memory for core_data. > >> diff --git a/drivers/hwmon/coretemp.c b/drivers/hwmon/coretemp.c >> index 6a0d94711eadb..c722b1d8e4804 100644 >> --- a/drivers/hwmon/coretemp.c >> +++ b/drivers/hwmon/coretemp.c > > [ ... ] > >> @@ -485,13 +484,7 @@ init_temp_data(struct platform_data *pdata, unsigned int cpu, int pkg_flag) >> struct temp_data *tdata; >> >> if (!pdata->core_data) { >> - /* >> - * TODO: >> - * The information of actual possible cores in a package is broken for now. >> - * Will replace hardcoded NUM_REAL_CORES with actual per package core count >> - * when this information becomes available. >> - */ >> - pdata->nr_cores = NUM_REAL_CORES; >> + pdata->nr_cores = topology_num_cores_per_package(); > > Since topology_num_cores_per_package() returns a global value that is set > once during boot, could this prevent sensor initialization on heterogeneous > packages or during CPU hot-add? > > In asymmetric multi-package architectures where a secondary package has more > cores than the primary, or when virtual CPUs are hot-added, pdata->nr_cores > would be capped at the primary package's boot core count. When initializing > extra cores, ida_alloc_max() would enforce this bound and return -ENOSPC, > silently dropping temperature sensors for valid cores. > > Additionally, could topology_num_cores_per_package() return 0 if the CPU > topology is broken or misconfigured by a hypervisor? > Per the submitter's statement, topology_num_cores_per_package() is stable. Other use in the kernel suggests that it can not return 0 (that would trigger a divide-by-zero crash), and other users seem to trust it. Given that, I assume the concerns raised are not valid, and I am going to apply this patch. Thanks, Guenter >> + pdata->core_data = kzalloc_objs(struct temp_data *, >> + pdata->nr_cores); >> if (!pdata->core_data) >> return NULL; > > If pdata->nr_cores is 0, kzalloc_objs() will allocate 0 bytes and return the > kernel's ZERO_SIZE_PTR instead of NULL, bypassing this error check. > > Later in this function, the index is allocated: > > } else { > tdata->index = ida_alloc_max(&pdata->ida, pdata->nr_cores - 1, GFP_KERNEL); > > If pdata->nr_cores is 0, the maximum bound (0 - 1) evaluates to -1, which > implicitly casts to UINT_MAX. ida_alloc_max() would accept this large bound > and return a valid index. > > if (tdata->index < 0) { > kfree(tdata); > return NULL; > } > pdata->core_data[tdata->index] = tdata; > > Could this result in an attempt to write to the ZERO_SIZE_PTR address, > causing an unhandled page fault and kernel panic? > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] hwmon: (coretemp) replace hardcoded core count with dynamic value 2026-05-16 11:42 ` [PATCH 1/2] hwmon: (coretemp) replace hardcoded core count with dynamic value bakshansky.lists 2026-05-16 12:16 ` sashiko-bot @ 2026-05-16 12:48 ` Guenter Roeck 1 sibling, 0 replies; 9+ messages in thread From: Guenter Roeck @ 2026-05-16 12:48 UTC (permalink / raw) To: bakshansky.lists; +Cc: linux-hwmon On Sat, May 16, 2026 at 02:42:52PM +0300, bakshansky.lists@gmail.com wrote: > From: Roman Bakshansky <bakshansky.lists@gmail.com> > > The hardcoded maximum of 512 cores per package was first defined by commit > 34cf8c657cf0 ("hwmon: (coretemp) Enlarge per package core count limit") > and later kept as a fallback with a TODO in commit 1a793caf6f69 ("hwmon: > (coretemp) Use dynamic allocated memory for core temp_data") because the > actual per-package core count was not reliably available at the time. > > Now that topology_num_cores_per_package() is stable and suitable for use, > it's time to complete the TODO and allocate only the needed amount of > memory for core_data. > > Signed-off-by: Roman Bakshansky <bakshansky.lists@gmail.com> Applied. Thanks, Guenter ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/2] hwmon: (coretemp) fix coding style issues 2026-05-16 11:42 [PATCH 0/2] hwmon: (coretemp) replace hardcoded core count and fix style issues bakshansky.lists 2026-05-16 11:42 ` [PATCH 1/2] hwmon: (coretemp) replace hardcoded core count with dynamic value bakshansky.lists @ 2026-05-16 11:42 ` bakshansky.lists 2026-05-16 12:26 ` sashiko-bot 2026-05-16 12:50 ` Guenter Roeck 1 sibling, 2 replies; 9+ messages in thread From: bakshansky.lists @ 2026-05-16 11:42 UTC (permalink / raw) To: linux-hwmon; +Cc: linux, Roman Bakshansky From: Roman Bakshansky <bakshansky.lists@gmail.com> Address several coding style warnings reported by checkpatch.pl: - Replace <asm/processor.h> with <linux/processor.h> - Add missing blank lines after declarations - Combine split quoted strings - Reorder __initconst placement No functional change. Signed-off-by: Roman Bakshansky <bakshansky.lists@gmail.com> --- drivers/hwmon/coretemp.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/hwmon/coretemp.c b/drivers/hwmon/coretemp.c index c722b1d8e480..a79c2d65a2be 100644 --- a/drivers/hwmon/coretemp.c +++ b/drivers/hwmon/coretemp.c @@ -25,7 +25,7 @@ #include <linux/moduleparam.h> #include <linux/pci.h> #include <asm/msr.h> -#include <asm/processor.h> +#include <linux/processor.h> #include <asm/cpu_device_id.h> #include <linux/sched/isolation.h> @@ -200,6 +200,7 @@ static int adjust_tjmax(struct cpuinfo_x86 *c, u32 id, struct device *dev) for (i = 0; i < ARRAY_SIZE(tjmax_model_table); i++) { const struct tjmax_model *tm = &tjmax_model_table[i]; + if (c->x86_vfm == tm->vfm && (tm->stepping_mask == ANY || tm->stepping_mask == c->x86_stepping)) @@ -222,8 +223,7 @@ static int adjust_tjmax(struct cpuinfo_x86 *c, u32 id, struct device *dev) err = rdmsr_safe_on_cpu(id, 0x17, &eax, &edx); if (err) { dev_warn(dev, - "Unable to access MSR 0x17, assuming desktop" - " CPU\n"); + "Unable to access MSR 0x17, assuming desktop CPU\n"); usemsr_ee = 0; } else if (c->x86_vfm < INTEL_CORE2_PENRYN && !(eax & 0x10000000)) { @@ -257,8 +257,7 @@ static int adjust_tjmax(struct cpuinfo_x86 *c, u32 id, struct device *dev) err = rdmsr_safe_on_cpu(id, 0xee, &eax, &edx); if (err) { dev_warn(dev, - "Unable to access MSR 0xEE, for Tjmax, left" - " at default\n"); + "Unable to access MSR 0xEE, for Tjmax, left at default\n"); } else if (eax & 0x40000000) { tjmax = tjmax_ee; } @@ -424,6 +423,7 @@ static ssize_t show_temp(struct device *dev, static int create_core_attrs(struct temp_data *tdata, struct device *dev) { int i; + static ssize_t (*const rd_ptr[TOTAL_ATTRS]) (struct device *dev, struct device_attribute *devattr, char *buf) = { show_label, show_crit_alarm, show_temp, show_tjmax, @@ -774,7 +774,7 @@ static int coretemp_cpu_offline(unsigned int cpu) } return 0; } -static const struct x86_cpu_id __initconst coretemp_ids[] = { +static const struct x86_cpu_id coretemp_ids[] __initconst = { X86_MATCH_VENDOR_FEATURE(INTEL, X86_FEATURE_DTHERM, NULL), {} }; -- 2.53.0 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] hwmon: (coretemp) fix coding style issues 2026-05-16 11:42 ` [PATCH 2/2] hwmon: (coretemp) fix coding style issues bakshansky.lists @ 2026-05-16 12:26 ` sashiko-bot 2026-05-16 12:50 ` Guenter Roeck 1 sibling, 0 replies; 9+ messages in thread From: sashiko-bot @ 2026-05-16 12:26 UTC (permalink / raw) To: bakshansky.lists; +Cc: linux-hwmon Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Low] A blank line was incorrectly added inside a contiguous block of declarations in `create_core_attrs()`, splitting the variable declarations. -- commit fd6a696ecbdee696c6e67fb77fb06e3c1baa31cc Author: Roman Bakshansky <bakshansky.lists@gmail.com> hwmon: (coretemp) fix coding style issues This commit addresses several coding style warnings reported by checkpatch.pl. It updates header inclusions, adds missing blank lines, combines split strings, and reorders the __initconst attribute. > diff --git a/drivers/hwmon/coretemp.c b/drivers/hwmon/coretemp.c > index c722b1d8e4804..a79c2d65a2bee 100644 > --- a/drivers/hwmon/coretemp.c > +++ b/drivers/hwmon/coretemp.c [ ... ] > @@ -424,6 +423,7 @@ static ssize_t show_temp(struct device *dev, > static int create_core_attrs(struct temp_data *tdata, struct device *dev) > { > int i; > + This isn't a bug, but does this newly added blank line split a contiguous block of variable declarations? The static array rd_ptr is also a variable declaration, so this separates it from the int i declaration. It looks like checkpatch.pl might have misparsed the complex function pointer array declaration as a statement. > static ssize_t (*const rd_ptr[TOTAL_ATTRS]) (struct device *dev, > struct device_attribute *devattr, char *buf) = { > show_label, show_crit_alarm, show_temp, show_tjmax, > show_ttarget }; -- Sashiko AI review · https://sashiko.dev/#/patchset/20260516114253.5466-1-bakshansky.lists@gmail.com?part=2 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] hwmon: (coretemp) fix coding style issues 2026-05-16 11:42 ` [PATCH 2/2] hwmon: (coretemp) fix coding style issues bakshansky.lists 2026-05-16 12:26 ` sashiko-bot @ 2026-05-16 12:50 ` Guenter Roeck 2026-05-16 15:10 ` bakshansky.lists 1 sibling, 1 reply; 9+ messages in thread From: Guenter Roeck @ 2026-05-16 12:50 UTC (permalink / raw) To: bakshansky.lists; +Cc: linux-hwmon On Sat, May 16, 2026 at 02:42:53PM +0300, bakshansky.lists@gmail.com wrote: > From: Roman Bakshansky <bakshansky.lists@gmail.com> > > Address several coding style warnings reported by checkpatch.pl: > - Replace <asm/processor.h> with <linux/processor.h> > - Add missing blank lines after declarations > - Combine split quoted strings > - Reorder __initconst placement > > No functional change. > > Signed-off-by: Roman Bakshansky <bakshansky.lists@gmail.com> > --- > drivers/hwmon/coretemp.c | 12 ++++++------ > 1 file changed, 6 insertions(+), 6 deletions(-) > > diff --git a/drivers/hwmon/coretemp.c b/drivers/hwmon/coretemp.c > index c722b1d8e480..a79c2d65a2be 100644 > --- a/drivers/hwmon/coretemp.c > +++ b/drivers/hwmon/coretemp.c > @@ -25,7 +25,7 @@ > #include <linux/moduleparam.h> > #include <linux/pci.h> > #include <asm/msr.h> > -#include <asm/processor.h> > +#include <linux/processor.h> > #include <asm/cpu_device_id.h> > #include <linux/sched/isolation.h> > > @@ -200,6 +200,7 @@ static int adjust_tjmax(struct cpuinfo_x86 *c, u32 id, struct device *dev) > > for (i = 0; i < ARRAY_SIZE(tjmax_model_table); i++) { > const struct tjmax_model *tm = &tjmax_model_table[i]; > + > if (c->x86_vfm == tm->vfm && > (tm->stepping_mask == ANY || > tm->stepping_mask == c->x86_stepping)) > @@ -222,8 +223,7 @@ static int adjust_tjmax(struct cpuinfo_x86 *c, u32 id, struct device *dev) > err = rdmsr_safe_on_cpu(id, 0x17, &eax, &edx); > if (err) { > dev_warn(dev, > - "Unable to access MSR 0x17, assuming desktop" > - " CPU\n"); > + "Unable to access MSR 0x17, assuming desktop CPU\n"); > usemsr_ee = 0; > } else if (c->x86_vfm < INTEL_CORE2_PENRYN && > !(eax & 0x10000000)) { > @@ -257,8 +257,7 @@ static int adjust_tjmax(struct cpuinfo_x86 *c, u32 id, struct device *dev) > err = rdmsr_safe_on_cpu(id, 0xee, &eax, &edx); > if (err) { > dev_warn(dev, > - "Unable to access MSR 0xEE, for Tjmax, left" > - " at default\n"); > + "Unable to access MSR 0xEE, for Tjmax, left at default\n"); > } else if (eax & 0x40000000) { > tjmax = tjmax_ee; > } > @@ -424,6 +423,7 @@ static ssize_t show_temp(struct device *dev, > static int create_core_attrs(struct temp_data *tdata, struct device *dev) > { > int i; > + As pointed out by Sashiko, this is a false positive. > static ssize_t (*const rd_ptr[TOTAL_ATTRS]) (struct device *dev, > struct device_attribute *devattr, char *buf) = { > show_label, show_crit_alarm, show_temp, show_tjmax, > @@ -774,7 +774,7 @@ static int coretemp_cpu_offline(unsigned int cpu) > } > return 0; > } > -static const struct x86_cpu_id __initconst coretemp_ids[] = { > +static const struct x86_cpu_id coretemp_ids[] __initconst = { checkpatch says: CHECK: Please use a blank line after function/struct/union/enum declarations #170: FILE: drivers/hwmon/coretemp.c:777: } +static const struct x86_cpu_id coretemp_ids[] __initconst = { Oh never mind, I'll fix that up and apply the patch. Guenter ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] hwmon: (coretemp) fix coding style issues 2026-05-16 12:50 ` Guenter Roeck @ 2026-05-16 15:10 ` bakshansky.lists 0 siblings, 0 replies; 9+ messages in thread From: bakshansky.lists @ 2026-05-16 15:10 UTC (permalink / raw) To: linux-hwmon; +Cc: linux > Oh never mind, I'll fix that up and apply the patch. > > Guenter Thank you for fixing that and for taking the patches, Guenter. This was my first contribution to the kernel, and your review made it a great experience. I appreciate it. Roman ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2026-05-16 15:10 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-05-16 11:42 [PATCH 0/2] hwmon: (coretemp) replace hardcoded core count and fix style issues bakshansky.lists 2026-05-16 11:42 ` [PATCH 1/2] hwmon: (coretemp) replace hardcoded core count with dynamic value bakshansky.lists 2026-05-16 12:16 ` sashiko-bot 2026-05-16 12:47 ` Guenter Roeck 2026-05-16 12:48 ` Guenter Roeck 2026-05-16 11:42 ` [PATCH 2/2] hwmon: (coretemp) fix coding style issues bakshansky.lists 2026-05-16 12:26 ` sashiko-bot 2026-05-16 12:50 ` Guenter Roeck 2026-05-16 15:10 ` bakshansky.lists
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.