Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.33 as permitted sender) client-ip=23.128.96.33;
Message-ID: <e2dc025e-4776-4876-961b-470a6273b8e8@intel.com>
Date:   Mon, 20 Nov 2023 14:19:04 -0800
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v11 3/8] x86/resctrl: Prepare for different scope for
 control/monitor operations
Content-Language: en-US
To:     Tony Luck <tony.luck@intel.com>, Fenghua Yu <fenghua.yu@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>,
        Babu Moger <babu.moger@amd.com>,
        Randy Dunlap <rdunlap@infradead.org>,
        <linux-kernel@vger.kernel.org>, <linux-doc@vger.kernel.org>,
        <patches@lists.linux.dev>
References: <20231031211708.37390-1-tony.luck@intel.com>
 <20231109230915.73600-1-tony.luck@intel.com>
 <20231109230915.73600-4-tony.luck@intel.com>
From:   Reinette Chatre <reinette.chatre@intel.com>
In-Reply-To: <20231109230915.73600-4-tony.luck@intel.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?WGVHazRwRGlxN2dJWlNTS2ppcTNSVUF5aDJHUFNOc2t5WHVTdDcyMmxwaXh1?=
 =?utf-8?B?bUlHNldvUjNhQ2VadWJINHR5ejYyY0M4OEN6YlpzL1RUODhkckxDZTkxSUw4?=
 =?utf-8?B?Y1lzSDgzZlNCYWEyNGhUMm54eDNUazFac2F5N0RZd2pxS0tDdVpSdEdUb0RO?=
 =?utf-8?B?dUJnM3FVZTd2cThtVGNyUkp1NG0wT3FxeHVkeHhoN05nbEprNXB6TUVqaWVm?=
 =?utf-8?B?Sm1hQldxcW5Vc25CbkpqUnNKblB1QmpDdlZ1Skt6cGhldVNKdk9TNGU0czJR?=
 =?utf-8?B?V2VoQnVzYXpJL0QrT2hjMHBzUllJcmx0ajF4eHkyMTZXUUdDWVV2dCt3L1dJ?=
 =?utf-8?B?UnZNT2thc1MyL2lLZlFPNEhFa2ZjaUNHM3ptY2c1TGtibS9mV3R3VmRoeHU0?=
 =?utf-8?B?OVVQUFl6ODhJb3pTYmlNaHRGeFVqN2lCKy9FaXVJMmMwejNwMzd1QVZjNzBD?=
 =?utf-8?B?RmNsSy9kT3I2U1V5UHVTdkNIOG1VOXJ4RVdQWWF5d1BFSEJySmZsU2hFZDBz?=
 =?utf-8?B?dVlUZkNaMG8wNEhSdjZzYUpDQlp3U2ROeDRBYmVRUlBJZEk3eEJSendoYjlW?=
 =?utf-8?B?eXpnekFlSno1UFpzRmpvWTEvTWJmbS96c2dRZzRqSjFORXYxQ0d2V0w5U0Jp?=
 =?utf-8?B?dlJWVEZFUmVHWEpFWkdTWFB0c3F5N293QWh2MHRaY3lURkExcEJsc3Z2WFc1?=
 =?utf-8?B?VWVNMDZoTVVxSjdwRG1GMjBjMXczaDRwUzljcW1Dbmd1TUdBYXhXNTN2TGM3?=
 =?utf-8?B?NkwxTXFMQzUvYjBRMTExYXoxRThsWS9ucnNTNWR5NDFiK1BWaWRKTzRrWkdE?=
 =?utf-8?B?Mk9OWWFyMlFaRndaNkdaNkluRWhUajJEbmZSaGJUZTBDWXdBcjVQQ2x1Y3Rv?=
 =?utf-8?B?aHpLdnQvL1Roeml0cjM3ak5uWDZ0dW1wRWpKWHlyQ0VyQ2xxOW1lOTl4bzdE?=
 =?utf-8?B?RW9sUktzYlJEOTBTVWdwQVNGUTd6L0NRS0VONUxFUEVKcnNqc0Z6Vk1vS0JM?=
 =?utf-8?B?TXV1Vy9FSWp1bURGUjVYTUVCcGFBVFYzQlhlcjJpdGw3eWtDL3M3T3JNUmla?=
 =?utf-8?B?cVF4SzJhblJqRkpWdnhrcGMxT3lwUEI3aEJwT3VLT0xTTzdhc2N5Zk1hNkpp?=
 =?utf-8?B?Y1FKcXVmS1NZSTgzZjFieGJrMVZqRlpYalBabWZqNUxUTzVycjJOSzJ6b2cz?=
 =?utf-8?B?eng4VFQzNTZ0M1gvNFIrb1Z6V21VWWpIR3hrYTI5TTZrdVJHeHlqa0NYbWJ0?=
 =?utf-8?B?b3Zqd2Yrc2prL1JPNi9tWFg3NTdnQmRSK25NdjRkZ0VCSllpN1lMelhyeUdV?=
 =?utf-8?B?RlFuNXFFWlZGbUE0WmhNQWRhVms0V1dTeUZOYmY3TzVFQnJQamJMd2k3cUZi?=
 =?utf-8?B?VHpSZEpGQUhwWGJRNFRrcnNLaVZEZFBzSmZ1TDN5SjJ3ZERHNTJPM3IyaW1K?=
 =?utf-8?B?cFRheHRPL0grODF0dG9hYmpJdnd0bE5aY3FGMVF4SUp4WG9lWmJyU3Z4QzNZ?=
 =?utf-8?B?d0ExSDI1ZjNCcXlXVlZQQy80VWYxRXdVWlpqT3BpWllERThYUTc2M25qOUZl?=
 =?utf-8?B?bmI3YjZhZnRnSzBoWm0rNlVleGV6RzhKU2hmTG1qUlpCU29vMUJLWFR0OEV2?=
 =?utf-8?B?dXBjbnRaMlNuNFRkY3NGS1VFcExlejRrV0oyMkcrRmJPY0xBbGtsWjNBZHdT?=
 =?utf-8?B?RTlnODZmNzQwMHgyanBadUJUV1FLbmpvNDAxQlUxRTFZR0NsTHUraDRJd2pw?=
 =?utf-8?B?RmlUZHQrN2xlNmtkdTdwRDBDM2lYMnZxSkt1ZityeURaeUZqNUZBVm50SGVj?=
 =?utf-8?B?dytrdzB3TUhhc1pwNmduem14VXlEZlp6OG9qSzdtcVlsbXBhRVk0NjYvNTl4?=
 =?utf-8?B?ekVNclI3bWp6TUJySnpkcWNyMXRlemV6SHZRTTVaUXA3OWI3T1RnZ1RwTms2?=
 =?utf-8?B?UGtha2QzSGYvK01vN2Y0OGRGK3RQcVlNOUpUcDltOWZ5ZzJOUFRST2RVWHZy?=
 =?utf-8?B?VXlESUVnNDBwYmJrOUxwV1hUZmVWczhyQitvdDN6K1J3SjJLMUVRbTU1YW8x?=
 =?utf-8?B?TDIrUldKKy9iVDFTTEJvQUJwTWtlRU1aOWs4S3p0ZUY2Z0Z0bGR0VVMxaE0w?=
 =?utf-8?B?NXJYcEdBR2J1YS9YS0dTNGkyVDV1YWc4eHAwN2hNUHFhSGFWVkpHNzNQZXZ6?=
 =?utf-8?B?ZEE9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 49dce4f6-dd75-42f7-09cb-08dbea16b527
X-MS-Exchange-CrossTenant-AuthSource: SJ2PR11MB7573.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 20 Nov 2023 22:19:05.3833
 (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: dwAlg4Ub1CAjBq6e6vQRp/K+hnjf3d4XzaFnBN8UQCOqAmD5sVK5kEFTDMnJ21932FGos6CSVW9sQTPh2vlR7NBAFYBK9jkwZ8Q3Nh0tzbQ=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MN2PR11MB4678
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 lipwig.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 (lipwig.vger.email [0.0.0.0]); Mon, 20 Nov 2023 14:19:45 -0800 (PST)

Hi Tony,

On 11/9/2023 3:09 PM, Tony Luck wrote:

...

> +static void domain_remove_cpu_ctrl(int cpu, struct rdt_resource *r)
> +{
> +	int id = get_domain_id_from_scope(cpu, r->ctrl_scope);
>  	struct rdt_hw_domain *hw_dom;
> +	struct rdt_domain_hdr *hdr;
>  	struct rdt_domain *d;
>  
>  	if (id < 0)
>  		return;
>  
> -	d = rdt_find_domain(r, id, NULL);
> -	if (!d) {
> -		pr_warn("Couldn't find domain with id=%d for CPU %d\n", id, cpu);
> +	hdr = rdt_find_domain(&r->ctrl_domains, id, NULL);
> +	if (!hdr) {
> +		pr_warn("Couldn't find control scope id=%d for CPU %d\n", id, cpu);

This error message seems strange to me. Shouldn't this just be "Couldn't find
control domain with id=..."? Also, can the resource name be added in error message
to help user dig into cause of error?


>  		return;
>  	}
> +
> +	if (WARN_ON_ONCE(hdr->type != RESCTRL_CTRL_DOMAIN))
> +		return;
> +
> +	d = container_of(hdr, struct rdt_domain, hdr);
>  	hw_dom = resctrl_to_arch_dom(d);
>  
>  	cpumask_clear_cpu(cpu, &d->hdr.cpu_mask);
>  	if (cpumask_empty(&d->hdr.cpu_mask)) {
> -		resctrl_offline_domain(r, d);
> +		resctrl_offline_ctrl_domain(r, d);
>  		list_del(&d->hdr.list);
>  
>  		/*
> @@ -596,6 +654,38 @@ static void domain_remove_cpu(int cpu, struct rdt_resource *r)
>  
>  		return;
>  	}
> +}
> +
> +static void domain_remove_cpu_mon(int cpu, struct rdt_resource *r)
> +{
> +	int id = get_domain_id_from_scope(cpu, r->mon_scope);
> +	struct rdt_hw_domain *hw_dom;
> +	struct rdt_domain_hdr *hdr;
> +	struct rdt_domain *d;
> +
> +	if (id < 0)
> +		return;
> +
> +	hdr = rdt_find_domain(&r->mon_domains, id, NULL);
> +	if (!hdr) {
> +		pr_warn("Couldn't find monitor scope id=%d for CPU %d\n", id, cpu);
> +		return;
> +	}

Similar to above, can this be "Couldn't find monitor domain with id..."?
Can resource name be added here also?

The rest of the patch looks good to me.

Reinette