Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) client-ip=2620:137:e000::3:7;
Message-ID: <f8e46bd8-fc27-4bde-bdbe-daac8ed7f27a@intel.com>
Date:   Mon, 20 Nov 2023 14:16:56 -0800
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v11 1/8] x86/resctrl: Prepare for new domain scope
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-2-tony.luck@intel.com>
Content-Language: en-US
From:   Reinette Chatre <reinette.chatre@intel.com>
In-Reply-To: <20231109230915.73600-2-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?NlF0cDlnNzBuQ0RaR1gyU2hhaWlPeENFQ2tUY1lycWpkaXZaaHlkK1NWWGZl?=
 =?utf-8?B?K0ZsNkEwRE5wVkt3UDZqbnhvejFvYU44Y0FEN1dqeHlEVGt5ZG9IUEVUcXNV?=
 =?utf-8?B?OS9UVXl4Q0dGY2FmRGJUeExyQ09oSVFRMlpFVlorN0pmMTBuQkZhYUJ6cmhs?=
 =?utf-8?B?MnM2TG5hQmFBam03bGlSejdWWGs5dk9lWU8zcFkxd3lVZU5ncXVPR1dFNWpO?=
 =?utf-8?B?YnpJZ2dWYTJoT2dvTjZjcVBDVk0xU2JYZDZXcGtPdGc4QllSY04zRVJTQmZV?=
 =?utf-8?B?aGNjaHYyZU5PWEFwblRmQmdCMWFYTDFQTG03UktXK3RKM3lHOUxDVzZkTzBZ?=
 =?utf-8?B?VGRhVUlDRzdqVmU5UTAvMjgyVGsvTDI3LytlZmZERW5Cb2tuUFJpUE1sK0Vs?=
 =?utf-8?B?K241ZXFETnRjelNCZGxOZkZWZzBSK3pNTlovRk1QcFpnMXlqQ3Z4dlZjakVO?=
 =?utf-8?B?anRja1JNTmloUjI4NU5YUjExWGx5T2xEdXlobmJnWTNGUEZuWkxIZGpQZ0Q4?=
 =?utf-8?B?OUFIYURyeGUvSk1LdGlVSVd3LzJwLytWY2hsdS9kcnRIOUJLbjBBUnFUVUhF?=
 =?utf-8?B?U2dPendIRW9ZTUdLNzVHWGxuN2d4eng3WkphekRlK1dZOU9tVlMxb1hjeUt0?=
 =?utf-8?B?QmM1ZWd5MzF6NnNpd21KdktyWVFON3JrTzZCM3JvR0wvVzFlOW1LVUFlVmZO?=
 =?utf-8?B?UzNSOEtidDJ4NSt2RXJVMmltN2lhUTZWbzRDRnVtN0VOTGdjT0hjalRUY3FE?=
 =?utf-8?B?VHhYcTlzb1E1cUNOLzc3YmV4OC9kU2N2L3VXai9HL0MxYjVuMW1qNE9LRzdt?=
 =?utf-8?B?bHVMMFFpZmRyU2pwV2F2ZHY1OXVnWkJjZnA3c051cUI5UWxUMkx0YXB6cUVt?=
 =?utf-8?B?WGVlK3dnbzNQbytHNFd4NXE5eDZXbGs2UW9RU3lNbG8vZjlBL0tmT29tUXNP?=
 =?utf-8?B?S29SakNocC9SaThSTWx0Ykg1Z2dCaVlPVDNneXd5QVZoTTJyRUwwRGZHZmFR?=
 =?utf-8?B?RmpIZ1F4Tjg0N0xjMXQ4cEpxaklkRWl1TDRxdmdmZTFjeHNJeWplNG93akNS?=
 =?utf-8?B?czVkbXJ0UlpkYSt2K3JEWGY5VGZZS084T1pmTzU1b3VNeWp5RnN4NWJ0WUxr?=
 =?utf-8?B?YUE0N251N3BjaFpEelVQUGZ1KytFbmR3Y29FOVZYMlJQdnZOcUxuYU56NUc2?=
 =?utf-8?B?enIyVVBINkdYV054ZGhwM1dqUkpmN25GY0JXZmIyaUlpWUFUWDlUaGcwdWJU?=
 =?utf-8?B?NW93dE9PdUtwWUxwd1JYeFNBeDJGRXVkbjhHRy9RejV3MkFaOUpQMVpLcGVx?=
 =?utf-8?B?SWR0K0RuUHlpTWtRcm9oNEp4R1hXMXlmdUN5NWFKTCtxQ3hUQ0FSeU5WdnQ0?=
 =?utf-8?B?bE10MUlCM2JaM3did3BQUy9obTloUUMzOCtycGxnR3gyMXhJU0ViK1g4R1F2?=
 =?utf-8?B?YXhaaGJRZVYvVCtFTE5lWG41WHhIS3FLSXZuN2w3eWt3QldFL2xKczhVVmcz?=
 =?utf-8?B?eUxUeXNBdWtjWTk0Y1V4bXo0d0pCUXBBZ2kzT1k4LzBMZzlZaFFTVkJrK29n?=
 =?utf-8?B?RGJlOWJ3TTZ3VnQvcFFnWGY0RE9PT2I5SDcxMDZ4RFRCdE9oSkpnVHBEdjdp?=
 =?utf-8?B?NzBYTHNFT3RRZkVmOTgwR1JTSU1mbEM1Mk13Y21panVBZlpSQmdUVFQybDBR?=
 =?utf-8?B?Lys5RTRGUkx5c3dSRUVEWU5tYU52WlFKWDd1MkVTYjBNaEozSXNRc3NhWkFu?=
 =?utf-8?B?bGNBWDNucHhQdTZjcWFjUW5tdGxCZy9OeDdWNFJrQ0xWUlJIZWpjV1NGbzZC?=
 =?utf-8?B?bEI2N00yTWZ4RGNackI4OEZtOTc2bWVwcjNPaWNxZnBsc1p2cDVzblFnUDZj?=
 =?utf-8?B?R3VrTFRPUEtzUlpTQWNWajhIU2xpYXlJNEo1Z3VSQ0dXU005SFNoZDhBRTJG?=
 =?utf-8?B?NGZJT01BNTJobjJSNzdoRjFWeW5ZY1d1T1o5VHFBK2REVHl5MUNxOUlyMTBC?=
 =?utf-8?B?TGxRaEtrWnlrSnJxYWtuMGlqVG4zWnMzZEN4MkRXWnBQV0pkZW1Ddk51dytv?=
 =?utf-8?B?NFFxYXdiUXJhUityQ0lYZFlOeG9jclJ2Vzg3LytKS3BEbHF2aFVGOHBFSG5v?=
 =?utf-8?B?bkF0UHVNWTFmaXI2ZStaTUdaTnFmd1JpWk02dUQ1S00xcWV0eXBVTGorejMw?=
 =?utf-8?B?TWc9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 5a0cb683-ec1f-4172-f04d-08dbea1669d5
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:16:59.0388
 (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: 3we6aeDaHlZvi85L/dlmJLTdeMqvuX+bG+VZM5sc075DNpaXXUez6DzPqwdQxnZD+xi0kltccz14dfGTFj50YVgeIzmu2aVGVwUX/DSGR3M=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: IA1PR11MB8222
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,
        RCVD_IN_DNSWL_BLOCKED,SPF_HELO_NONE,SPF_NONE,T_SCC_BODY_TEXT_LINE
        autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
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 (snail.vger.email [0.0.0.0]); Mon, 20 Nov 2023 14:17:09 -0800 (PST)

Hi Tony,

On 11/9/2023 3:09 PM, Tony Luck wrote:
> @@ -556,13 +567,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->cache_level);
> +	int id = get_domain_id_from_scope(cpu, r->scope);
>  	struct rdt_hw_domain *hw_dom;
>  	struct rdt_domain *d;
>  
> +	if (id < 0)
> +		return;
> +

Apologies for this late comment to this code that has been like this for
a few versions. Something must have gone really wrong if id < 0 so I do
not think a silent failure is ideal. Could the same warning as above be
included here? That is:

	pr_warn_once("Can't find domain id for CPU:%d scope:%d for resource %s\n",
		     cpu, r->scope, r->name);


If you agree, please do ensure that this change propagates to
domain_remove_cpu_ctrl() and domain_remove_cpu_mon() in patches that
follow, in which case the text is expected to adjust to "Can't find control
domain id" and "Can't find monitor domain id" respectively. (btw ... is
the switching between "Can't" and "Couldn't" intentional?)

With this change please feel free to include:
Reviewed-by: Reinette Chatre <reinette.chatre@intel.com>

Reinette