Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <9e340c96-507c-d6b9-b4b6-0c1d9aadf6f8@intel.com>
Date:   Thu, 15 Jun 2023 15:23:31 -0700
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Firefox/102.0 Thunderbird/102.11.2
Subject: Re: [PATCH v4 20/24] x86/resctrl: Add cpu online callback for resctrl
 work
Content-Language: en-US
To:     James Morse <james.morse@arm.com>, <x86@kernel.org>,
        <linux-kernel@vger.kernel.org>
CC:     Fenghua Yu <fenghua.yu@intel.com>,
        Thomas Gleixner <tglx@linutronix.de>,
        Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
        H Peter Anvin <hpa@zytor.com>,
        Babu Moger <Babu.Moger@amd.com>,
        <shameerali.kolothum.thodi@huawei.com>,
        D Scott Phillips OS <scott@os.amperecomputing.com>,
        <carl@os.amperecomputing.com>, <lcherian@marvell.com>,
        <bobo.shaobowang@huawei.com>, <tan.shaopeng@fujitsu.com>,
        <xingxin.hx@openanolis.org>, <baolin.wang@linux.alibaba.com>,
        Jamie Iles <quic_jiles@quicinc.com>,
        Xin Hao <xhao@linux.alibaba.com>, <peternewman@google.com>,
        <dfustini@baylibre.com>
References: <20230525180209.19497-1-james.morse@arm.com>
 <20230525180209.19497-21-james.morse@arm.com>
From:   Reinette Chatre <reinette.chatre@intel.com>
In-Reply-To: <20230525180209.19497-21-james.morse@arm.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?R1RjM1EvSHVwak5aZnI5ZjRlMXRCcklNeUNqNGl2WURkSWlwdGdJMDJUZmJx?=
 =?utf-8?B?OUhrYzIzb0dBRG9ReXdwbHZZR1UwMmZkc2hjT0QybTJxcE9Jd2huMzlvbXlh?=
 =?utf-8?B?K2ZYV0luUnZxOE80V3p6YnZzaFZNUU1zcVphb3Q5VG1rVlhkTDVMVktmZnFh?=
 =?utf-8?B?Y1UzWGI1K0FmSlVsYnZaaXhpS1RMaTl0aWM3dlpiQ0JieE5GRkJybVBNUS83?=
 =?utf-8?B?d0xjd3p3NmZRbVhqaTNteko4UUZHTTlzVDFIKytBY1dwS1JucDY5Zkp2ZTdN?=
 =?utf-8?B?Mk1naldKQ25OVWZhaVUyZUMxb1ZIRzY3R1VwVUNpV3B4aFlXbUtxRXUvZGFM?=
 =?utf-8?B?NXRsazZlVDlnQnA2Rm5tNysrdVU0WnpzZTUzUWR2dzdMMUNKbk1HOVBBMFp1?=
 =?utf-8?B?bElEbW9pK0dKNmJRQ0dWa05PQ3QxU1NjSmxQZEsvcXN3cVNuT3V4MWN1d1Vm?=
 =?utf-8?B?Y01uWEdvSkNSRDVUUXhqSW5LMy8rTFVJZVQ4b2EwK3AwaUp3K0laVFovcU1F?=
 =?utf-8?B?OWNuSU5pU1NVZ2ZZZ3lkVEo2TmZEUnRyeksrWDB0elZGMlh1Tng2bCtQTGNo?=
 =?utf-8?B?MDlTeEp4SjM3R01pWG01OXlVN2QwdlRyMTFid1pmUmhEWGxoMWgwRzRQZUhp?=
 =?utf-8?B?VUpZalVzTkdCOVJCdi9tQnordGxnb3kzMGZaUnlBWGlVNnphbjVFb2Z6bDU5?=
 =?utf-8?B?QWg2NktSWXk4SmlkdXdMK3lUYktDUXplN0Q3aTBCSWdGVTA0LzV1S211d1lX?=
 =?utf-8?B?NVhySldXdGY5VlZIZ3Q2Smd0L0JBVHhWcVhsaHJmQURJNkE3a3JFL29XY1Za?=
 =?utf-8?B?bnQyY3JEaUZJMEZQcGRrZjdtSm9mZjlCTVhrVjlnWlllTDNYclhTUDdlMW4v?=
 =?utf-8?B?NG5Xeit5VWFqWFJnNzErUk9kT2RIV3h5RHorVEdSNkpVOVhZY0R1WldlM1hR?=
 =?utf-8?B?UnpEUDFzaDdtYnNBRjhYdmFqUlVlYTR6THdwb3Bxai9ESHU0ZG9aSVVNbzRq?=
 =?utf-8?B?WmxLdTEzQWZLWFB5bEFnZXo2RHA3cWhPYkxGWTZReHhOOWRIZDdKblFUTVVF?=
 =?utf-8?B?THZCSUdvZDA1UHNaSFE2TWkrc3BnU2ZaMFdrbkhwNFdla0NXUmNCbUVxcnhS?=
 =?utf-8?B?djZnT2lLNU5qVThGN2xGRmR5VWdsTWRlL2FMRG1xczVMWmU1aGNDeFJxTGI0?=
 =?utf-8?B?OFBFdHlZVi9SSzcyWlVuOWNIb0xHNm5RSzU5eEVPdTJaZCswTmpLaWN1ZCtt?=
 =?utf-8?B?aklpV0V5RnJYUEpjQjYzenZjOUlwaFp2MXBZbElicU9rZkhvWTEyUkI2dzFG?=
 =?utf-8?B?YnpncjBuSExxOXBEblhVc0o0QTI3V0oybWlwWE1oM2IxcXR2eFA2a3RxUHUr?=
 =?utf-8?B?aEpJQ1ovSmlzSVBpWCtBQ08wTURrSVo0WjluNHJOdE14QmMwRWUxc0hENVFh?=
 =?utf-8?B?NzJUcXRlVEVjZndWNjE0c0lCR1ZWZ1hhMVpWVHRyVVRYeVdWdmlqWmlKOUV4?=
 =?utf-8?B?SEVLMXRQMnJsZjdJNnh1ZlZUTHdNMDd3QUc3Y3hDby9ES3UzWFlmOGtJRVM0?=
 =?utf-8?B?cG9aSFZidjNnZi9rZjZIRVRwbVFjTVpybHRzVE8rdHhzMnYxejRMTkRkU3dp?=
 =?utf-8?B?SEgrM1hTK21sZTViamRUSDZYNXNZZzErY1pPdVVhYTNFejRjVzhpNm0vWlhj?=
 =?utf-8?B?bjB5M2t2VnNoMFpBMG9SSEg2U2RHVVY2WTdSVko3RThUeU8vOHJwZ20xSThl?=
 =?utf-8?B?VWZZYTkrdjh2MkdKbm9qT2dlY2tCY281RnVjVVp0aVNJK3BrTGRKVW51Uzd3?=
 =?utf-8?B?SzdNLzFIUkhzYmovQzlqcHFud1RqZ3BVRTRKeHpVVDZZVkt0YmVoSHpWUklS?=
 =?utf-8?B?eEp6Y0ZxYWRGVWRMUW5MWW5TU2xjRE8za3Y2WExiNGF3UUZ5OHIzV1RCRG1n?=
 =?utf-8?B?M2Jla2FNOHBJa3JuTGIreW9qU2lpZXFPeW1SYnp4R2RidDJzeDBmbFM0QTNM?=
 =?utf-8?B?aWdTOC9TYzFkWitDR3FJYzdYUmdYUDl0SjNDZlY4ZUZOZElBSXh5T1FjbTFJ?=
 =?utf-8?B?b1pnTUJ2ekE2US80cmlxMEM1RWdwMFhRcy8zaDdJZUVmQkNhUTQ2S3NWRmwr?=
 =?utf-8?B?S1hJSlY3SXRMY2hIdzRjRXlDOCtVZlY3L2g2ZGpaZEdQQUZwRE0rWnRLTnJ0?=
 =?utf-8?B?Rnc9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 4479810f-669a-43d3-568c-08db6def27fb
X-MS-Exchange-CrossTenant-AuthSource: SJ2PR11MB7573.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 15 Jun 2023 22:23:34.0559
 (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: ILycAQ/gzLp/JFEJ1U3VYXiYe6avMZWHGUSSFHcEiAOtInIuxTJjUEj0c4ck6YbmIDA5Q/4smZPtfEp6yEBDJHy6b2Y0QAl3baqgHApZRoA=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MW5PR11MB5932
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-4.5 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,
        RCVD_IN_DNSWL_MED,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

Hi James,

On 5/25/2023 11:02 AM, James Morse wrote:
> The resctrl architecture specific code may need to create a domain when
> a CPU comes online, it also needs to reset the CPUs PQR_ASSOC register.
> The resctrl filesystem code needs to update the rdtgroup_default cpu
> mask when cpus are brought online.

CPU mask when CPUs

> 
> Currently this is all done in one function, resctrl_online_cpu().
> This will need to be split into architecture and filesystem parts
> before resctrl can be moved to /fs/.
> 
> Pull the rdtgroup_default update work out as a filesystem specific
> cpu_online helper. resctrl_online_cpu() is the obvious name for this,
> which means the version in core.c needs renaming.
> 
> resctrl_online_cpu() is called by the arch code once it has done the
> work to add the new cpu to any domains.

CPU

> 
> In future patches, resctrl_online_cpu() will take the rdtgroup_mutex
> itself.
> 
> Tested-by: Shaopeng Tan <tan.shaopeng@fujitsu.com>
> Signed-off-by: James Morse <james.morse@arm.com>
> ---
> Changes since v3:
>  * Renamed err to ret
> ---
>  arch/x86/kernel/cpu/resctrl/core.c     | 11 ++++++-----
>  arch/x86/kernel/cpu/resctrl/rdtgroup.c | 10 ++++++++++
>  include/linux/resctrl.h                |  1 +
>  3 files changed, 17 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c
> index 4bea032d072e..e00f3542e60e 100644
> --- a/arch/x86/kernel/cpu/resctrl/core.c
> +++ b/arch/x86/kernel/cpu/resctrl/core.c
> @@ -603,19 +603,20 @@ static void clear_closid_rmid(int cpu)
>  	wrmsr(MSR_IA32_PQR_ASSOC, 0, RESCTRL_RESERVED_CLOSID);
>  }
>  
> -static int resctrl_online_cpu(unsigned int cpu)
> +static int resctrl_arch_online_cpu(unsigned int cpu)
>  {
>  	struct rdt_resource *r;
> +	int ret;
>  
>  	mutex_lock(&rdtgroup_mutex);
>  	for_each_capable_rdt_resource(r)
>  		domain_add_cpu(cpu, r);
> -	/* The cpu is set in default rdtgroup after online. */
> -	cpumask_set_cpu(cpu, &rdtgroup_default.cpu_mask);
>  	clear_closid_rmid(cpu);
> +
> +	ret = resctrl_online_cpu(cpu);
>  	mutex_unlock(&rdtgroup_mutex);
>  
> -	return 0;
> +	return ret;
>  }
>  
>  static void clear_childcpus(struct rdtgroup *r, unsigned int cpu)
> @@ -965,7 +966,7 @@ static int __init resctrl_late_init(void)
>  
>  	state = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN,
>  				  "x86/resctrl/cat:online:",
> -				  resctrl_online_cpu, resctrl_offline_cpu);
> +				  resctrl_arch_online_cpu, resctrl_offline_cpu);
>  	if (state < 0)
>  		return state;
>  
> diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> index 5330c0bdeffc..7c3de5ea0482 100644
> --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> @@ -3725,6 +3725,16 @@ int resctrl_online_domain(struct rdt_resource *r, struct rdt_domain *d)
>  	return 0;
>  }
>  
> +int resctrl_online_cpu(unsigned int cpu)
> +{
> +	lockdep_assert_held(&rdtgroup_mutex);
> +
> +	/* The cpu is set in default rdtgroup after online. */

I know that you just copied this but please use CPU instead of cpu.

Reinette