Received: by 2002:a05:6358:9144:b0:117:f937:c515 with SMTP id r4csp722385rwr; Thu, 27 Apr 2023 07:23:17 -0700 (PDT) X-Google-Smtp-Source: ACHHUZ4iZ04OjSTD+S2JJKl5xVlRNHa56X0uxlyVq86ULCMdxf7JeVSEdzvekk690Mb6JOwelBh+ X-Received: by 2002:a17:902:dac1:b0:1a6:93cc:924b with SMTP id q1-20020a170902dac100b001a693cc924bmr2149370plx.3.1682605397105; Thu, 27 Apr 2023 07:23:17 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1682605397; cv=none; d=google.com; s=arc-20160816; b=HsF+TYgPTt2hjjBDQJvCQVs5LRn+x3URHIuW2frBXF9x4gpDmHj4Nr8CguEs1hlu/a hqQHFlnFzN3rvj0bUJS4kMIDhfGw6PXBnHhbYgsiW5ujZU/yNZfpkfzqKFIr2qa4Tvd0 Ogd6r89D+OemM4Ad8Q5z1nPHL1pKcFQqC0ovRG6Gpl1m3+7gfWnIOqYmrOtQSTahBF1d LQ/W4ZK/GK2HZEWectUwoKO0zIBXdB4Au/a1PyxwzNdEOWlLu8c7KjI6nmAR+ZowfWNa DzW59/plhH3Ns+/Dfv2XNyMKQ8tIsKvsYf03+Lo5+7WvNZBGVEhKWKhUhTABGJQVRopC 5FtQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:in-reply-to:from :references:cc:to:content-language:subject:user-agent:mime-version :date:message-id; bh=GCHd8F4uyX2YUha0H/72sy09mcw24mgObMXv3JAo1+8=; b=PRSBzgfZ7HwHJ+Ew6m6rIGVkp4sQNxqFsNStYgjbbZ+XTw+PswIMClCJf08WRgvizm EGjnVZxM6y3V0HrHTpzOz5P0zGihf6G2+mJyA1I5P91PqjcaRxyPY4PT+AI2ds0qMYvB B1D0KNjvgsgc6luM1ATDORXpcIT+EuOmBHnZ3peKrpngPMWz5uMKMqwkvDk5FhJ48Lhv AlVh7ZhjDLuGZenm4bUIk2nikctueYeR1O30Mb+17TpKdcRVLoxUDYEb9wc+gS7vncrb DllRjtCVgsdJzUMCIaMi28tEb4nHyn6JwuKqB78r+LN/1yBLgmSXfVQlt96v0kkEpVhz bR2Q== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=arm.com Return-Path: Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id lk4-20020a17090308c400b0019ccffb3fd3si18348354plb.509.2023.04.27.07.23.04; Thu, 27 Apr 2023 07:23:17 -0700 (PDT) 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; Authentication-Results: mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=arm.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S243945AbjD0OUB (ORCPT + 99 others); Thu, 27 Apr 2023 10:20:01 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:38040 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S243964AbjD0OTw (ORCPT ); Thu, 27 Apr 2023 10:19:52 -0400 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by lindbergh.monkeyblade.net (Postfix) with ESMTP id 8164E10DD for ; Thu, 27 Apr 2023 07:19:51 -0700 (PDT) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 4A5CDC14; Thu, 27 Apr 2023 07:20:35 -0700 (PDT) Received: from [10.1.196.177] (eglon.cambridge.arm.com [10.1.196.177]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 4F9C53F7D8; Thu, 27 Apr 2023 07:19:44 -0700 (PDT) Message-ID: <24d3616a-7800-ba91-deed-8bcc639ce6ba@arm.com> Date: Thu, 27 Apr 2023 15:19:41 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux aarch64; rv:91.0) Gecko/20100101 Thunderbird/91.13.0 Subject: Re: [PATCH v3 11/19] x86/resctrl: Allow arch to allocate memory needed in resctrl_arch_rmid_read() Content-Language: en-GB To: Reinette Chatre , x86@kernel.org, linux-kernel@vger.kernel.org Cc: Fenghua Yu , Thomas Gleixner , Ingo Molnar , Borislav Petkov , H Peter Anvin , Babu Moger , shameerali.kolothum.thodi@huawei.com, D Scott Phillips OS , 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 , Xin Hao , peternewman@google.com References: <20230320172620.18254-1-james.morse@arm.com> <20230320172620.18254-12-james.morse@arm.com> <36af82d5-0d48-f899-9e95-1ec89be20581@intel.com> From: James Morse In-Reply-To: <36af82d5-0d48-f899-9e95-1ec89be20581@intel.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-5.6 required=5.0 tests=BAYES_00,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: X-Mailing-List: linux-kernel@vger.kernel.org Hi Reinette, On 01/04/2023 00:27, Reinette Chatre wrote: > On 3/20/2023 10:26 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. >> >> All callers of resctrl_arch_rmid_read() read the counter on more than >> one domain. If the monitor is allocated globally, there is no need to > > This does not seem accurate considering the __check_limbo() call that > is called for a single domain. True, it was add_rmid_to_limbo() that motivated this being global, but its conflated with holding the allocation for multiple invocations of resctrl_arch_rmid_read() for the multiple groups that __check_limbo() walks over, and the calls for each monitor group that mon_event_count() makes. >> allocate and free it for each call to 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. >> diff --git a/arch/x86/kernel/cpu/resctrl/monitor.c b/arch/x86/kernel/cpu/resctrl/monitor.c >> index de72df06b37b..f38cd2f12285 100644 >> --- a/arch/x86/kernel/cpu/resctrl/monitor.c >> +++ b/arch/x86/kernel/cpu/resctrl/monitor.c >> @@ -15,6 +15,7 @@ >> * Software Developer Manual June 2016, volume 3, section 17.17. >> */ >> >> +#include > > Why is this needed? lockdep_assert_cpus_held(), but that got folded out to a latter patch. I've moved it there. >> #include >> #include >> #include >> @@ -271,7 +272,7 @@ static void smp_call_rmid_read(void *_arg) >> >> 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, int ignored) >> { >> struct rdt_hw_resource *hw_res = resctrl_to_arch_res(r); >> struct rdt_hw_domain *hw_dom = resctrl_to_arch_dom(d); >> @@ -317,9 +318,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; >> + int 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; > The vision for this is not clear to me. When I read that context needs to be allocated > I expect it to return a pointer to some new context, not an int. What would the > "context" consist of? It might just need a different name. For MPAM, this is allocating a monitor, which is the hardware that does the counting in the cache or the memory controller. The number of monitors is an implementation choice, and may not match the number of CLOSID/RMID that are in use. There aren't guaranteed to be enough to allocate one for every control or monitor group up front. The int being returned is the allocated monitor's index. It identifies which monitor needs programming to read the provided CLOSID/RMID, and the counter register to read with the value. I can allocate memory for an int if you think that is clearer. (I was hoping to leave that for whoever needs something bigger than a pointer) >> diff --git a/include/linux/resctrl.h b/include/linux/resctrl.h >> index ff7452f644e4..03e4f41cd336 100644 >> --- a/include/linux/resctrl.h >> +++ b/include/linux/resctrl.h >> @@ -233,6 +233,7 @@ void resctrl_offline_domain(struct rdt_resource *r, struct rdt_domain *d); >> * @rmid: rmid of the counter to read. >> * @eventid: eventid to read, e.g. L3 occupancy. >> * @val: result of the counter read in bytes. >> + * @arch_mon_ctx: An allocated context from resctrl_arch_mon_ctx_alloc(). >> * > Could this description be expanded to indicate what this context is used for? Sure, "An architecture specific value from resctrl_arch_mon_ctx_alloc(), for MPAM this identifies the hardware monitor allocated for this read request". Thanks, James