On Fri, Dec 01, 2023 at 09:47:30AM -0800, Zhang, Rui wrote:
> On Thu, 2023-11-30 at 18:08 -0800, Ashok Raj wrote:
> > On Mon, Nov 27, 2023 at 09:16:51PM +0800, Zhang Rui wrote:
> > >
> > > +static struct temp_data *get_tdata(struct platform_data *pdata,
> > > int cpu)
> > > +{
> > > + struct temp_data *tdata;
> > > +
> > > + mutex_lock(&pdata->core_data_lock);
> > > + list_for_each_entry(tdata, &pdata->core_data_list, node) {
> > > + if (cpu >= 0 && !tdata->is_pkg_data && tdata-
> > > >cpu_core_id == topology_core_id(cpu))
> > > + goto found;
> > > + if (cpu < 0 && tdata->is_pkg_data)
> > > + goto found;
> > > + }
> > > + tdata = NULL;
> >
> > What used to be an array, is now a list? Is it possible to get the
> > number
> > of cores_per_package during initialization and allocate the per-core?
> > You
> > can still get them indexing from core_id and you can possibly lose
> > the
> > mutex and search?
> >
> > I don't know this code well enough... Just a thought.
>
> yeah, sadly cores_per_package is not available for now as I mentioned
> in the other email.
Couldn't we reuse the logic from Thomas's topology work that gives this
upfront based on 0x1f?
>
> >
> > > +found:
> > > + mutex_unlock(&pdata->core_data_lock);
> > > + return tdata;
> > > +}
> > > +
> > > static int create_core_data(struct platform_device *pdev, unsigned
> > > int cpu,
> > > int pkg_flag)
> > > {
> > > @@ -498,37 +511,29 @@ 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, index, attr_no;
> > > + int err, attr_no;
> > >
> > > if (!housekeeping_cpu(cpu, HK_TYPE_MISC))
> > > return 0;
> > >
> > > + tdata = get_tdata(pdata, pkg_flag ? -1 : cpu);
> > > + if (tdata)
> > > + return -EEXIST;
> > > +
> > > + tdata = init_temp_data(cpu, pkg_flag);
> >
> > Is temp_data per_cpu or per_core?
>
> it is per_core.
>
> > Wasn't sure if temp_data needs a CPU
> > number there along with cpu_core_id
>
> CPU number is needed to access the core temperature MSRs.
What if that cpu is currently offline? maybe you can simply search
for_each_online_cpu() and match core_id from cpuinfo_x86?
