From: Guenter Roeck <linux@roeck-us.net>
To: Zhang Rui <rui.zhang@intel.com>
Cc: linux-kernel@vger.kernel.org, x86@kernel.org,
linux-hwmon@vger.kernel.org, tglx@linutronix.de,
mingo@redhat.com, bp@alien8.de, dave.hansen@linux.intel.com,
hpa@zytor.com, corbet@lwn.net, fenghua.yu@intel.com,
jdelvare@suse.com, len.brown@intel.com
Subject: Re: [PATCH V2 3/8] hwmon/coretemp: Handle large core ID value
Date: Tue, 16 Aug 2022 04:16:15 -0700 [thread overview]
Message-ID: <20220816111615.GD1108868@roeck-us.net> (raw)
In-Reply-To: <20220816051633.17775-4-rui.zhang@intel.com>
On Tue, Aug 16, 2022 at 01:16:28PM +0800, Zhang Rui wrote:
> The coretemp driver supports up to a hard-coded limit of 128 cores.
>
> Today, the driver can not support a core with an ID above that limit.
> Yet, the encoding of core ID's is arbitrary (BIOS APIC-ID) and so they
> may be sparse and they may be large.
>
> Update the driver to map arbitrary core ID numbers into appropriate
> array indexes so that 128 cores can be supported, no matter the encoding
> of core ID's.
>
> Acked-by: Len Brown <len.brown@intel.com>
> Signed-off-by: Zhang Rui <rui.zhang@intel.com>
Acked-by: Guenter Roeck <linux@roeck-us.net>
> ---
> drivers/hwmon/coretemp.c | 56 +++++++++++++++++++++++++++++-----------
> 1 file changed, 41 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/hwmon/coretemp.c b/drivers/hwmon/coretemp.c
> index bfdcfe8ccb34..291566aeb703 100644
> --- a/drivers/hwmon/coretemp.c
> +++ b/drivers/hwmon/coretemp.c
> @@ -46,9 +46,6 @@ MODULE_PARM_DESC(tjmax, "TjMax value in degrees Celsius");
> #define TOTAL_ATTRS (MAX_CORE_ATTRS + 1)
> #define MAX_CORE_DATA (NUM_REAL_CORES + BASE_SYSFS_ATTR_NO)
>
> -#define TO_CORE_ID(cpu) (cpu_data(cpu).cpu_core_id)
> -#define TO_ATTR_NO(cpu) (TO_CORE_ID(cpu) + BASE_SYSFS_ATTR_NO)
> -
> #ifdef CONFIG_SMP
> #define for_each_sibling(i, cpu) \
> for_each_cpu(i, topology_sibling_cpumask(cpu))
> @@ -91,6 +88,8 @@ struct temp_data {
> struct platform_data {
> struct device *hwmon_dev;
> u16 pkg_id;
> + u16 cpu_map[NUM_REAL_CORES];
> + struct ida ida;
> struct cpumask cpumask;
> struct temp_data *core_data[MAX_CORE_DATA];
> struct device_attribute name_attr;
> @@ -441,7 +440,7 @@ static struct temp_data *init_temp_data(unsigned int cpu, int pkg_flag)
> MSR_IA32_THERM_STATUS;
> tdata->is_pkg_data = pkg_flag;
> tdata->cpu = cpu;
> - tdata->cpu_core_id = TO_CORE_ID(cpu);
> + tdata->cpu_core_id = topology_core_id(cpu);
> tdata->attr_size = MAX_CORE_ATTRS;
> mutex_init(&tdata->update_lock);
> return tdata;
> @@ -454,7 +453,7 @@ static int create_core_data(struct platform_device *pdev, unsigned int cpu,
> struct platform_data *pdata = platform_get_drvdata(pdev);
> struct cpuinfo_x86 *c = &cpu_data(cpu);
> u32 eax, edx;
> - int err, attr_no;
> + int err, index, attr_no;
>
> /*
> * Find attr number for sysfs:
> @@ -462,14 +461,26 @@ static int create_core_data(struct platform_device *pdev, unsigned int cpu,
> * The attr number is always core id + 2
> * The Pkgtemp will always show up as temp1_*, if available
> */
> - attr_no = pkg_flag ? PKG_SYSFS_ATTR_NO : TO_ATTR_NO(cpu);
> + if (pkg_flag) {
> + attr_no = PKG_SYSFS_ATTR_NO;
> + } else {
> + index = ida_alloc(&pdata->ida, GFP_KERNEL);
> + if (index < 0)
> + return index;
> + pdata->cpu_map[index] = topology_core_id(cpu);
> + attr_no = index + BASE_SYSFS_ATTR_NO;
> + }
>
> - if (attr_no > MAX_CORE_DATA - 1)
> - return -ERANGE;
> + if (attr_no > MAX_CORE_DATA - 1) {
> + err = -ERANGE;
> + goto ida_free;
> + }
>
> tdata = init_temp_data(cpu, pkg_flag);
> - if (!tdata)
> - return -ENOMEM;
> + if (!tdata) {
> + err = -ENOMEM;
> + goto ida_free;
> + }
>
> /* Test if we can access the status register */
> err = rdmsr_safe_on_cpu(cpu, tdata->status_reg, &eax, &edx);
> @@ -505,6 +516,9 @@ static int create_core_data(struct platform_device *pdev, unsigned int cpu,
> exit_free:
> pdata->core_data[attr_no] = NULL;
> kfree(tdata);
> +ida_free:
> + if (!pkg_flag)
> + ida_free(&pdata->ida, index);
> return err;
> }
>
> @@ -524,6 +538,9 @@ static void coretemp_remove_core(struct platform_data *pdata, int index)
>
> kfree(pdata->core_data[index]);
> pdata->core_data[index] = NULL;
> +
> + if (index >= BASE_SYSFS_ATTR_NO)
> + ida_free(&pdata->ida, index - BASE_SYSFS_ATTR_NO);
> }
>
> static int coretemp_probe(struct platform_device *pdev)
> @@ -537,6 +554,7 @@ static int coretemp_probe(struct platform_device *pdev)
> return -ENOMEM;
>
> pdata->pkg_id = pdev->id;
> + ida_init(&pdata->ida);
> platform_set_drvdata(pdev, pdata);
>
> pdata->hwmon_dev = devm_hwmon_device_register_with_groups(dev, DRVNAME,
> @@ -553,6 +571,7 @@ static int coretemp_remove(struct platform_device *pdev)
> if (pdata->core_data[i])
> coretemp_remove_core(pdata, i);
>
> + ida_destroy(&pdata->ida);
> return 0;
> }
>
> @@ -647,7 +666,7 @@ static int coretemp_cpu_offline(unsigned int cpu)
> struct platform_device *pdev = coretemp_get_pdev(cpu);
> struct platform_data *pd;
> struct temp_data *tdata;
> - int index, target;
> + int i, index = -1, target;
>
> /*
> * Don't execute this on suspend as the device remove locks
> @@ -660,12 +679,19 @@ static int coretemp_cpu_offline(unsigned int cpu)
> if (!pdev)
> return 0;
>
> - /* The core id is too big, just return */
> - index = TO_ATTR_NO(cpu);
> - if (index > MAX_CORE_DATA - 1)
> + pd = platform_get_drvdata(pdev);
> +
> + for (i = 0; i < NUM_REAL_CORES; i++) {
> + if (pd->cpu_map[i] == topology_core_id(cpu)) {
> + index = i + BASE_SYSFS_ATTR_NO;
> + break;
> + }
> + }
> +
> + /* Too many cores and this core is not populated, just return */
> + if (index < 0)
> return 0;
>
> - pd = platform_get_drvdata(pdev);
> tdata = pd->core_data[index];
>
> cpumask_clear_cpu(cpu, &pd->cpumask);
> --
> 2.25.1
>
next prev parent reply other threads:[~2022-08-16 11:45 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-08-16 5:16 [PATCH V2 0/8] x86/topology: Improve CPUID.1F handling Zhang Rui
2022-08-16 5:16 ` [PATCH V2 1/8] perf/x86/intel/P4: Fix smp_num_siblings usage Zhang Rui
2022-08-16 8:27 ` Peter Zijlstra
2022-08-16 9:47 ` Zhang Rui
2022-08-16 10:08 ` Peter Zijlstra
2022-08-16 5:16 ` [PATCH V2 2/8] hwmon/coretemp: Rename indx to index Zhang Rui
2022-08-16 5:16 ` [PATCH V2 3/8] hwmon/coretemp: Handle large core ID value Zhang Rui
2022-08-16 11:16 ` Guenter Roeck [this message]
2022-08-16 5:16 ` [PATCH V2 4/8] x86/topology: Fix multiple packages shown on a single-package system Zhang Rui
2022-09-21 16:26 ` Dave Hansen
2022-09-22 13:38 ` Zhang Rui
2022-08-16 5:16 ` [PATCH V2 5/8] x86/topology: Fix duplicated core ID within a package Zhang Rui
2022-08-16 5:16 ` [PATCH V2 6/8] x86/topology: Fix max_siblings calculation Zhang Rui
2022-08-16 5:16 ` [PATCH V2 7/8] Documentation: x86: Update smp_num_siblings/x86_max_cores description Zhang Rui
2022-08-16 5:16 ` [PATCH V2 8/8] Documentation: x86: Remove obsolete x86_max_dies description Zhang Rui
2022-09-20 1:57 ` [PATCH V2 0/8] x86/topology: Improve CPUID.1F handling Zhang Rui
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20220816111615.GD1108868@roeck-us.net \
--to=linux@roeck-us.net \
--cc=bp@alien8.de \
--cc=corbet@lwn.net \
--cc=dave.hansen@linux.intel.com \
--cc=fenghua.yu@intel.com \
--cc=hpa@zytor.com \
--cc=jdelvare@suse.com \
--cc=len.brown@intel.com \
--cc=linux-hwmon@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=rui.zhang@intel.com \
--cc=tglx@linutronix.de \
--cc=x86@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).