[PATCH] hwmon: (coretemp) remove duplicated coretemp for same core id

[PATCH] hwmon: (coretemp) remove duplicated coretemp for same core id

[shuwang]

From: Shu Wang <shuwang@redhat.com>

Fix kernel warning on my 4cpus 2core_id system. The cpu0 and cpu1 have
same core_id 0, so both cpu0 and cpu1 will try to create file temp2_label
when it's online.

- coretemp_cpu_online(cpu=0)
  - create_core_data(cpu=0, attr_no=2)
   - create_core_attrs(attr_no=2)
- coretemp_cpu_online(cpu=1)
  - create_core_data(cpu=1, attr_no=2)
   - create_core_attrs(attr_no=2)

$ grep -e processor -e 'core id' /proc/cpuinfo
processor       : 0
core id         : 0
processor       : 1
core id         : 0
processor       : 2
core id         : 1
processor       : 3
core id         : 1

dmesg:
sysfs: cannot create duplicate filename '/devices/platform/coretemp.0/hwmon/hwmon3/temp2_label'
sysfs: cannot create duplicate filename '/devices/platform/coretemp.0/hwmon/hwmon3/temp3_label'
WARNING: CPU: 3 PID: 27 at fs/sysfs/dir.c:31 sysfs_warn_dup+0x58/0x70
Call Trace:
 sysfs_add_file_mode_ns+0x170/0x180
 internal_create_group+0xe3/0x2c0
 sysfs_create_group+0x13/0x20
 create_core_data+0x3ab/0x5e0 [coretemp]
 coretemp_cpu_online+0x14b/0x1f7 [coretemp]
 ? create_core_data+0x5e0/0x5e0 [coretemp]
 cpuhp_invoke_callback+0xae/0x5c0
 ? __schedule+0x295/0x880
 cpuhp_thread_fun+0xcb/0x170
 smpboot_thread_fn+0x110/0x160

Signed-off-by: Shu Wang <shuwang@redhat.com>
---
 drivers/hwmon/coretemp.c | 26 +++++++++++++++-----------
 1 file changed, 15 insertions(+), 11 deletions(-)

diff --git a/drivers/hwmon/coretemp.c b/drivers/hwmon/coretemp.c
index c13a4fd86b3c..2fb29ab1080b 100644
--- a/drivers/hwmon/coretemp.c
+++ b/drivers/hwmon/coretemp.c
@@ -459,6 +459,17 @@ static struct temp_data *init_temp_data(unsigned int cpu, int pkg_flag)
 	return tdata;
 }
 
+static void coretemp_remove_core(struct platform_data *pdata, int indx)
+{
+	struct temp_data *tdata = pdata->core_data[indx];
+
+	/* Remove the sysfs attributes */
+	sysfs_remove_group(&pdata->hwmon_dev->kobj, &tdata->attr_group);
+
+	kfree(pdata->core_data[indx]);
+	pdata->core_data[indx] = NULL;
+}
+
 static int create_core_data(struct platform_device *pdev, unsigned int cpu,
 			    int pkg_flag)
 {
@@ -479,6 +490,10 @@ static int create_core_data(struct platform_device *pdev, unsigned int cpu,
 	if (attr_no > MAX_CORE_DATA - 1)
 		return -ERANGE;
 
+	tdata = pdata->core_data[attr_no];
+	if (tdata != NULL)
+		coretemp_remove_core(pdata, attr_no);
+
 	tdata = init_temp_data(cpu, pkg_flag);
 	if (!tdata)
 		return -ENOMEM;
@@ -527,17 +542,6 @@ coretemp_add_core(struct platform_device *pdev, unsigned int cpu, int pkg_flag)
 		dev_err(&pdev->dev, "Adding Core %u failed\n", cpu);
 }
 
-static void coretemp_remove_core(struct platform_data *pdata, int indx)
-{
-	struct temp_data *tdata = pdata->core_data[indx];
-
-	/* Remove the sysfs attributes */
-	sysfs_remove_group(&pdata->hwmon_dev->kobj, &tdata->attr_group);
-
-	kfree(pdata->core_data[indx]);
-	pdata->core_data[indx] = NULL;
-}
-
 static int coretemp_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
-- 
2.13.5

Re: [PATCH] hwmon: (coretemp) remove duplicated coretemp for same core id

[Guenter Roeck]

On Tue, Oct 17, 2017 at 04:44:50PM +0800, shuwang@redhat.com wrote:
> From: Shu Wang <shuwang@redhat.com>
> 
> Fix kernel warning on my 4cpus 2core_id system. The cpu0 and cpu1 have
> same core_id 0, so both cpu0 and cpu1 will try to create file temp2_label
> when it's online.
> 
What system/cpu is that ?

Normally I would assume that each CPU (package) instantiates
a separate instance of the driver.

> - coretemp_cpu_online(cpu=0)
>   - create_core_data(cpu=0, attr_no=2)
>    - create_core_attrs(attr_no=2)
> - coretemp_cpu_online(cpu=1)
>   - create_core_data(cpu=1, attr_no=2)
>    - create_core_attrs(attr_no=2)
> 
> $ grep -e processor -e 'core id' /proc/cpuinfo
> processor       : 0
> core id         : 0
> processor       : 1
> core id         : 0
> processor       : 2
> core id         : 1
> processor       : 3
> core id         : 1

Complete output of /proc/cpuinfo might be helpful.

> 
> dmesg:
> sysfs: cannot create duplicate filename '/devices/platform/coretemp.0/hwmon/hwmon3/temp2_label'
> sysfs: cannot create duplicate filename '/devices/platform/coretemp.0/hwmon/hwmon3/temp3_label'
> WARNING: CPU: 3 PID: 27 at fs/sysfs/dir.c:31 sysfs_warn_dup+0x58/0x70
> Call Trace:
>  sysfs_add_file_mode_ns+0x170/0x180
>  internal_create_group+0xe3/0x2c0
>  sysfs_create_group+0x13/0x20
>  create_core_data+0x3ab/0x5e0 [coretemp]
>  coretemp_cpu_online+0x14b/0x1f7 [coretemp]
>  ? create_core_data+0x5e0/0x5e0 [coretemp]
>  cpuhp_invoke_callback+0xae/0x5c0
>  ? __schedule+0x295/0x880
>  cpuhp_thread_fun+0xcb/0x170
>  smpboot_thread_fn+0x110/0x160
> 
> Signed-off-by: Shu Wang <shuwang@redhat.com>
> ---
>  drivers/hwmon/coretemp.c | 26 +++++++++++++++-----------
>  1 file changed, 15 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/hwmon/coretemp.c b/drivers/hwmon/coretemp.c
> index c13a4fd86b3c..2fb29ab1080b 100644
> --- a/drivers/hwmon/coretemp.c
> +++ b/drivers/hwmon/coretemp.c
> @@ -459,6 +459,17 @@ static struct temp_data *init_temp_data(unsigned int cpu, int pkg_flag)
>  	return tdata;
>  }
>  
> +static void coretemp_remove_core(struct platform_data *pdata, int indx)
> +{
> +	struct temp_data *tdata = pdata->core_data[indx];
> +
> +	/* Remove the sysfs attributes */
> +	sysfs_remove_group(&pdata->hwmon_dev->kobj, &tdata->attr_group);
> +
> +	kfree(pdata->core_data[indx]);
> +	pdata->core_data[indx] = NULL;
> +}
> +
>  static int create_core_data(struct platform_device *pdev, unsigned int cpu,
>  			    int pkg_flag)
>  {
> @@ -479,6 +490,10 @@ static int create_core_data(struct platform_device *pdev, unsigned int cpu,
>  	if (attr_no > MAX_CORE_DATA - 1)
>  		return -ERANGE;
>  
> +	tdata = pdata->core_data[attr_no];
> +	if (tdata != NULL)
> +		coretemp_remove_core(pdata, attr_no);
> +

That looks more like a hack than a fix. Presumably the other cpu/core
is still online ?

>  	tdata = init_temp_data(cpu, pkg_flag);
>  	if (!tdata)
>  		return -ENOMEM;
> @@ -527,17 +542,6 @@ coretemp_add_core(struct platform_device *pdev, unsigned int cpu, int pkg_flag)
>  		dev_err(&pdev->dev, "Adding Core %u failed\n", cpu);
>  }
>  
> -static void coretemp_remove_core(struct platform_data *pdata, int indx)
> -{
> -	struct temp_data *tdata = pdata->core_data[indx];
> -
> -	/* Remove the sysfs attributes */
> -	sysfs_remove_group(&pdata->hwmon_dev->kobj, &tdata->attr_group);
> -
> -	kfree(pdata->core_data[indx]);
> -	pdata->core_data[indx] = NULL;
> -}
> -
>  static int coretemp_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
> -- 
> 2.13.5
> 

Re: [PATCH] hwmon: (coretemp) remove duplicated coretemp for same core id

[Shu Wang]

> From: "Guenter Roeck" <linux@roeck-us.net>
> To: shuwang@redhat.com
> Cc: "fenghua yu" <fenghua.yu@intel.com>, jdelvare@suse.com, linux-hwmon@vger.kernel.org,
> linux-kernel@vger.kernel.org, chuhu@redhat.com, yizhan@redhat.com
> Sent: Tuesday, October 17, 2017 11:25:50 PM
> Subject: Re: [PATCH] hwmon: (coretemp) remove duplicated coretemp for same core id
> 
> On Tue, Oct 17, 2017 at 04:44:50PM +0800, shuwang@redhat.com wrote:
> > From: Shu Wang <shuwang@redhat.com>
> > 
> > Fix kernel warning on my 4cpus 2core_id system. The cpu0 and cpu1 have
> > same core_id 0, so both cpu0 and cpu1 will try to create file temp2_label
> > when it's online.
> > 
> What system/cpu is that ?
> 
> Normally I would assume that each CPU (package) instantiates
> a separate instance of the driver.

The system is ThinkPad X1 Carbon 3rd laptop, model 20BTS1N70F.

> 
> > - coretemp_cpu_online(cpu=0)
> >   - create_core_data(cpu=0, attr_no=2)
> >    - create_core_attrs(attr_no=2)
> > - coretemp_cpu_online(cpu=1)
> >   - create_core_data(cpu=1, attr_no=2)
> >    - create_core_attrs(attr_no=2)
> > 
> > $ grep -e processor -e 'core id' /proc/cpuinfo
> > processor       : 0
> > core id         : 0
> > processor       : 1
> > core id         : 0
> > processor       : 2
> > core id         : 1
> > processor       : 3
> > core id         : 1
> 
> Complete output of /proc/cpuinfo might be helpful.

$ cat /proc/cpuinfo 
processor	: 0
vendor_id	: GenuineIntel
cpu family	: 6
model		: 61
model name	: Intel(R) Core(TM) i7-5600U CPU @ 2.60GHz
stepping	: 4
microcode	: 0x25
cpu MHz		: 2593.949
cache size	: 4096 KB
physical id	: 0
siblings	: 4
core id		: 0
cpu cores	: 4
apicid		: 0
initial apicid	: 0
fpu		: yes
fpu_exception	: yes
cpuid level	: 20
wp		: yes
flags		: fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov pat pse36 clflush dts acpi mmx fxsr sse sse2 ss ht tm pbe syscall nx pdpe1gb rdtscp lm constant_tsc arch_perfmon pebs bts rep_good nopl xtopology nonstop_tsc cpuid aperfmperf pni pclmulqdq dtes64 monitor ds_cpl vmx smx est tm2 ssse3 sdbg fma cx16 xtpr pdcm pcid sse4_1 sse4_2 x2apic movbe popcnt tsc_deadline_timer aes xsave avx f16c rdrand lahf_lm abm 3dnowprefetch cpuid_fault epb intel_pt tpr_shadow vnmi flexpriority ept vpid fsgsbase tsc_adjust bmi1 hle avx2 smep bmi2 erms invpcid rtm rdseed adx smap xsaveopt dtherm ida arat pln pts
bugs		:
bogomips	: 5187.89
clflush size	: 64
cache_alignment	: 64
address sizes	: 39 bits physical, 48 bits virtual
power management:

processor	: 1
vendor_id	: GenuineIntel
cpu family	: 6
model		: 61
model name	: Intel(R) Core(TM) i7-5600U CPU @ 2.60GHz
stepping	: 4
microcode	: 0x25
cpu MHz		: 2593.949
cache size	: 4096 KB
physical id	: 0
siblings	: 4
core id		: 0
cpu cores	: 4
apicid		: 1
initial apicid	: 1
fpu		: yes
fpu_exception	: yes
cpuid level	: 20
wp		: yes
flags		: fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov pat pse36 clflush dts acpi mmx fxsr sse sse2 ss ht tm pbe syscall nx pdpe1gb rdtscp lm constant_tsc arch_perfmon pebs bts rep_good nopl xtopology nonstop_tsc cpuid aperfmperf pni pclmulqdq dtes64 monitor ds_cpl vmx smx est tm2 ssse3 sdbg fma cx16 xtpr pdcm pcid sse4_1 sse4_2 x2apic movbe popcnt tsc_deadline_timer aes xsave avx f16c rdrand lahf_lm abm 3dnowprefetch cpuid_fault epb intel_pt tpr_shadow vnmi flexpriority ept vpid fsgsbase tsc_adjust bmi1 hle avx2 smep bmi2 erms invpcid rtm rdseed adx smap xsaveopt dtherm ida arat pln pts
bugs		:
bogomips	: 5209.02
clflush size	: 64
cache_alignment	: 64
address sizes	: 39 bits physical, 48 bits virtual
power management:

processor	: 2
vendor_id	: GenuineIntel
cpu family	: 6
model		: 61
model name	: Intel(R) Core(TM) i7-5600U CPU @ 2.60GHz
stepping	: 4
microcode	: 0x25
cpu MHz		: 2593.949
cache size	: 4096 KB
physical id	: 0
siblings	: 4
core id		: 1
cpu cores	: 4
apicid		: 2
initial apicid	: 2
fpu		: yes
fpu_exception	: yes
cpuid level	: 20
wp		: yes
flags		: fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov pat pse36 clflush dts acpi mmx fxsr sse sse2 ss ht tm pbe syscall nx pdpe1gb rdtscp lm constant_tsc arch_perfmon pebs bts rep_good nopl xtopology nonstop_tsc cpuid aperfmperf pni pclmulqdq dtes64 monitor ds_cpl vmx smx est tm2 ssse3 sdbg fma cx16 xtpr pdcm pcid sse4_1 sse4_2 x2apic movbe popcnt tsc_deadline_timer aes xsave avx f16c rdrand lahf_lm abm 3dnowprefetch cpuid_fault epb intel_pt tpr_shadow vnmi flexpriority ept vpid fsgsbase tsc_adjust bmi1 hle avx2 smep bmi2 erms invpcid rtm rdseed adx smap xsaveopt dtherm ida arat pln pts
bugs		:
bogomips	: 5201.77
clflush size	: 64
cache_alignment	: 64
address sizes	: 39 bits physical, 48 bits virtual
power management:

processor	: 3
vendor_id	: GenuineIntel
cpu family	: 6
model		: 61
model name	: Intel(R) Core(TM) i7-5600U CPU @ 2.60GHz
stepping	: 4
microcode	: 0x25
cpu MHz		: 2593.949
cache size	: 4096 KB
physical id	: 0
siblings	: 4
core id		: 1
cpu cores	: 4
apicid		: 3
initial apicid	: 3
fpu		: yes
fpu_exception	: yes
cpuid level	: 20
wp		: yes
flags		: fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov pat pse36 clflush dts acpi mmx fxsr sse sse2 ss ht tm pbe syscall nx pdpe1gb rdtscp lm constant_tsc arch_perfmon pebs bts rep_good nopl xtopology nonstop_tsc cpuid aperfmperf pni pclmulqdq dtes64 monitor ds_cpl vmx smx est tm2 ssse3 sdbg fma cx16 xtpr pdcm pcid sse4_1 sse4_2 x2apic movbe popcnt tsc_deadline_timer aes xsave avx f16c rdrand lahf_lm abm 3dnowprefetch cpuid_fault epb intel_pt tpr_shadow vnmi flexpriority ept vpid fsgsbase tsc_adjust bmi1 hle avx2 smep bmi2 erms invpcid rtm rdseed adx smap xsaveopt dtherm ida arat pln pts
bugs		:
bogomips	: 5202.42
clflush size	: 64
cache_alignment	: 64
address sizes	: 39 bits physical, 48 bits virtual
power management:

> 
> > 
> > dmesg:
> > sysfs: cannot create duplicate filename
> > '/devices/platform/coretemp.0/hwmon/hwmon3/temp2_label'
> > sysfs: cannot create duplicate filename
> > '/devices/platform/coretemp.0/hwmon/hwmon3/temp3_label'
> > WARNING: CPU: 3 PID: 27 at fs/sysfs/dir.c:31 sysfs_warn_dup+0x58/0x70
> > Call Trace:
> >  sysfs_add_file_mode_ns+0x170/0x180
> >  internal_create_group+0xe3/0x2c0
> >  sysfs_create_group+0x13/0x20
> >  create_core_data+0x3ab/0x5e0 [coretemp]
> >  coretemp_cpu_online+0x14b/0x1f7 [coretemp]
> >  ? create_core_data+0x5e0/0x5e0 [coretemp]
> >  cpuhp_invoke_callback+0xae/0x5c0
> >  ? __schedule+0x295/0x880
> >  cpuhp_thread_fun+0xcb/0x170
> >  smpboot_thread_fn+0x110/0x160
> > 
> > Signed-off-by: Shu Wang <shuwang@redhat.com>
> > ---
> >  drivers/hwmon/coretemp.c | 26 +++++++++++++++-----------
> >  1 file changed, 15 insertions(+), 11 deletions(-)
> > 
> > diff --git a/drivers/hwmon/coretemp.c b/drivers/hwmon/coretemp.c
> > index c13a4fd86b3c..2fb29ab1080b 100644
> > --- a/drivers/hwmon/coretemp.c
> > +++ b/drivers/hwmon/coretemp.c
> > @@ -459,6 +459,17 @@ static struct temp_data *init_temp_data(unsigned int
> > cpu, int pkg_flag)
> >  	return tdata;
> >  }
> >  
> > +static void coretemp_remove_core(struct platform_data *pdata, int indx)
> > +{
> > +	struct temp_data *tdata = pdata->core_data[indx];
> > +
> > +	/* Remove the sysfs attributes */
> > +	sysfs_remove_group(&pdata->hwmon_dev->kobj, &tdata->attr_group);
> > +
> > +	kfree(pdata->core_data[indx]);
> > +	pdata->core_data[indx] = NULL;
> > +}
> > +
> >  static int create_core_data(struct platform_device *pdev, unsigned int
> >  cpu,
> >  			    int pkg_flag)
> >  {
> > @@ -479,6 +490,10 @@ static int create_core_data(struct platform_device
> > *pdev, unsigned int cpu,
> >  	if (attr_no > MAX_CORE_DATA - 1)
> >  		return -ERANGE;
> >  
> > +	tdata = pdata->core_data[attr_no];
> > +	if (tdata != NULL)
> > +		coretemp_remove_core(pdata, attr_no);
> > +
> 
> That looks more like a hack than a fix. Presumably the other cpu/core
> is still online ?
> 
> >  	tdata = init_temp_data(cpu, pkg_flag);
> >  	if (!tdata)
> >  		return -ENOMEM;
> > @@ -527,17 +542,6 @@ coretemp_add_core(struct platform_device *pdev,
> > unsigned int cpu, int pkg_flag)
> >  		dev_err(&pdev->dev, "Adding Core %u failed\n", cpu);
> >  }
> >  
> > -static void coretemp_remove_core(struct platform_data *pdata, int indx)
> > -{
> > -	struct temp_data *tdata = pdata->core_data[indx];
> > -
> > -	/* Remove the sysfs attributes */
> > -	sysfs_remove_group(&pdata->hwmon_dev->kobj, &tdata->attr_group);
> > -
> > -	kfree(pdata->core_data[indx]);
> > -	pdata->core_data[indx] = NULL;
> > -}
> > -
> >  static int coretemp_probe(struct platform_device *pdev)
> >  {
> >  	struct device *dev = &pdev->dev;
> > --
> > 2.13.5
> > 
> 

Re: [PATCH] hwmon: (coretemp) remove duplicated coretemp for same core id

[Guenter Roeck]

On 10/17/2017 08:21 PM, Shu Wang wrote:
>> From: "Guenter Roeck" <linux@roeck-us.net>
>> To: shuwang@redhat.com
>> Cc: "fenghua yu" <fenghua.yu@intel.com>, jdelvare@suse.com, linux-hwmon@vger.kernel.org,
>> linux-kernel@vger.kernel.org, chuhu@redhat.com, yizhan@redhat.com
>> Sent: Tuesday, October 17, 2017 11:25:50 PM
>> Subject: Re: [PATCH] hwmon: (coretemp) remove duplicated coretemp for same core id
>>
>> On Tue, Oct 17, 2017 at 04:44:50PM +0800, shuwang@redhat.com wrote:
>>> From: Shu Wang <shuwang@redhat.com>
>>>
>>> Fix kernel warning on my 4cpus 2core_id system. The cpu0 and cpu1 have
>>> same core_id 0, so both cpu0 and cpu1 will try to create file temp2_label
>>> when it's online.
>>>
>> What system/cpu is that ?
>>
>> Normally I would assume that each CPU (package) instantiates
>> a separate instance of the driver.
> 
> The system is ThinkPad X1 Carbon 3rd laptop, model 20BTS1N70F.
> 
>>
>>> - coretemp_cpu_online(cpu=0)
>>>    - create_core_data(cpu=0, attr_no=2)
>>>     - create_core_attrs(attr_no=2)
>>> - coretemp_cpu_online(cpu=1)
>>>    - create_core_data(cpu=1, attr_no=2)
>>>     - create_core_attrs(attr_no=2)
>>>
>>> $ grep -e processor -e 'core id' /proc/cpuinfo
>>> processor       : 0
>>> core id         : 0
>>> processor       : 1
>>> core id         : 0
>>> processor       : 2
>>> core id         : 1
>>> processor       : 3
>>> core id         : 1
>>
>> Complete output of /proc/cpuinfo might be helpful.
> 
> $ cat /proc/cpuinfo
> processor	: 0
> vendor_id	: GenuineIntel
> cpu family	: 6
> model		: 61
> model name	: Intel(R) Core(TM) i7-5600U CPU @ 2.60GHz

This is a hyperthreading CPU, which should already be handled,
and the problem would affect pretty much everyone. I'll have
to look into this more closely. Is this with the ToT kernel ?

Guenter

Re: [PATCH] hwmon: (coretemp) remove duplicated coretemp for same core id

[Shu Wang]

> From: "Guenter Roeck" <linux@roeck-us.net>
> To: "Shu Wang" <shuwang@redhat.com>
> Cc: "fenghua yu" <fenghua.yu@intel.com>, jdelvare@suse.com, linux-hwmon@vger.kernel.org,
> linux-kernel@vger.kernel.org, chuhu@redhat.com, yizhan@redhat.com
> Sent: Wednesday, October 18, 2017 9:14:39 PM
> Subject: Re: [PATCH] hwmon: (coretemp) remove duplicated coretemp for same core id
> 
> On 10/17/2017 08:21 PM, Shu Wang wrote:
> >> From: "Guenter Roeck" <linux@roeck-us.net>
> >> To: shuwang@redhat.com
> >> Cc: "fenghua yu" <fenghua.yu@intel.com>, jdelvare@suse.com,
> >> linux-hwmon@vger.kernel.org,
> >> linux-kernel@vger.kernel.org, chuhu@redhat.com, yizhan@redhat.com
> >> Sent: Tuesday, October 17, 2017 11:25:50 PM
> >> Subject: Re: [PATCH] hwmon: (coretemp) remove duplicated coretemp for same
> >> core id
> >>
> >> On Tue, Oct 17, 2017 at 04:44:50PM +0800, shuwang@redhat.com wrote:
> >>> From: Shu Wang <shuwang@redhat.com>
> >>>
> >>> Fix kernel warning on my 4cpus 2core_id system. The cpu0 and cpu1 have
> >>> same core_id 0, so both cpu0 and cpu1 will try to create file temp2_label
> >>> when it's online.
> >>>
> >> What system/cpu is that ?
> >>
> >> Normally I would assume that each CPU (package) instantiates
> >> a separate instance of the driver.
> > 
> > The system is ThinkPad X1 Carbon 3rd laptop, model 20BTS1N70F.
> > 
> >>
> >>> - coretemp_cpu_online(cpu=0)
> >>>    - create_core_data(cpu=0, attr_no=2)
> >>>     - create_core_attrs(attr_no=2)
> >>> - coretemp_cpu_online(cpu=1)
> >>>    - create_core_data(cpu=1, attr_no=2)
> >>>     - create_core_attrs(attr_no=2)
> >>>
> >>> $ grep -e processor -e 'core id' /proc/cpuinfo
> >>> processor       : 0
> >>> core id         : 0
> >>> processor       : 1
> >>> core id         : 0
> >>> processor       : 2
> >>> core id         : 1
> >>> processor       : 3
> >>> core id         : 1
> >>
> >> Complete output of /proc/cpuinfo might be helpful.
> > 
> > $ cat /proc/cpuinfo
> > processor	: 0
> > vendor_id	: GenuineIntel
> > cpu family	: 6
> > model		: 61
> > model name	: Intel(R) Core(TM) i7-5600U CPU @ 2.60GHz
> 
> This is a hyperthreading CPU, which should already be handled,

Do you mean that for my system, coretemp_cpu_online should only
be called twice instead of four times to create two core attrs?

> and the problem would affect pretty much everyone. I'll have
> to look into this more closely. Is this with the ToT kernel ?

What's a ToT kernel? The kernel I built was the latest
kernel-4.14.0_rc5+ from linus repo.

> 
> Guenter
> 

Re: [PATCH] hwmon: (coretemp) remove duplicated coretemp for same core id

[Guenter Roeck]

On 10/18/2017 07:28 PM, Shu Wang wrote:
>> From: "Guenter Roeck" <linux@roeck-us.net>
>> To: "Shu Wang" <shuwang@redhat.com>
>> Cc: "fenghua yu" <fenghua.yu@intel.com>, jdelvare@suse.com, linux-hwmon@vger.kernel.org,
>> linux-kernel@vger.kernel.org, chuhu@redhat.com, yizhan@redhat.com
>> Sent: Wednesday, October 18, 2017 9:14:39 PM
>> Subject: Re: [PATCH] hwmon: (coretemp) remove duplicated coretemp for same core id
>>
>> On 10/17/2017 08:21 PM, Shu Wang wrote:
>>>> From: "Guenter Roeck" <linux@roeck-us.net>
>>>> To: shuwang@redhat.com
>>>> Cc: "fenghua yu" <fenghua.yu@intel.com>, jdelvare@suse.com,
>>>> linux-hwmon@vger.kernel.org,
>>>> linux-kernel@vger.kernel.org, chuhu@redhat.com, yizhan@redhat.com
>>>> Sent: Tuesday, October 17, 2017 11:25:50 PM
>>>> Subject: Re: [PATCH] hwmon: (coretemp) remove duplicated coretemp for same
>>>> core id
>>>>
>>>> On Tue, Oct 17, 2017 at 04:44:50PM +0800, shuwang@redhat.com wrote:
>>>>> From: Shu Wang <shuwang@redhat.com>
>>>>>
>>>>> Fix kernel warning on my 4cpus 2core_id system. The cpu0 and cpu1 have
>>>>> same core_id 0, so both cpu0 and cpu1 will try to create file temp2_label
>>>>> when it's online.
>>>>>
>>>> What system/cpu is that ?
>>>>
>>>> Normally I would assume that each CPU (package) instantiates
>>>> a separate instance of the driver.
>>>
>>> The system is ThinkPad X1 Carbon 3rd laptop, model 20BTS1N70F.
>>>
>>>>
>>>>> - coretemp_cpu_online(cpu=0)
>>>>>     - create_core_data(cpu=0, attr_no=2)
>>>>>      - create_core_attrs(attr_no=2)
>>>>> - coretemp_cpu_online(cpu=1)
>>>>>     - create_core_data(cpu=1, attr_no=2)
>>>>>      - create_core_attrs(attr_no=2)
>>>>>
>>>>> $ grep -e processor -e 'core id' /proc/cpuinfo
>>>>> processor       : 0
>>>>> core id         : 0
>>>>> processor       : 1
>>>>> core id         : 0
>>>>> processor       : 2
>>>>> core id         : 1
>>>>> processor       : 3
>>>>> core id         : 1
>>>>
>>>> Complete output of /proc/cpuinfo might be helpful.
>>>
>>> $ cat /proc/cpuinfo
>>> processor	: 0
>>> vendor_id	: GenuineIntel
>>> cpu family	: 6
>>> model		: 61
>>> model name	: Intel(R) Core(TM) i7-5600U CPU @ 2.60GHz
>>
>> This is a hyperthreading CPU, which should already be handled,
> 
> Do you mean that for my system, coretemp_cpu_online should only
> be called twice instead of four times to create two core attrs?
> 

coretemp_add_core() should only be called twice, and cpumask_intersects()
should filter out the duplicate ones.

         /*
          * Check whether a thread sibling is already online. If not add the
          * interface for this CPU core.
          */
         if (!cpumask_intersects(&pdata->cpumask, topology_sibling_cpumask(cpu)))
                 coretemp_add_core(pdev, cpu, 0);

Thomas, is it possible that something is wrong with this code ?

Guenter

>> and the problem would affect pretty much everyone. I'll have
>> to look into this more closely. Is this with the ToT kernel ?
> 
> What's a ToT kernel? The kernel I built was the latest
> kernel-4.14.0_rc5+ from linus repo.
> 

ToT => Top Of Tree. My apologies, I thought that was a commonly used acronym.

Guenter

Re: [PATCH] hwmon: (coretemp) remove duplicated coretemp for same core id

[Thomas Gleixner]

On Wed, 18 Oct 2017, Guenter Roeck wrote:
> On 10/18/2017 07:28 PM, Shu Wang wrote:

> > > > > > Fix kernel warning on my 4cpus 2core_id system. The cpu0 and cpu1
> > > > > > have same core_id 0, so both cpu0 and cpu1 will try to create file
> > > > > > temp2_label when it's online.
> > > > > > 
> > > > > 
> > > > > > - coretemp_cpu_online(cpu=0)
> > > > > >     - create_core_data(cpu=0, attr_no=2)
> > > > > >      - create_core_attrs(attr_no=2)
> > > > > > - coretemp_cpu_online(cpu=1)
> > > > > >     - create_core_data(cpu=1, attr_no=2)
> > > > > >      - create_core_attrs(attr_no=2)
> > > > > > 
> > > > > > $ grep -e processor -e 'core id' /proc/cpuinfo
> > > > > > processor       : 0
> > > > > > core id         : 0
> > > > > > processor       : 1
> > > > > > core id         : 0
> > > > > > processor       : 2
> > > > > > core id         : 1
> > > > > > processor       : 3
> > > > > > core id         : 1
> > > > > 
> > > > > Complete output of /proc/cpuinfo might be helpful.
> > > > 
> > > > $ cat /proc/cpuinfo
> > > > processor	: 0
> > > > vendor_id	: GenuineIntel
> > > > cpu family	: 6
> > > > model		: 61
> > > > model name	: Intel(R) Core(TM) i7-5600U CPU @ 2.60GHz
> > > 
> > > This is a hyperthreading CPU, which should already be handled,
> > 
> > Do you mean that for my system, coretemp_cpu_online should only
> > be called twice instead of four times to create two core attrs?
> > 
> 
> coretemp_add_core() should only be called twice, and cpumask_intersects()
> should filter out the duplicate ones.
> 
>         /*
>          * Check whether a thread sibling is already online. If not add the
>          * interface for this CPU core.
>          */
>         if (!cpumask_intersects(&pdata->cpumask,
> topology_sibling_cpumask(cpu)))
>                 coretemp_add_core(pdev, cpu, 0);
> 
> Thomas, is it possible that something is wrong with this code ?

Hrmm. Not that I can see. The only thing I can think of is that the logical
package association of the CPUs is screwed.

Debug patch below.

Thanks,

	tglx

8<------------
--- a/drivers/hwmon/coretemp.c
+++ b/drivers/hwmon/coretemp.c
@@ -611,6 +611,9 @@ static int coretemp_cpu_online(unsigned
 	if (cpuhp_tasks_frozen)
 		return 0;
 
+	pr_info("CPU %u %u pkgid %d\n", cpu, smp_processor_id(),
+		topology_logical_package_id(cpu));
+
 	/*
 	 * CPUID.06H.EAX[0] indicates whether the CPU has thermal
 	 * sensors. We check this bit only, all the early CPUs

Re: [PATCH] hwmon: (coretemp) remove duplicated coretemp for same core id

[Shu Wang]

> From: "Thomas Gleixner" <tglx@linutronix.de>
> To: "Guenter Roeck" <linux@roeck-us.net>
> Cc: "Shu Wang" <shuwang@redhat.com>, "fenghua yu" <fenghua.yu@intel.com>, jdelvare@suse.com,
> linux-hwmon@vger.kernel.org, linux-kernel@vger.kernel.org, chuhu@redhat.com, yizhan@redhat.com
> Sent: Thursday, October 19, 2017 4:02:50 PM
> Subject: Re: [PATCH] hwmon: (coretemp) remove duplicated coretemp for same core id
> 
> On Wed, 18 Oct 2017, Guenter Roeck wrote:
> > On 10/18/2017 07:28 PM, Shu Wang wrote:
> 
> > > > > > > Fix kernel warning on my 4cpus 2core_id system. The cpu0 and cpu1
> > > > > > > have same core_id 0, so both cpu0 and cpu1 will try to create
> > > > > > > file
> > > > > > > temp2_label when it's online.
> > > > > > > 
> > > > > > 
> > > > > > > - coretemp_cpu_online(cpu=0)
> > > > > > >     - create_core_data(cpu=0, attr_no=2)
> > > > > > >      - create_core_attrs(attr_no=2)
> > > > > > > - coretemp_cpu_online(cpu=1)
> > > > > > >     - create_core_data(cpu=1, attr_no=2)
> > > > > > >      - create_core_attrs(attr_no=2)
> > > > > > > 
> > > > > > > $ grep -e processor -e 'core id' /proc/cpuinfo
> > > > > > > processor       : 0
> > > > > > > core id         : 0
> > > > > > > processor       : 1
> > > > > > > core id         : 0
> > > > > > > processor       : 2
> > > > > > > core id         : 1
> > > > > > > processor       : 3
> > > > > > > core id         : 1
> > > > > > 
> > > > > > Complete output of /proc/cpuinfo might be helpful.
> > > > > 
> > > > > $ cat /proc/cpuinfo
> > > > > processor	: 0
> > > > > vendor_id	: GenuineIntel
> > > > > cpu family	: 6
> > > > > model		: 61
> > > > > model name	: Intel(R) Core(TM) i7-5600U CPU @ 2.60GHz
> > > > 
> > > > This is a hyperthreading CPU, which should already be handled,
> > > 
> > > Do you mean that for my system, coretemp_cpu_online should only
> > > be called twice instead of four times to create two core attrs?
> > > 
> > 
> > coretemp_add_core() should only be called twice, and cpumask_intersects()
> > should filter out the duplicate ones.
> > 
> >         /*
> >          * Check whether a thread sibling is already online. If not add the
> >          * interface for this CPU core.
> >          */
> >         if (!cpumask_intersects(&pdata->cpumask,
> > topology_sibling_cpumask(cpu)))
> >                 coretemp_add_core(pdev, cpu, 0);
> > 
> > Thomas, is it possible that something is wrong with this code ?

Sorry, I got the root cause, not coretemp's problem. I enabled numa=fake=2
cmdline param to simulate 2 nodes on my systems. 2 threads of the a same core
happened to be on different node, so they are not siblings.
  
Thanks for you reply

> 
> Hrmm. Not that I can see. The only thing I can think of is that the logical
> package association of the CPUs is screwed.
> 
> Debug patch below.
> 
> Thanks,