[v6,10/24] x86/resctrl: Allocate the cleanest CLOSID by searching closid_num_dirty_rmid
Message ID | 20230914172138.11977-11-james.morse@arm.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a05:612c:172:b0:3f2:4152:657d with SMTP id h50csp512126vqi; Thu, 14 Sep 2023 10:34:49 -0700 (PDT) X-Google-Smtp-Source: AGHT+IFhsL+hMXdwjyh+fqcLHQQUaANQeBM4Pg8ec+gSt/4e0hCC9PrCn2Mlxb1ReQeGQIRMkyDD X-Received: by 2002:a17:902:cec4:b0:1c3:c687:4793 with SMTP id d4-20020a170902cec400b001c3c6874793mr7653124plg.63.1694712889203; Thu, 14 Sep 2023 10:34:49 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1694712889; cv=none; d=google.com; s=arc-20160816; b=ZRaogV7HHn9K3BLU/DgKdDJyD+MZgDe9SzSWr0uLPOE+WUAvZ5+fxKysE+hvqzqCTi ZEUIsKGGdCNGl6xt0DSspQFyVXDBg9rTeG87ZZ9JPbKOnpNHOSj7hqiDzdi8Q3Ns7WfN /Rg+5C+ClIIZwTGoUq3nVem+Ux1RBcmkiDPm7lPWTAaJS/zpuPougP9AP92UrACHtt3m mmbf7I8tt1mDdNFPsbU+uCYGrupcnvVJxjKLRbN2Egagg7rlUR8HBOrQO3FoKPm2slUr lozvq+KISc/YNlm3DGQ32d+02r0qo7NqvWItr9GCQxPc/Kk2X8ngsqx60fSA9OZEmFH3 SWKA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from; bh=WAJYn3+0BcA0Dq+01H+qBJxQjyLSKwv482QuJEqjqSc=; fh=6Nl2MjDBXofbhMXyzAhf7L7loTtEBflFxRNey74qSFc=; b=oJZlkisHiai7iH18cwFBxU5qlwkN/H6EIQ3T8/MQszh7IEQs3GqYc2MxB5a5LUo7te p1TfEb0enXTTBPbd6Tdm7b0d/7M+bfZYMzAw5ZN5qmBcBEWJQMikMTXFnfejAksKMyyJ jWvlt1U3rZVAK28eJ1xO3a6dnw/diL7Wug+hs3OvEsOS6gx247pEn4se4XtS15FID6A1 8RK0ko83K66V+q6wh35AlxbJF/aImOr3zVYEi2c2UO/mWcGbLobB7JrSTXQ6ocZ7UIct yaVnjei5FqixlT/sntU5umxvMUwodRLDMHAr6JMQFD6UQVQ53xsskpgTYIDRqaxUlYyR XQng== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.31 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=arm.com Received: from morse.vger.email (morse.vger.email. [23.128.96.31]) by mx.google.com with ESMTPS id j4-20020a170902da8400b001c1f1394bf9si2201139plx.357.2023.09.14.10.34.47 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 14 Sep 2023 10:34:49 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.31 as permitted sender) client-ip=23.128.96.31; Authentication-Results: mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.31 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=arm.com Received: from out1.vger.email (depot.vger.email [IPv6:2620:137:e000::3:0]) by morse.vger.email (Postfix) with ESMTP id 5319F8241E03; Thu, 14 Sep 2023 10:23:11 -0700 (PDT) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.10 at morse.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S240324AbjINRW7 (ORCPT <rfc822;pwkd43@gmail.com> + 33 others); Thu, 14 Sep 2023 13:22:59 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:38658 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S240199AbjINRWn (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Thu, 14 Sep 2023 13:22:43 -0400 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by lindbergh.monkeyblade.net (Postfix) with ESMTP id DE2941FF5 for <linux-kernel@vger.kernel.org>; Thu, 14 Sep 2023 10:22:26 -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 DA9AA1FB; Thu, 14 Sep 2023 10:23:03 -0700 (PDT) Received: from merodach.members.linode.com (unknown [172.31.20.19]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id D7FAF3F5A1; Thu, 14 Sep 2023 10:22:23 -0700 (PDT) From: James Morse <james.morse@arm.com> To: x86@kernel.org, linux-kernel@vger.kernel.org Cc: Fenghua Yu <fenghua.yu@intel.com>, Reinette Chatre <reinette.chatre@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>, James Morse <james.morse@arm.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, amitsinght@marvell.com Subject: [PATCH v6 10/24] x86/resctrl: Allocate the cleanest CLOSID by searching closid_num_dirty_rmid Date: Thu, 14 Sep 2023 17:21:24 +0000 Message-Id: <20230914172138.11977-11-james.morse@arm.com> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20230914172138.11977-1-james.morse@arm.com> References: <20230914172138.11977-1-james.morse@arm.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit 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 (morse.vger.email [0.0.0.0]); Thu, 14 Sep 2023 10:23:11 -0700 (PDT) X-Spam-Status: No, score=-0.8 required=5.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on morse.vger.email X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1777035262603730559 X-GMAIL-MSGID: 1777035262603730559 |
Series |
x86/resctrl: monitored closid+rmid together, separate arch/fs locking
|
|
Commit Message
James Morse
Sept. 14, 2023, 5:21 p.m. UTC
MPAM's PMG bits extend its PARTID space, meaning the same PMG value can be used for different control groups. This means once a CLOSID is allocated, all its monitoring ids may still be dirty, and held in limbo. Instead of allocating the first free CLOSID, on architectures where CONFIG_RESCTRL_RMID_DEPENDS_ON_COSID is enabled, search closid_num_dirty_rmid[] to find the cleanest CLOSID. The CLOSID found is returned to closid_alloc() for the free list to be updated. Reviewed-by: Shaopeng Tan <tan.shaopeng@fujitsu.com> Tested-by: Shaopeng Tan <tan.shaopeng@fujitsu.com> Tested-By: Peter Newman <peternewman@google.com> Signed-off-by: James Morse <james.morse@arm.com> --- Changes since v4: * Dropped stale section from comment --- arch/x86/kernel/cpu/resctrl/internal.h | 2 ++ arch/x86/kernel/cpu/resctrl/monitor.c | 42 ++++++++++++++++++++++++++ arch/x86/kernel/cpu/resctrl/rdtgroup.c | 19 +++++++++--- 3 files changed, 58 insertions(+), 5 deletions(-)
Comments
Hi James, On 9/14/2023 10:21 AM, James Morse wrote: > MPAM's PMG bits extend its PARTID space, meaning the same PMG value can be > used for different control groups. > > This means once a CLOSID is allocated, all its monitoring ids may still be > dirty, and held in limbo. > > Instead of allocating the first free CLOSID, on architectures where > CONFIG_RESCTRL_RMID_DEPENDS_ON_COSID is enabled, search > closid_num_dirty_rmid[] to find the cleanest CLOSID. > > The CLOSID found is returned to closid_alloc() for the free list > to be updated. > > Reviewed-by: Shaopeng Tan <tan.shaopeng@fujitsu.com> > Tested-by: Shaopeng Tan <tan.shaopeng@fujitsu.com> > Tested-By: Peter Newman <peternewman@google.com> > Signed-off-by: James Morse <james.morse@arm.com> Reviewed-by: Reinette Chatre <reinette.chatre@intel.com> Reinette
Hi James, On 9/14/2023 12:21 PM, James Morse wrote: > MPAM's PMG bits extend its PARTID space, meaning the same PMG value can be > used for different control groups. > > This means once a CLOSID is allocated, all its monitoring ids may still be > dirty, and held in limbo. > > Instead of allocating the first free CLOSID, on architectures where > CONFIG_RESCTRL_RMID_DEPENDS_ON_COSID is enabled, search > closid_num_dirty_rmid[] to find the cleanest CLOSID. > > The CLOSID found is returned to closid_alloc() for the free list > to be updated. > > Reviewed-by: Shaopeng Tan <tan.shaopeng@fujitsu.com> > Tested-by: Shaopeng Tan <tan.shaopeng@fujitsu.com> > Tested-By: Peter Newman <peternewman@google.com> > Signed-off-by: James Morse <james.morse@arm.com> > --- > Changes since v4: > * Dropped stale section from comment > --- > arch/x86/kernel/cpu/resctrl/internal.h | 2 ++ > arch/x86/kernel/cpu/resctrl/monitor.c | 42 ++++++++++++++++++++++++++ > arch/x86/kernel/cpu/resctrl/rdtgroup.c | 19 +++++++++--- > 3 files changed, 58 insertions(+), 5 deletions(-) > > diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h > index ad6e874d9ed2..f06d3d3e0808 100644 > --- a/arch/x86/kernel/cpu/resctrl/internal.h > +++ b/arch/x86/kernel/cpu/resctrl/internal.h > @@ -558,5 +558,7 @@ void rdt_domain_reconfigure_cdp(struct rdt_resource *r); > void __init thread_throttle_mode_init(void); > void __init mbm_config_rftype_init(const char *config); > void rdt_staged_configs_clear(void); > +bool closid_allocated(unsigned int closid); > +int resctrl_find_cleanest_closid(void); > > #endif /* _ASM_X86_RESCTRL_INTERNAL_H */ > diff --git a/arch/x86/kernel/cpu/resctrl/monitor.c b/arch/x86/kernel/cpu/resctrl/monitor.c > index 0c783301d106..0bbed8c62d42 100644 > --- a/arch/x86/kernel/cpu/resctrl/monitor.c > +++ b/arch/x86/kernel/cpu/resctrl/monitor.c > @@ -388,6 +388,48 @@ static struct rmid_entry *resctrl_find_free_rmid(u32 closid) > return ERR_PTR(-ENOSPC); > } > > +/** > + * resctrl_find_cleanest_closid() - Find a CLOSID where all the associated > + * RMID are clean, or the CLOSID that has > + * the most clean RMID. > + * > + * MPAM's equivalent of RMID are per-CLOSID, meaning a freshly allocated CLOSID > + * may not be able to allocate clean RMID. To avoid this the allocator will > + * choose the CLOSID with the most clean RMID. > + * > + * When the CLOSID and RMID are independent numbers, the first free CLOSID will > + * be returned. > + */ > +int resctrl_find_cleanest_closid(void) > +{ > + u32 cleanest_closid = ~0, iter_num_dirty; Just naming num_dirty should have been fine. I will leave it you. > + int i = 0; > + > + lockdep_assert_held(&rdtgroup_mutex); > + > + if (!IS_ENABLED(CONFIG_RESCTRL_RMID_DEPENDS_ON_CLOSID)) > + return -EIO; > + > + for (i = 0; i < closids_supported(); i++) { > + if (closid_allocated(i)) > + continue; > + > + iter_num_dirty = closid_num_dirty_rmid[i]; > + if (iter_num_dirty == 0) > + return i; > + > + if (cleanest_closid == ~0) > + cleanest_closid = i; > + > + if (iter_num_dirty < closid_num_dirty_rmid[cleanest_closid]) > + cleanest_closid = i; > + } > + > + if (cleanest_closid == ~0) > + return -ENOSPC; > + return cleanest_closid; Line before the return looks clean. * if (cleanest_closid == ~0) + return -ENOSPC; + + return cleanest_closid; Thanks Babu > +} > + > /* > * For MPAM the RMID value is not unique, and has to be considered with > * the CLOSID. The (CLOSID, RMID) pair is allocated on all domains, which > diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c > index fa449ee0d1a7..1f8f1c417a4b 100644 > --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c > +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c > @@ -132,11 +132,20 @@ static void closid_init(void) > > static int closid_alloc(void) > { > - u32 closid = ffs(closid_free_map); > + u32 closid; > + int err; > > - if (closid == 0) > - return -ENOSPC; > - closid--; > + if (IS_ENABLED(CONFIG_RESCTRL_RMID_DEPENDS_ON_CLOSID)) { > + err = resctrl_find_cleanest_closid(); > + if (err < 0) > + return err; > + closid = err; > + } else { > + closid = ffs(closid_free_map); > + if (closid == 0) > + return -ENOSPC; > + closid--; > + } > clear_bit(closid, &closid_free_map); > > return closid; > @@ -154,7 +163,7 @@ void closid_free(int closid) > * Return: true if @closid is currently associated with a resource group, > * false if @closid is free > */ > -static bool closid_allocated(unsigned int closid) > +bool closid_allocated(unsigned int closid) > { > return !test_bit(closid, &closid_free_map); > }
Hi James, One more comment. On 9/14/2023 12:21 PM, James Morse wrote: > MPAM's PMG bits extend its PARTID space, meaning the same PMG value can be > used for different control groups. > > This means once a CLOSID is allocated, all its monitoring ids may still be > dirty, and held in limbo. > > Instead of allocating the first free CLOSID, on architectures where > CONFIG_RESCTRL_RMID_DEPENDS_ON_COSID is enabled, search > closid_num_dirty_rmid[] to find the cleanest CLOSID. > > The CLOSID found is returned to closid_alloc() for the free list > to be updated. > > Reviewed-by: Shaopeng Tan <tan.shaopeng@fujitsu.com> > Tested-by: Shaopeng Tan <tan.shaopeng@fujitsu.com> > Tested-By: Peter Newman <peternewman@google.com> > Signed-off-by: James Morse <james.morse@arm.com> > --- > Changes since v4: > * Dropped stale section from comment > --- > arch/x86/kernel/cpu/resctrl/internal.h | 2 ++ > arch/x86/kernel/cpu/resctrl/monitor.c | 42 ++++++++++++++++++++++++++ > arch/x86/kernel/cpu/resctrl/rdtgroup.c | 19 +++++++++--- > 3 files changed, 58 insertions(+), 5 deletions(-) > > diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h > index ad6e874d9ed2..f06d3d3e0808 100644 > --- a/arch/x86/kernel/cpu/resctrl/internal.h > +++ b/arch/x86/kernel/cpu/resctrl/internal.h > @@ -558,5 +558,7 @@ void rdt_domain_reconfigure_cdp(struct rdt_resource *r); > void __init thread_throttle_mode_init(void); > void __init mbm_config_rftype_init(const char *config); > void rdt_staged_configs_clear(void); > +bool closid_allocated(unsigned int closid); > +int resctrl_find_cleanest_closid(void); > > #endif /* _ASM_X86_RESCTRL_INTERNAL_H */ > diff --git a/arch/x86/kernel/cpu/resctrl/monitor.c b/arch/x86/kernel/cpu/resctrl/monitor.c > index 0c783301d106..0bbed8c62d42 100644 > --- a/arch/x86/kernel/cpu/resctrl/monitor.c > +++ b/arch/x86/kernel/cpu/resctrl/monitor.c > @@ -388,6 +388,48 @@ static struct rmid_entry *resctrl_find_free_rmid(u32 closid) > return ERR_PTR(-ENOSPC); > } > > +/** > + * resctrl_find_cleanest_closid() - Find a CLOSID where all the associated > + * RMID are clean, or the CLOSID that has > + * the most clean RMID. > + * > + * MPAM's equivalent of RMID are per-CLOSID, meaning a freshly allocated CLOSID > + * may not be able to allocate clean RMID. To avoid this the allocator will > + * choose the CLOSID with the most clean RMID. > + * > + * When the CLOSID and RMID are independent numbers, the first free CLOSID will > + * be returned. > + */ > +int resctrl_find_cleanest_closid(void) > +{ > + u32 cleanest_closid = ~0, iter_num_dirty; > + int i = 0; > + > + lockdep_assert_held(&rdtgroup_mutex); > + > + if (!IS_ENABLED(CONFIG_RESCTRL_RMID_DEPENDS_ON_CLOSID)) > + return -EIO; > + > + for (i = 0; i < closids_supported(); i++) { > + if (closid_allocated(i)) > + continue; > + > + iter_num_dirty = closid_num_dirty_rmid[i]; > + if (iter_num_dirty == 0) > + return i; > + > + if (cleanest_closid == ~0) > + cleanest_closid = i; > + > + if (iter_num_dirty < closid_num_dirty_rmid[cleanest_closid]) > + cleanest_closid = i; > + } > + > + if (cleanest_closid == ~0) > + return -ENOSPC; > + return cleanest_closid; > +} > + > /* > * For MPAM the RMID value is not unique, and has to be considered with > * the CLOSID. The (CLOSID, RMID) pair is allocated on all domains, which > diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c > index fa449ee0d1a7..1f8f1c417a4b 100644 > --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c > +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c > @@ -132,11 +132,20 @@ static void closid_init(void) > > static int closid_alloc(void) > { > - u32 closid = ffs(closid_free_map); > + u32 closid; > + int err; Naming "err" seems odd here. How about cleanest_closid ? Thanks Babu > > - if (closid == 0) > - return -ENOSPC; > - closid--; > + if (IS_ENABLED(CONFIG_RESCTRL_RMID_DEPENDS_ON_CLOSID)) { > + err = resctrl_find_cleanest_closid(); > + if (err < 0) > + return err; > + closid = err; > + } else { > + closid = ffs(closid_free_map); > + if (closid == 0) > + return -ENOSPC; > + closid--; > + } > clear_bit(closid, &closid_free_map); > > return closid; > @@ -154,7 +163,7 @@ void closid_free(int closid) > * Return: true if @closid is currently associated with a resource group, > * false if @closid is free > */ > -static bool closid_allocated(unsigned int closid) > +bool closid_allocated(unsigned int closid) > { > return !test_bit(closid, &closid_free_map); > }
On 2023-09-14 at 17:21:24 +0000, James Morse wrote: >MPAM's PMG bits extend its PARTID space, meaning the same PMG value can be >used for different control groups. > >This means once a CLOSID is allocated, all its monitoring ids may still be >dirty, and held in limbo. > >Instead of allocating the first free CLOSID, on architectures where >CONFIG_RESCTRL_RMID_DEPENDS_ON_COSID is enabled, search "CONFIG_RESCTRL_RMID_DEPENDS_ON_COSID" > "CONFIG_RESCTRL_RMID_DEPENDS_ON_CLOSID"? >closid_num_dirty_rmid[] to find the cleanest CLOSID. > >The CLOSID found is returned to closid_alloc() for the free list >to be updated.
Hi Babu, On 05/10/2023 21:13, Moger, Babu wrote: > On 9/14/2023 12:21 PM, James Morse wrote: >> MPAM's PMG bits extend its PARTID space, meaning the same PMG value can be >> used for different control groups. >> >> This means once a CLOSID is allocated, all its monitoring ids may still be >> dirty, and held in limbo. >> >> Instead of allocating the first free CLOSID, on architectures where >> CONFIG_RESCTRL_RMID_DEPENDS_ON_COSID is enabled, search >> closid_num_dirty_rmid[] to find the cleanest CLOSID. >> >> The CLOSID found is returned to closid_alloc() for the free list >> to be updated. >> diff --git a/arch/x86/kernel/cpu/resctrl/internal.h >> b/arch/x86/kernel/cpu/resctrl/internal.h >> index ad6e874d9ed2..f06d3d3e0808 100644 >> --- a/arch/x86/kernel/cpu/resctrl/internal.h >> +++ b/arch/x86/kernel/cpu/resctrl/internal.h >> @@ -558,5 +558,7 @@ void rdt_domain_reconfigure_cdp(struct rdt_resource *r); >> void __init thread_throttle_mode_init(void); >> void __init mbm_config_rftype_init(const char *config); >> void rdt_staged_configs_clear(void); >> +bool closid_allocated(unsigned int closid); >> +int resctrl_find_cleanest_closid(void); >> #endif /* _ASM_X86_RESCTRL_INTERNAL_H */ >> diff --git a/arch/x86/kernel/cpu/resctrl/monitor.c b/arch/x86/kernel/cpu/resctrl/monitor.c >> index 0c783301d106..0bbed8c62d42 100644 >> --- a/arch/x86/kernel/cpu/resctrl/monitor.c >> +++ b/arch/x86/kernel/cpu/resctrl/monitor.c >> @@ -388,6 +388,48 @@ static struct rmid_entry *resctrl_find_free_rmid(u32 closid) >> return ERR_PTR(-ENOSPC); >> } >> +/** >> + * resctrl_find_cleanest_closid() - Find a CLOSID where all the associated >> + * RMID are clean, or the CLOSID that has >> + * the most clean RMID. >> + * >> + * MPAM's equivalent of RMID are per-CLOSID, meaning a freshly allocated CLOSID >> + * may not be able to allocate clean RMID. To avoid this the allocator will >> + * choose the CLOSID with the most clean RMID. >> + * >> + * When the CLOSID and RMID are independent numbers, the first free CLOSID will >> + * be returned. >> + */ >> +int resctrl_find_cleanest_closid(void) >> +{ >> + u32 cleanest_closid = ~0, iter_num_dirty; > > Just naming num_dirty should have been fine. I will leave it you. That was to make it obvious its something to do with the loop, so the value can't be relied on outside that. I'll rename it and move the declaration into the loop, that way its out of scope if someone tries to use it later. >> + int i = 0; >> + >> + lockdep_assert_held(&rdtgroup_mutex); >> + >> + if (!IS_ENABLED(CONFIG_RESCTRL_RMID_DEPENDS_ON_CLOSID)) >> + return -EIO; >> + >> + for (i = 0; i < closids_supported(); i++) { >> + if (closid_allocated(i)) >> + continue; >> + >> + iter_num_dirty = closid_num_dirty_rmid[i]; >> + if (iter_num_dirty == 0) >> + return i; >> + >> + if (cleanest_closid == ~0) >> + cleanest_closid = i; >> + >> + if (iter_num_dirty < closid_num_dirty_rmid[cleanest_closid]) >> + cleanest_closid = i; >> + } >> + >> + if (cleanest_closid == ~0) >> + return -ENOSPC; >> + return cleanest_closid; > > Line before the return looks clean. > > * if (cleanest_closid == ~0) > + return -ENOSPC; > + > + return cleanest_closid; Sure, Thanks, James
Hi Babu, On 05/10/2023 21:26, Moger, Babu wrote: > On 9/14/2023 12:21 PM, James Morse wrote: >> MPAM's PMG bits extend its PARTID space, meaning the same PMG value can be >> used for different control groups. >> >> This means once a CLOSID is allocated, all its monitoring ids may still be >> dirty, and held in limbo. >> >> Instead of allocating the first free CLOSID, on architectures where >> CONFIG_RESCTRL_RMID_DEPENDS_ON_COSID is enabled, search >> closid_num_dirty_rmid[] to find the cleanest CLOSID. >> >> The CLOSID found is returned to closid_alloc() for the free list >> to be updated. >> diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c >> b/arch/x86/kernel/cpu/resctrl/rdtgroup.c >> index fa449ee0d1a7..1f8f1c417a4b 100644 >> --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c >> +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c >> @@ -132,11 +132,20 @@ static void closid_init(void) >> static int closid_alloc(void) >> { >> - u32 closid = ffs(closid_free_map); >> + u32 closid; >> + int err; > Naming "err" seems odd here. How about cleanest_closid ? That's just habit because the value might be an error code until its been checked, and once it has, its called 'closid'. But sure, if you think that is clearer. Thanks, James
diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h index ad6e874d9ed2..f06d3d3e0808 100644 --- a/arch/x86/kernel/cpu/resctrl/internal.h +++ b/arch/x86/kernel/cpu/resctrl/internal.h @@ -558,5 +558,7 @@ void rdt_domain_reconfigure_cdp(struct rdt_resource *r); void __init thread_throttle_mode_init(void); void __init mbm_config_rftype_init(const char *config); void rdt_staged_configs_clear(void); +bool closid_allocated(unsigned int closid); +int resctrl_find_cleanest_closid(void); #endif /* _ASM_X86_RESCTRL_INTERNAL_H */ diff --git a/arch/x86/kernel/cpu/resctrl/monitor.c b/arch/x86/kernel/cpu/resctrl/monitor.c index 0c783301d106..0bbed8c62d42 100644 --- a/arch/x86/kernel/cpu/resctrl/monitor.c +++ b/arch/x86/kernel/cpu/resctrl/monitor.c @@ -388,6 +388,48 @@ static struct rmid_entry *resctrl_find_free_rmid(u32 closid) return ERR_PTR(-ENOSPC); } +/** + * resctrl_find_cleanest_closid() - Find a CLOSID where all the associated + * RMID are clean, or the CLOSID that has + * the most clean RMID. + * + * MPAM's equivalent of RMID are per-CLOSID, meaning a freshly allocated CLOSID + * may not be able to allocate clean RMID. To avoid this the allocator will + * choose the CLOSID with the most clean RMID. + * + * When the CLOSID and RMID are independent numbers, the first free CLOSID will + * be returned. + */ +int resctrl_find_cleanest_closid(void) +{ + u32 cleanest_closid = ~0, iter_num_dirty; + int i = 0; + + lockdep_assert_held(&rdtgroup_mutex); + + if (!IS_ENABLED(CONFIG_RESCTRL_RMID_DEPENDS_ON_CLOSID)) + return -EIO; + + for (i = 0; i < closids_supported(); i++) { + if (closid_allocated(i)) + continue; + + iter_num_dirty = closid_num_dirty_rmid[i]; + if (iter_num_dirty == 0) + return i; + + if (cleanest_closid == ~0) + cleanest_closid = i; + + if (iter_num_dirty < closid_num_dirty_rmid[cleanest_closid]) + cleanest_closid = i; + } + + if (cleanest_closid == ~0) + return -ENOSPC; + return cleanest_closid; +} + /* * For MPAM the RMID value is not unique, and has to be considered with * the CLOSID. The (CLOSID, RMID) pair is allocated on all domains, which diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c index fa449ee0d1a7..1f8f1c417a4b 100644 --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c @@ -132,11 +132,20 @@ static void closid_init(void) static int closid_alloc(void) { - u32 closid = ffs(closid_free_map); + u32 closid; + int err; - if (closid == 0) - return -ENOSPC; - closid--; + if (IS_ENABLED(CONFIG_RESCTRL_RMID_DEPENDS_ON_CLOSID)) { + err = resctrl_find_cleanest_closid(); + if (err < 0) + return err; + closid = err; + } else { + closid = ffs(closid_free_map); + if (closid == 0) + return -ENOSPC; + closid--; + } clear_bit(closid, &closid_free_map); return closid; @@ -154,7 +163,7 @@ void closid_free(int closid) * Return: true if @closid is currently associated with a resource group, * false if @closid is free */ -static bool closid_allocated(unsigned int closid) +bool closid_allocated(unsigned int closid) { return !test_bit(closid, &closid_free_map); }