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@xxxxxxxxxxxx>
> > To: "Shu Wang" <shuwang@xxxxxxxxxx>
> > Cc: "fenghua yu" <fenghua.yu@xxxxxxxxx>, jdelvare@xxxxxxxx, linux-hwmon@xxxxxxxxxxxxxxx,
> > linux-kernel@xxxxxxxxxxxxxxx, chuhu@xxxxxxxxxx, yizhan@xxxxxxxxxx
> > 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@xxxxxxxxxxxx>
> > > > To: shuwang@xxxxxxxxxx
> > > > Cc: "fenghua yu" <fenghua.yu@xxxxxxxxx>, jdelvare@xxxxxxxx,
> > > > linux-hwmon@xxxxxxxxxxxxxxx,
> > > > linux-kernel@xxxxxxxxxxxxxxx, chuhu@xxxxxxxxxx, yizhan@xxxxxxxxxx
> > > > 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@xxxxxxxxxx wrote:
> > > > > From: Shu Wang <shuwang@xxxxxxxxxx>
> > > > >
> > > > > 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