Received-SPF: pass (google.com: domain of linux-kernel+bounces-59536-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Message-ID: <ef4b6a21-a477-42f0-950c-246b5cc48f47@amd.com>
Date: Fri, 9 Feb 2024 09:28:52 -0600
User-Agent: Mozilla Thunderbird
From: "Moger, Babu" <babu.moger@amd.com>
Subject: Re: [PATCH v15-RFC 4/8] x86/resctrl: Add helper function to look up
 domain_id from scope
Reply-To: babu.moger@amd.com
To: Tony Luck <tony.luck@intel.com>, Fenghua Yu <fenghua.yu@intel.com>,
 Reinette Chatre <reinette.chatre@intel.com>,
 Peter Newman <peternewman@google.com>, Jonathan Corbet <corbet@lwn.net>,
 Shuah Khan <skhan@linuxfoundation.org>, x86@kernel.org
Cc: Shaopeng Tan <tan.shaopeng@fujitsu.com>, James Morse
 <james.morse@arm.com>, Jamie Iles <quic_jiles@quicinc.com>,
 Randy Dunlap <rdunlap@infradead.org>, Drew Fustini <dfustini@baylibre.com>,
 linux-kernel@vger.kernel.org, linux-doc@vger.kernel.org,
 patches@lists.linux.dev
References: <20240126223837.21835-1-tony.luck@intel.com>
 <20240130222034.37181-1-tony.luck@intel.com>
 <20240130222034.37181-5-tony.luck@intel.com>
Content-Language: en-US
In-Reply-To: <20240130222034.37181-5-tony.luck@intel.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0:
	=?utf-8?B?eThId3Fzc0x3ek5Od09uK1lLTXRIUXVlQ1pJZjI0TDBLQjlzSjlPWkJjL1FJ?=
 =?utf-8?B?cTl1MnJpNnZtMmwrZy82aVhvWVRTdHJTa3VGNC8xSGtVK0VFMms0bCtkVUN5?=
 =?utf-8?B?cWhFTXpkZHY1eVlxVWNBa2h3OVY0QVdDcVE0ZTFFYWJMWE1uQ3RLSE81dnk5?=
 =?utf-8?B?K0lOVlBGbkcxZUJoa1ltUFl5Yk9zZW14RFFWTVRqc3FXVmtWRW5wWkE2MC9u?=
 =?utf-8?B?Y0NaU0g0VUdLekdOalFXOVhoSUpQdWFqTU5FSkxTRUp4QXNiNUFFMXJhNWVL?=
 =?utf-8?B?cmFFc0pwVm1RYngrSXVxVXNhSW9oWUpaeXh0VUhrWDN4Yzd4RHJ6eERINXIx?=
 =?utf-8?B?b3plc3UrZEFYNllsQUp0RHlBQ0JKNnNWL1ExSU1sallTanRKUG91Q1Mzbm9y?=
 =?utf-8?B?UURDWDFDc0pISVJjdW44L0Q3bE83aXM1UXZKU3I1eGlNRXRsV2k0MnN5VlR6?=
 =?utf-8?B?TjBDc1FiN3FEaWJ0bHFmejlGNVV5MDNmWXRCSVpJWERxNGpwdUpaQ1VZUjFJ?=
 =?utf-8?B?bUFkS1VnOEF4cmpJS0tOZ2tJaC9zUDFDdWRUUnc3NnIvVG1ESGo2TGNuTDdh?=
 =?utf-8?B?c0oyaG9zaDhhWHlmZjhLVVdRQUM4M0pjV0QzQ0NJbmlIMkJCRU5xRGdNdEhH?=
 =?utf-8?B?RHREenMxYVJkN1F0MWc1d2NYTjNnRHBOdU5YcUdMZFQ5NHpCNzBMd1FIMzVW?=
 =?utf-8?B?OWVESFVtSkZpTHpLeWQ0SHVZY0FITzFUQ0JwSGZ3b3cySGNWZFhmVjdUM2xE?=
 =?utf-8?B?YWpuUEVEeUJIRmNNM2JOK2NITHF3ZmdtcWtJa3lNOGU1aEZXbkppeHVxaXZH?=
 =?utf-8?B?ODNGU0dnTzdJVzJJSXNnNlN5ZlVUdHo0UHdxZFF4blJwYjExRFJYaGh5ZTJB?=
 =?utf-8?B?ZG54U1RpeHRwVUUrUDdUWmcvVnJNQnViWStWeUxFRFlpRDRSamtNc1JkbUZp?=
 =?utf-8?B?SkNScUZyaUFwQnU0eUVqZDVqbXU3RHV1NFV3M2ZoOWNtaFM5T3ZUaUNuajUz?=
 =?utf-8?B?cytJalJsVVV4NXB2ZllDbGpJeit6UkF0UzhENGhKYnpXdGlBMWY1V3dlZmtH?=
 =?utf-8?B?ZEZpMlNQR1VHcXdndEQwUkh4NEgvd01vU2s1OXEwcW1QV0tYWnI5U3oxOVRk?=
 =?utf-8?B?Rm5zZGttaU5WRSs4V1pLUmtLTEhUajltekF4KzNYZzkvZ0crdEw5b2ZFV29W?=
 =?utf-8?B?MGxJb2doSk1kVS92ZXZFYVpOeUI3b1lQMStaWGVZTkk4c0QrODhoejFxSXEx?=
 =?utf-8?B?YW8zQlpUUjNzUm8xNFdCNjRQRmkyeFpRVGx5eFFsZDFHSnVOM0NSb1VFSTQv?=
 =?utf-8?B?ckVrMXkxQ0t2R0dXc25Eb2hlYjQ1bzFnV3ZrbGRwcGZxTDE0czdWTER0aytX?=
 =?utf-8?B?Ty9qeWhhY1J4bEZqNTdYRVZqYTM1Wmw0K2IwM1ZGbWd1N2NMV0FaOWMrN1Bj?=
 =?utf-8?B?RzlWNVF4VGtTMkJpYVJhSm8wV3dYNkEzNlgwSDczNFdlT2JRRzU5bXloNmt5?=
 =?utf-8?B?YUpPWkVBSHdoNU1WdXN4clVDYURvU1hGMWdkWXN4N2k2MGJZV0s5UjF2d1hW?=
 =?utf-8?B?ZjhhVGQ0TVlnK3pILzhvaVRIOWgxalhncmZRS2c2MHdqVmUvaEQ2UmVjRFFp?=
 =?utf-8?B?bTN0N1JmQkVFZEN4QnJ3UGY0WElBaUhyeUJxZkdad256a3lDaUpYcllKdXU4?=
 =?utf-8?B?QUVKdXlOS0U1ZTQydXlMaFVOTnZFWFZkVjhKZnpWa2V2c0xDem5sQWRKQXFN?=
 =?utf-8?B?bFUwZXNxRXZOVnZrSkd2QWcxdWdUVEdPMUplNTk2Q1loMHNkNndwTC9yRUdx?=
 =?utf-8?B?cWtEbDRqNm5XT0l4OFpqS2k1QjhBMHA4RHZTRHh3QmRWcHVmTGl6bEQ4Y3A3?=
 =?utf-8?B?c3RBYzljRWlaSndPZFRubHJpUy9tWDdsSUI1Q2lYTjRPUFQwTXZMS0FTVXRy?=
 =?utf-8?B?dHFGTzJTMTF1UGlBakc0bWlhUmxYZjl4VWxxeFNWQUtCcUZicjhMTVhOUFZs?=
 =?utf-8?B?dW03Tkg1VlF0MnVTVnNzckE5cytjWEtQR1FNU1lMQUMvbk5veUpPa1AxN2w5?=
 =?utf-8?B?Wko4WThZa3FqeUlpL24zM0NEV0tVcWxJZmNSWVNja014TWlZR3BFMVY1ZkpU?=
 =?utf-8?Q?G5E8=3D?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 98edcffe-db00-43e6-55da-08dc2983d2dd
X-MS-Exchange-CrossTenant-AuthSource: MW3PR12MB4553.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 09 Feb 2024 15:28:53.6002
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: 5Gw29coTdT7Tx+z7QRybZwJ0GilwGBDYp03VuXZRzYP36GNJ5+4cCDEHbfk3e54K
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM4PR12MB7503

Hi Tony,

On 1/30/24 16:20, Tony Luck wrote:
> Prepare for more options for scope of resources. Add some diagnostic
> messages if lookup fails.
> 
> Signed-off-by: Tony Luck <tony.luck@intel.com>
> ---
>  arch/x86/kernel/cpu/resctrl/core.c | 29 +++++++++++++++++++++++++++--
>  1 file changed, 27 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c
> index d89dce63397b..59e6aa7abef5 100644
> --- a/arch/x86/kernel/cpu/resctrl/core.c
> +++ b/arch/x86/kernel/cpu/resctrl/core.c
> @@ -499,6 +499,19 @@ static int arch_domain_mbm_alloc(u32 num_rmid, struct rdt_hw_domain *hw_dom)
>  	return 0;
>  }
>  
> +static int get_domain_id_from_scope(int cpu, enum resctrl_scope scope)
> +{
> +	switch (scope) {
> +	case RESCTRL_L2_CACHE:
> +	case RESCTRL_L3_CACHE:
> +		return get_cpu_cacheinfo_id(cpu, scope);
> +	default:
> +		break;
> +	}
> +
> +	return -EINVAL;
> +}
> +
>  /*
>   * domain_add_cpu - Add a cpu to a resource's domain list.
>   *
> @@ -514,12 +527,18 @@ static int arch_domain_mbm_alloc(u32 num_rmid, struct rdt_hw_domain *hw_dom)
>   */
>  static void domain_add_cpu(int cpu, struct rdt_resource *r)
>  {
> -	int id = get_cpu_cacheinfo_id(cpu, r->scope);
> +	int id = get_domain_id_from_scope(cpu, r->scope);
>  	struct list_head *add_pos = NULL;
>  	struct rdt_hw_domain *hw_dom;
>  	struct rdt_domain *d;
>  	int err;
>  
> +	if (id < 0) {
> +		pr_warn_once("Can't find domain id for CPU:%d scope:%d for resource %s\n",
> +			     cpu, r->scope, r->name);

Will it be good to move pr_warn_once inside get_domain_id_from_scope
instead of repeating during every call?

> +		return;
> +	}
> +
>  	d = rdt_find_domain(r, id, &add_pos);
>  	if (IS_ERR(d)) {
>  		pr_warn("Couldn't find cache id for CPU %d\n", cpu);
> @@ -564,10 +583,16 @@ static void domain_add_cpu(int cpu, struct rdt_resource *r)
>  
>  static void domain_remove_cpu(int cpu, struct rdt_resource *r)
>  {
> -	int id = get_cpu_cacheinfo_id(cpu, r->scope);
> +	int id = get_domain_id_from_scope(cpu, r->scope);
>  	struct rdt_hw_domain *hw_dom;
>  	struct rdt_domain *d;
>  
> +	if (id < 0) {
> +		pr_warn_once("Can't find domain id for CPU:%d scope:%d for resource %s\n",
> +			     cpu, r->scope, r->name);

Same comment as above. Will it be good to move pr_warn_once inside
get_domain_id_from_scope ?

> +		return;
> +	}
> +
>  	d = rdt_find_domain(r, id, NULL);
>  	if (IS_ERR_OR_NULL(d)) {
>  		pr_warn("Couldn't find cache id for CPU %d\n", cpu);

-- 
Thanks
Babu Moger