Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:6 as permitted sender) client-ip=2620:137:e000::3:6;
Date:   Fri, 1 Dec 2023 14:58:18 -0800
From:   Ashok Raj <ashok.raj@intel.com>
To:     "Zhang, Rui" <rui.zhang@intel.com>
CC:     "ashok_raj@linux.intel.com" <ashok_raj@linux.intel.com>,
        "linux@roeck-us.net" <linux@roeck-us.net>,
        "Yu, Fenghua" <fenghua.yu@intel.com>,
        "linux-hwmon@vger.kernel.org" <linux-hwmon@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "jdelvare@suse.com" <jdelvare@suse.com>,
        Ashok Raj <ashok.raj@intel.com>
Subject: Re: [PATCH 3/3] hwmon: (coretemp) Fix core count limitation
Message-ID: <ZWplCq1hg9gydfEy@a4bf019067fa.jf.intel.com>
References: <20231127131651.476795-1-rui.zhang@intel.com>
 <20231127131651.476795-4-rui.zhang@intel.com>
 <ZWlAKS2iK9Oy+7tQ@araj-dh-work.jf.intel.com>
 <9f97885ace55b6f928160a178bc0ec505ac6eeb3.camel@intel.com>
Content-Type: text/plain; charset="iso-8859-1"
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <9f97885ace55b6f928160a178bc0ec505ac6eeb3.camel@intel.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?iso-8859-1?Q?7Y9yrQX1YL4UA8lNNmdohrVWcDWFTt7TW/7XEIWcHFYmPBsSmy3mdaLkbe?=
 =?iso-8859-1?Q?ayPILe2r/gMut7+wCuU6sKn4fwJqoQmni1P7StgIHFyr1j51d43h6ecrpf?=
 =?iso-8859-1?Q?r3nC58R9YCrub3kTWiBq3+si092aToIOaL9i/UeHy+OxHHwvGcwNOjeQ7t?=
 =?iso-8859-1?Q?ToTXcV8ODE9ehlMK1x+Du4qagFy56plMHi/SAUsZfZdareI0ikmncimdDY?=
 =?iso-8859-1?Q?ou2dGdEt3RdNaVQALF1dA6S44zTkAfX3YX2GPHKCkYaReiLbjDv94VuYFe?=
 =?iso-8859-1?Q?mEu5So/Mb7QcaJRHk8aa4ksQi/E126Np9qazCdD7ridO9BYZHLJBJVMjUJ?=
 =?iso-8859-1?Q?tQ0fsw4wEb5wjFdPRs1+n8MSxGOYnnJ3/oOnEf2Xnz2fcoYcp/IopOe5G/?=
 =?iso-8859-1?Q?ARbxdK/kg1k0yfxpzrCfUdfb4HWh/2AvuFg4Ih12YrBKHr2tu6mI8K7+Yu?=
 =?iso-8859-1?Q?s20k+eRZvcxkQh2kJocZ9F2avnEgvSiDP1VhuoTjSeBgN1oJZTX8fdF85z?=
 =?iso-8859-1?Q?eo+1rlq6vbjJs7npryLr9W9epcI0QyVSpAjdxjdt6uEiFW3lsRR0MkmX/G?=
 =?iso-8859-1?Q?Y4dRJgXWE2yrIsMMNfvv29+fubqC+maKOmNByPtgS0BzRnAdouSyhdF6mn?=
 =?iso-8859-1?Q?3NHHyeuXNmt1GPVx6uwOfrkiOmzcx7Bau8pipZ8ubNB2ZcSRPn9t7YWEqA?=
 =?iso-8859-1?Q?5SIvqxJwv1x3uG5Mk2Md/n6AGizWpQ6jwEf4ENxTB2/hfA3UgK0MF/oiId?=
 =?iso-8859-1?Q?mAqjQEqAp8L66uTwL0vShfvrbufC0gjXlZkZg6C+KF87TyJ012Ru82biSP?=
 =?iso-8859-1?Q?X9e3VKJ1F3GJf5bE/oKRg79yaOW8ZLxdAa4pMfoz69u4FPzLCdeknRxo/L?=
 =?iso-8859-1?Q?7rTw1Esas2+hWqnzi7urKCt9o1nncdddOwsrpi/V1etxKMCMlGc7lUyDcz?=
 =?iso-8859-1?Q?jDW0Mfgt5bEsHfdGpQH6EiDR152WML3hhhdZV3RY/2K5vS+hiOXl2iLEzW?=
 =?iso-8859-1?Q?O6t+C518O3wUk2vGBd9ZDeBv+H0dksr+OS4CeVRlFXgiYgKCzdzD4nFU+b?=
 =?iso-8859-1?Q?zya1lrX5cZEMe/pj4I10OdCLFfpTcqIrdzyXoNCys1F+xT6JPdUlPWVlaJ?=
 =?iso-8859-1?Q?0GUrF6Mp68VKFCCk/Iju5ln6Ovjh9Yz/9/gXiehNnpRhAI2A8oewrL18BG?=
 =?iso-8859-1?Q?NzVwEes3dPLUQJe+YDY65ucUQan4O7tXBt0TskrQWfUrvTpW+Ujs7l4EnA?=
 =?iso-8859-1?Q?rvxEEci4ZKv+D0zmcCgOWCgjCzFCq/3ODXlv0v92r5CwMOoEaU91Jwb72E?=
 =?iso-8859-1?Q?0aHQjE8BWtj3Jfgj5hCJ60B93CvR3iJJ+VzZDJJzWwvcrWu1cS14DfVEdW?=
 =?iso-8859-1?Q?qg2546rvbzTez5k3gR9NaPiUeuCXsOME0/3YMChOwY8ws3d/MDNTqVlGa8?=
 =?iso-8859-1?Q?+St0USqLkvplTiAJ/6Poc0hXDH2GFueHuOcGXdwnD+WSA+LQ1AXPsvBZX0?=
 =?iso-8859-1?Q?V+6yhRzW4fA01nL3RmMD7ehzlj1el8GG56tzvEBEfAKWkHdNl9hL0yoFkY?=
 =?iso-8859-1?Q?+6QUAAUeKBm0u+y1l/ry3Zr0ftSwZNSG5X+hGsz/2ZmCVB5RU4Fd3DQ4yH?=
 =?iso-8859-1?Q?pKHNFN2meMh4fA/HDR7XDs68bu5BDrnIFj?=
X-MS-Exchange-CrossTenant-Network-Message-Id: b14a14dc-f85b-42ff-e35d-08dbf2c10573
X-MS-Exchange-CrossTenant-AuthSource: SJ1PR11MB6201.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 01 Dec 2023 22:58:23.8337
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: 9e8VN3e6KkImpcnjHr+TCGzfpBaGsBHzwWuM11da3IPxOq4CgDixVTIarwPuO2H5MxF2aI8r4lnnOmzld85n8w==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY8PR11MB7337
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-0.9 required=5.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED,
	DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,
	MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE
	autolearn=unavailable autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on pete.vger.email
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (pete.vger.email [0.0.0.0]); Fri, 01 Dec 2023 14:58:37 -0800 (PST)

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?