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: <c3f3ce07-3402-4cd5-fcf8-1d505ee820e7@intel.com>
Date:   Thu, 15 Jun 2023 15:13:35 -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 15/24] x86/resctrl: Allow arch to allocate memory
 needed in resctrl_arch_rmid_read()
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-16-james.morse@arm.com>
From:   Reinette Chatre <reinette.chatre@intel.com>
In-Reply-To: <20230525180209.19497-16-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?REROanluc2cxTVdHbDU3djd0Qi9MWFBhZ0RxazZXcmZNcDVGazRjSUZLMTkx?=
 =?utf-8?B?eFBITmlwVTJpb3pHSnU1c3djWHJ0WDNXZmNETHh5Q29VNDdKMjkxdGJ2T0s0?=
 =?utf-8?B?UVRURFFKYU5CYXhxSThTemxxSG1CTC9tdjQ0aUYyaHZGT2l1R0ZZc0U1Nnpq?=
 =?utf-8?B?dlVLSmZYZTFIUWVvbVE3S2RiSXM2N0ZaNk5GQWxDbFpVOXFuSEowaWR0OEJX?=
 =?utf-8?B?OTNFYXQwU3I4ZzZXVXUrdWV6LzRNc2RCVjN6aFJBNEZLR3dHNnRHOHYwL2Jv?=
 =?utf-8?B?TDMrS3h2azlxYVZVL05jSlRHaWFhaStmYWJDeDQ2dEQzN2o5bHg1c3orWUZt?=
 =?utf-8?B?NzJoMHpUbnlxVTBCV2ExWVBrUVYzUC93TjhhWllzdlVySzZrTGY5aUxzWGRq?=
 =?utf-8?B?ZVpJaEV6WUNBTVF3V1RMa0Q2eXFDRlp6WXFUT1RPZnFnWG5FUG1MV0pwMUF3?=
 =?utf-8?B?aHJiUTZPczdYWkIxUGRHNTNtWENGMXJjVWJ5V2xCTjcwd0FrbEFpZ05UTkV6?=
 =?utf-8?B?b2kvK0FFWmkrUXBqSXY0eGJRa0xYWkU4cC9EZHJtTXBkK2grMWVlYkhlcE92?=
 =?utf-8?B?V202TDB1ZklnWjU4SjZzTFBpRWhFeXRWbUhDZnhkbTFSWFdKU3VacUlyQk5Z?=
 =?utf-8?B?YXZsbGJvWERjMzB1QldwMlJJQ0dXU2MxYlV6UGQ2NEZ4cG1LSzNvTlpzQUlM?=
 =?utf-8?B?c01GbmdZQUZtYjZuOVFrNFBrNW9KYW5vTXIrUG1EeWpoZEExUkdYbndwKzI5?=
 =?utf-8?B?ejZLUEV5dWtWaDJScXhLQm9PZ21JMTVlRjA1dVdwaGl3VmROdW5XaE1oNGtR?=
 =?utf-8?B?YUV6MTF6bHY2UHBCT2hjaEQ3cHJ0L0grekhFYWY2dG5ieTlxNlptaFJOMWxp?=
 =?utf-8?B?RjJqbzE4ZXRObXp5TkI0SEM4SzFtYmJhN0kyU043ZUhQdGNuaUNCU253Rmt5?=
 =?utf-8?B?YXJGcEVDOFo5Z2RKOHd5STVGRDVValZaRTZDbm50SlpreEhscEE1Y2Z4Nzhr?=
 =?utf-8?B?RHJFY1dxcSs2RlBqdUNiYlhpWkRoRnBzbHpsMWUyb25sQmxDQmNvQnA4TzM0?=
 =?utf-8?B?MUx1elFEN2xDSVNRck85dlJQRE9YZW91SjZhdFlpNlpiYVlKdjZsSVJoOE8r?=
 =?utf-8?B?cFJ4bjFpdTdlWTdvNHp1NjducXlFc1JBWGNEK3JpOGdRZHFhTVlYZGw1amVJ?=
 =?utf-8?B?SUIwREpPN3hnTHdWZnIydndibDRSalZVaTlWcGVOdUIyOW9jV0tWUW1GU2ox?=
 =?utf-8?B?ZlRmQUxCNG9ISzhESDZ4Q2daZHpLOWRRWlFNTnZDNVl6b0ViMHZRWFQ2U2dn?=
 =?utf-8?B?VS9OVWpNcnZFSVJDaGEyVWtYZ3dOTVpEOG03NzZzUEt2RGtEMHhvUkdYTGVB?=
 =?utf-8?B?N25VSmNmekc3WGNKVmlqQmJnQUlZdmc4S2k2UWVmdUp5MER2MFdpZHlrUG03?=
 =?utf-8?B?QkVFV0d1T3B3WVZMcndoemJNbndwQi93b3YxWnhON3ZsbHBHQWE0Z2RiNEtO?=
 =?utf-8?B?cElxdGhFVVJab1gzQmQxdVVGelh6Sm5XQWdmRzBmRVdRQXpjc2ZxanE3U0Zr?=
 =?utf-8?B?VmNMT1BHcDRIOVlqR0oyeWtWdkw1U0JyTmFScmZITWxkYkpTNGdQWGxPSlRN?=
 =?utf-8?B?MVpRVW9DU0FwakQrenlPek1VMk5PUHIwa3hEcWN1RmlENURsUHpjdWQzMDNO?=
 =?utf-8?B?d0h2NVJqbHhmMFExZGYrSU4rUis2MnFJK0lrMlRlVEdQbk9TV0V5WU5oMWtY?=
 =?utf-8?B?OVplYUlBbTdSRGFFRDZBM2NZZTBNajRsdUNnejNjMmlOSTdtc2Q2NXplQU5n?=
 =?utf-8?B?c2p5YUR2a0VJWVcvYlJoTnBrVmdGd0JtWFNnRzUxZnljZHIzZ1JMZHI1dUdO?=
 =?utf-8?B?ZFNOOUNZaFVRSU9qSVF5UEZLSHh2VFFqTWVJNmoyOXpEWHpwN1ZnMTdNa0FS?=
 =?utf-8?B?bVJiSlg2eUJtSXY2ZHdvRndUenpROGxGRGM4dlNwRFlMQzRZdHZXeEJSekh2?=
 =?utf-8?B?OXNkSHhJMnBkK3NCNGlmTFdQb3o0cW9KcEJKeE8weDdkU0ZOSitlL3lZVVFx?=
 =?utf-8?B?dHN5N2tIblVnM3hISXJLVmdtSFhNS25GWkh2bXFTK25OVS9OZTQrb1YyaTZO?=
 =?utf-8?B?L1NxVkVWMXk1S0I1bSsrZzk2NkNyck1TSkZZc2x2ZmxUYW9IUzlLUGlnYWVN?=
 =?utf-8?B?OXc9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 73a40892-9b01-40a6-ddbe-08db6dedc50b
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:13:38.4928
 (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: zCuHhrdFriVa4BSBh+1G1yWw+MoBWo0rdfPAURotz/4El5fYSpc1EcZPT2Th8+D+UZUKImw3MirqXY2D6Ep7sHKNp3rZQymnWcLMDvBblOQ=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY5PR11MB6367
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-2.2 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,
        RCVD_IN_MSPIKE_H2,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:
> Depending on the number of monitors available, Arm's MPAM may need to
> allocate a monitor prior to reading the counter value. Allocating a
> contended resource may involve sleeping.
> 
> add_rmid_to_limbo() calls resctrl_arch_rmid_read() for multiple domains,
> the allocation should be valid for all domains.
> 
> __check_limbo() and mon_event_count() each make multiple calls to
> resctrl_arch_rmid_read(), to avoid extra work on contended systems,
> the allocation should be valid for multiple invocations of
> resctrl_arch_rmid_read().
> 
> Add arch hooks for this allocation, which need calling before
> resctrl_arch_rmid_read(). The allocated monitor is passed to
> resctrl_arch_rmid_read(), then freed again afterwards. The helper
> can be called on any CPU, and can sleep.
> 
> Tested-by: Shaopeng Tan <tan.shaopeng@fujitsu.com>
> Signed-off-by: James Morse <james.morse@arm.com>
> ----

Note the extra "-" 

> Changes since v3:
>  * Expanded comment.
>  * Removed stray header include.
>  * Reworded commit message.
>  * Made ctx a void * instead of an int.
> ---

...

> diff --git a/arch/x86/kernel/cpu/resctrl/monitor.c b/arch/x86/kernel/cpu/resctrl/monitor.c
> index fb33100e172b..6d140018358a 100644
> --- a/arch/x86/kernel/cpu/resctrl/monitor.c
> +++ b/arch/x86/kernel/cpu/resctrl/monitor.c
> @@ -264,7 +264,7 @@ static u64 mbm_overflow_count(u64 prev_msr, u64 cur_msr, unsigned int width)
>  
>  int resctrl_arch_rmid_read(struct rdt_resource *r, struct rdt_domain *d,
>  			   u32 closid, u32 rmid, enum resctrl_event_id eventid,
> -			   u64 *val)
> +			   u64 *val, void *ignored)
>  {
>  	struct rdt_hw_resource *hw_res = resctrl_to_arch_res(r);
>  	struct rdt_hw_domain *hw_dom = resctrl_to_arch_dom(d);
> @@ -331,9 +331,14 @@ void __check_limbo(struct rdt_domain *d, bool force_free)
>  	u32 idx_limit = resctrl_arch_system_num_rmid_idx();
>  	struct rmid_entry *entry;
>  	u32 idx, cur_idx = 1;
> +	void *arch_mon_ctx;
>  	bool rmid_dirty;
>  	u64 val = 0;
>  
> +	arch_mon_ctx = resctrl_arch_mon_ctx_alloc(r, QOS_L3_OCCUP_EVENT_ID);
> +	if (arch_mon_ctx < 0)
> +		return;
> +

It seems that not all has adapted to the new type.

>  	/*
>  	 * Skip RMID 0 and start from RMID 1 and check all the RMIDs that
>  	 * are marked as busy for occupancy < threshold. If the occupancy
> @@ -347,7 +352,8 @@ void __check_limbo(struct rdt_domain *d, bool force_free)
>  
>  		entry = __rmid_entry(idx);
>  		if (resctrl_arch_rmid_read(r, d, entry->closid, entry->rmid,
> -					   QOS_L3_OCCUP_EVENT_ID, &val)) {
> +					   QOS_L3_OCCUP_EVENT_ID, &val,
> +					   arch_mon_ctx)) {
>  			rmid_dirty = true;
>  		} else {
>  			rmid_dirty = (val >= resctrl_rmid_realloc_threshold);
> @@ -360,6 +366,8 @@ void __check_limbo(struct rdt_domain *d, bool force_free)
>  		}
>  		cur_idx = idx + 1;
>  	}
> +
> +	resctrl_arch_mon_ctx_free(r, QOS_L3_OCCUP_EVENT_ID, arch_mon_ctx);
>  }
>  
>  bool has_busy_rmid(struct rdt_resource *r, struct rdt_domain *d)
> @@ -539,7 +547,7 @@ static int __mon_event_count(u32 closid, u32 rmid, struct rmid_read *rr)
>  	}
>  
>  	rr->err = resctrl_arch_rmid_read(rr->r, rr->d, closid, rmid, rr->evtid,
> -					 &tval);
> +					 &tval, rr->arch_mon_ctx);
>  	if (rr->err)
>  		return rr->err;
>  
> @@ -589,7 +597,6 @@ void mon_event_count(void *info)
>  	int ret;
>  
>  	rdtgrp = rr->rgrp;
> -
>  	ret = __mon_event_count(rdtgrp->closid, rdtgrp->mon.rmid, rr);
>  

Stray change.

>  	/*


Reinette