From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by smtp.subspace.kernel.org (Postfix) with ESMTP id E30E03AE6F5 for ; Wed, 4 Feb 2026 11:40:09 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=217.140.110.172 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1770205211; cv=none; b=u2oOc1dNMLF1gC2tp7p2Vt1HwucACGcbqaUxandp2LBdPeYOZjq0ElyszJGmYD/EUMEs1hrRdQUOqRGUuwPHJUZo+N0elbuWYUVuBPffUZW4k+/x2WYk/T0LBfOwyRaqxX4qm4x7aBUNwL9kEaBEUGOlaon30po9Mh56OA+cMDc= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1770205211; c=relaxed/simple; bh=b6H4q2+d9xSBIq8Ek7QGI9gutTUgbd4U8/MSLN7PSjY=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=oWKEiI54V1W3K0OYSPhVud9MNWZ1qCdrZqIIDtZ6RY+RtuqxwdusEnjxCmwSCx42VqyqwJtWKOrcOu87VtuNRdh2irOp/jvBJjt9yVE2xy4l8Iar/i2Nfgt7/0OrUUgZ1tv8ToBjXkiipJxTMSdNug6Mf5f+3BxMUM/shhWvlio= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=arm.com; spf=pass smtp.mailfrom=arm.com; arc=none smtp.client-ip=217.140.110.172 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=arm.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=arm.com 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 52AD0339; Wed, 4 Feb 2026 03:40:02 -0800 (PST) Received: from [10.1.196.46] (e134344.arm.com [10.1.196.46]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 8FBAC3F778; Wed, 4 Feb 2026 03:40:07 -0800 (PST) Message-ID: <009c0e49-2219-482e-a1fe-9920fa0832c7@arm.com> Date: Wed, 4 Feb 2026 11:40:06 +0000 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v2] fs/resctrl: Add missing kconfig entry for CONFIG_RESCTRL_ASSIGN_FIXED To: Reinette Chatre , linux-kernel@vger.kernel.org Cc: tony.luck@intel.com, Dave.Martin@arm.com, james.morse@arm.com, babu.moger@amd.com, bp@alien8.de References: <20260128161245.3085391-1-ben.horgan@arm.com> <69603ed1-5425-4efa-a786-e4e2a73c4851@intel.com> From: Ben Horgan Content-Language: en-US In-Reply-To: <69603ed1-5425-4efa-a786-e4e2a73c4851@intel.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Hi Reinette, On 2/2/26 21:50, Reinette Chatre wrote: > Hi Ben, > > I just have a couple of comments regarding style. > > Subject can be shortened by removing redundant words: > fs/resctrl: Add CONFIG_RESCTRL_ASSIGN_FIXED Kconfig entry Ok, I'll update to this. > > Please note the kconfig -> Kconfig change for rest of changelog also. > > On 1/28/26 8:12 AM, Ben Horgan wrote: >> The commit 3b497c3f4f04 ("fs/resctrl: Introduce the interface to display >> monitoring modes") introduced CONFIG_RESCTRL_ASSIGN_FIXED but left adding > > In x86 area there is a custom how to make references to a commit stand out > in the changelog. Please see commit a121798ae669 ("x86/resctrl: Fix allocation > of cleanest CLOSID on platforms with no monitors") for an example. > >> the kconfig entry until it was necessary. Add the kconfig entry as it is > > Note the kconfig -> Kconfig and please split the context from the solution (see > "Changelog" in Documentation/process/maintainer-tip.rst for reference). > >> now necessary in order to support ABMC on MPAM where the counter assignment > > ABMC is name of AMD's feature. MPAM need not adopt AMD's feature name. On resctrl > fs side it is just a generic "counter assignment" or "mbm_event" mode. > >> mode is indeed fixed. >> >> Also, take the opportunity ensure that a user attempt to change between >> different counter assignment mode fails from the resctrl code rather than >> delegating to the arch specific code and let the user know by adding a >> message in last_cmd_status. > > This can be connected to the change to not make it seem like an afterthought nor > contain trigger word ("Also, ...") that hints this is a logical change that > belongs in separate patch. Finally, the "let the user know ..." is not necessary > since it can be seen from the patch self. > > Here is an example that addresses above notes but please consider it critically > and do not just copy&paste without reading and considering corrections and improvement: > > > Commit > > 3b497c3f4f04 ("fs/resctrl: Introduce the interface to display monitoring modes") > > introduced CONFIG_RESCTRL_ASSIGN_FIXED but left adding the Kconfig entry until it > was necessary. > > Add CONFIG_RESCTRL_ASSIGN_FIXED in order to support MPAM where the counter assignment > mode is indeed fixed. > > CONFIG_RESCTRL_ASSIGN_FIXED is a resctrl fs Kconfig option so handle attempts > to modify counter assignment mode when it is enabled in resctrl fs rather than > delegating to arch specific code. Based on your comments and having a look at maintainer-tip.rst this seems like a good commit message that matches the commit. So, I'll go with it. Thanks for suggesting it. > > >> >> Signed-off-by: Ben Horgan >> --- >> Changes since v1: >> Update the commit message to make it clear this is an anticipated follow on >> patch rather than a fix. >> Only fail attempts to change to a different counter assignment mode. >> Kconfig indenting. >> Use "counter assignment mode" text throughout. >> --- >> fs/resctrl/Kconfig | 9 +++++++++ >> fs/resctrl/monitor.c | 6 ++++++ >> 2 files changed, 15 insertions(+) >> >> diff --git a/fs/resctrl/Kconfig b/fs/resctrl/Kconfig >> index 21671301bd8a..d833dea81aea 100644 >> --- a/fs/resctrl/Kconfig >> +++ b/fs/resctrl/Kconfig >> @@ -37,3 +37,12 @@ config RESCTRL_RMID_DEPENDS_ON_CLOSID >> Enabled by the architecture when the RMID values depend on the CLOSID. >> This causes the CLOSID allocator to search for CLOSID with clean >> RMID. >> + >> +config RESCTRL_ASSIGN_FIXED >> + bool >> + depends on RESCTRL_FS >> + help >> + Enabled by the architecture when the counter assignment mode is not >> + configurable. This ensures that counter assignment mode is not >> + advertised as configurable and attempts to change counter assignment >> + mode fail. >> diff --git a/fs/resctrl/monitor.c b/fs/resctrl/monitor.c >> index 572a9925bd6c..93756be50062 100644 >> --- a/fs/resctrl/monitor.c >> +++ b/fs/resctrl/monitor.c >> @@ -1451,6 +1451,12 @@ ssize_t resctrl_mbm_assign_mode_write(struct kernfs_open_file *of, char *buf, >> } >> >> if (enable != resctrl_arch_mbm_cntr_assign_enabled(r)) { >> + if (IS_ENABLED(CONFIG_RESCTRL_ASSIGN_FIXED)) { >> + ret = -EINVAL; >> + rdt_last_cmd_puts("counter assignment mode is not configurable\n"); > > Please start output to user space with upper case (unless it is a term that is usually written with > lower case). You can use "grep rdt_last_cmd_puts fs/resctrl/*" to get an idea if the patterns used. Yes, it looks better with a capital C. > >> + goto out_unlock; >> + } >> + >> ret = resctrl_arch_mbm_cntr_assign_set(r, enable); >> if (ret) >> goto out_unlock; > > > Reinette Thanks, Ben