public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Dave Martin <Dave.Martin@arm.com>
To: x86@kernel.org, linux-kernel@vger.kernel.org
Cc: James Morse <james.morse@arm.com>,
	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>,
	shameerali.kolothum.thodi@huawei.com,
	D Scott Phillips OS <scott@os.amperecomputing.com>,
	carl@os.amperecomputing.com, lcherian@marvell.com,
	bobo.shaobowang@huawei.com,
	"Shaopeng Tan (Fujitsu)" <tan.shaopeng@fujitsu.com>,
	baolin.wang@linux.alibaba.com,
	Jamie Iles <quic_jiles@quicinc.com>,
	Xin Hao <xhao@linux.alibaba.com>,
	Peter Newman <peternewman@google.com>,
	dfustini@baylibre.com, amitsinght@marvell.com,
	David Hildenbrand <david@redhat.com>,
	Rex Nie <rex.nie@jaguarmicro.com>
Subject: [PATCH v2 03/35] x86/resctrl: Move ctrlval string parsing policy away from the arch code
Date: Fri, 26 Apr 2024 16:08:32 +0100	[thread overview]
Message-ID: <20240426150904.8854-3-Dave.Martin@arm.com> (raw)
In-Reply-To: <20240426150904.8854-1-Dave.Martin@arm.com>

From: James Morse <james.morse@arm.com>

The policy for parsing the configuration values as a string from
user-space is specified by a function pointer the arch code specifies.

These strings are part of resctrl's ABI, and the functions and their
caller both live in the same file. Exporting the parsing functions and
allowing the architecture to choose how a schema is parsed allows an
architecture to get this wrong.

Keep this all in the filesystem parts of resctrl. This should prevent
any architecture's string-parsing behaviour from varying without core
code changes. Use the fflags to spot caches and bandwidth resources,
and use the appropriate helper.

Signed-off-by: James Morse <james.morse@arm.com>

---

FYI: Dave Martin (et al.):

 * Needs refactoring to minimise exposure of resctrl internals to arch
   code, particularly regarding fflags, print format and format parser.

   See discussion thread:

   https://lore.kernel.org/all/e85e7786-7995-42d5-a5ac-1e08a84492fe@intel.com/

Changes in v2:

 * [Commit message only] Typo fix:
   s/flesystem/filesystem/g
---
 arch/x86/kernel/cpu/resctrl/core.c        |  4 ----
 arch/x86/kernel/cpu/resctrl/ctrlmondata.c | 28 +++++++++++++++++++----
 arch/x86/kernel/cpu/resctrl/internal.h    | 10 --------
 include/linux/resctrl.h                   |  7 ------
 4 files changed, 23 insertions(+), 26 deletions(-)

diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c
index b773b3bdebe3..d07eff7d6750 100644
--- a/arch/x86/kernel/cpu/resctrl/core.c
+++ b/arch/x86/kernel/cpu/resctrl/core.c
@@ -75,7 +75,6 @@ struct rdt_hw_resource rdt_resources_all[] = {
 			.name			= "L3",
 			.cache_level		= 3,
 			.domains		= domain_init(RDT_RESOURCE_L3),
-			.parse_ctrlval		= parse_cbm,
 			.format_str		= "%d=%0*x",
 			.fflags			= RFTYPE_RES_CACHE,
 		},
@@ -89,7 +88,6 @@ struct rdt_hw_resource rdt_resources_all[] = {
 			.name			= "L2",
 			.cache_level		= 2,
 			.domains		= domain_init(RDT_RESOURCE_L2),
-			.parse_ctrlval		= parse_cbm,
 			.format_str		= "%d=%0*x",
 			.fflags			= RFTYPE_RES_CACHE,
 		},
@@ -103,7 +101,6 @@ struct rdt_hw_resource rdt_resources_all[] = {
 			.name			= "MB",
 			.cache_level		= 3,
 			.domains		= domain_init(RDT_RESOURCE_MBA),
-			.parse_ctrlval		= parse_bw,
 			.format_str		= "%d=%*u",
 			.fflags			= RFTYPE_RES_MB,
 		},
@@ -115,7 +112,6 @@ struct rdt_hw_resource rdt_resources_all[] = {
 			.name			= "SMBA",
 			.cache_level		= 3,
 			.domains		= domain_init(RDT_RESOURCE_SMBA),
-			.parse_ctrlval		= parse_bw,
 			.format_str		= "%d=%*u",
 			.fflags			= RFTYPE_RES_MB,
 		},
diff --git a/arch/x86/kernel/cpu/resctrl/ctrlmondata.c b/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
index 788ac0395645..72a651671c68 100644
--- a/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
+++ b/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
@@ -23,6 +23,15 @@
 
 #include "internal.h"
 
+struct rdt_parse_data {
+	struct rdtgroup		*rdtgrp;
+	char			*buf;
+};
+
+typedef int (ctrlval_parser_t)(struct rdt_parse_data *data,
+			       struct resctrl_schema *s,
+			       struct rdt_domain *d);
+
 /*
  * Check whether MBA bandwidth percentage value is correct. The value is
  * checked against the minimum and max bandwidth values specified by the
@@ -59,8 +68,8 @@ static bool bw_validate(char *buf, unsigned long *data, struct rdt_resource *r)
 	return true;
 }
 
-int parse_bw(struct rdt_parse_data *data, struct resctrl_schema *s,
-	     struct rdt_domain *d)
+static int parse_bw(struct rdt_parse_data *data, struct resctrl_schema *s,
+		    struct rdt_domain *d)
 {
 	struct resctrl_staged_config *cfg;
 	u32 closid = data->rdtgrp->closid;
@@ -138,8 +147,8 @@ static bool cbm_validate(char *buf, u32 *data, struct rdt_resource *r)
  * Read one cache bit mask (hex). Check that it is valid for the current
  * resource type.
  */
-int parse_cbm(struct rdt_parse_data *data, struct resctrl_schema *s,
-	      struct rdt_domain *d)
+static int parse_cbm(struct rdt_parse_data *data, struct resctrl_schema *s,
+		     struct rdt_domain *d)
 {
 	struct rdtgroup *rdtgrp = data->rdtgrp;
 	struct resctrl_staged_config *cfg;
@@ -195,6 +204,14 @@ int parse_cbm(struct rdt_parse_data *data, struct resctrl_schema *s,
 	return 0;
 }
 
+static ctrlval_parser_t *get_parser(struct rdt_resource *res)
+{
+	if (res->fflags & RFTYPE_RES_CACHE)
+		return &parse_cbm;
+	else
+		return &parse_bw;
+}
+
 /*
  * For each domain in this resource we expect to find a series of:
  *	id=mask
@@ -204,6 +221,7 @@ int parse_cbm(struct rdt_parse_data *data, struct resctrl_schema *s,
 static int parse_line(char *line, struct resctrl_schema *s,
 		      struct rdtgroup *rdtgrp)
 {
+	ctrlval_parser_t *parse_ctrlval = get_parser(s->res);
 	enum resctrl_conf_type t = s->conf_type;
 	struct resctrl_staged_config *cfg;
 	struct rdt_resource *r = s->res;
@@ -235,7 +253,7 @@ static int parse_line(char *line, struct resctrl_schema *s,
 		if (d->id == dom_id) {
 			data.buf = dom;
 			data.rdtgrp = rdtgrp;
-			if (r->parse_ctrlval(&data, s, d))
+			if (parse_ctrlval(&data, s, d))
 				return -EINVAL;
 			if (rdtgrp->mode ==  RDT_MODE_PSEUDO_LOCKSETUP) {
 				cfg = &d->staged_config[t];
diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
index 65990def6c79..9048bd32e86f 100644
--- a/arch/x86/kernel/cpu/resctrl/internal.h
+++ b/arch/x86/kernel/cpu/resctrl/internal.h
@@ -413,11 +413,6 @@ static inline bool is_mbm_event(int e)
 		e <= QOS_L3_MBM_LOCAL_EVENT_ID);
 }
 
-struct rdt_parse_data {
-	struct rdtgroup		*rdtgrp;
-	char			*buf;
-};
-
 /**
  * struct rdt_hw_resource - arch private attributes of a resctrl resource
  * @r_resctrl:		Attributes of the resource used directly by resctrl.
@@ -455,11 +450,6 @@ static inline struct rdt_hw_resource *resctrl_to_arch_res(struct rdt_resource *r
 	return container_of(r, struct rdt_hw_resource, r_resctrl);
 }
 
-int parse_cbm(struct rdt_parse_data *data, struct resctrl_schema *s,
-	      struct rdt_domain *d);
-int parse_bw(struct rdt_parse_data *data, struct resctrl_schema *s,
-	     struct rdt_domain *d);
-
 extern struct mutex rdtgroup_mutex;
 
 extern struct rdt_hw_resource rdt_resources_all[];
diff --git a/include/linux/resctrl.h b/include/linux/resctrl.h
index 168cc9510069..6e87bc95f5ea 100644
--- a/include/linux/resctrl.h
+++ b/include/linux/resctrl.h
@@ -157,9 +157,6 @@ struct resctrl_membw {
 	u32				*mb_map;
 };
 
-struct rdt_parse_data;
-struct resctrl_schema;
-
 /**
  * struct rdt_resource - attributes of a resctrl resource
  * @rid:		The index of the resource
@@ -174,7 +171,6 @@ struct resctrl_schema;
  * @data_width:		Character width of data when displaying
  * @default_ctrl:	Specifies default cache cbm or memory B/W percent.
  * @format_str:		Per resource format string to show domain value
- * @parse_ctrlval:	Per resource function pointer to parse control values
  * @evt_list:		List of monitoring events
  * @fflags:		flags to choose base and info files
  * @cdp_capable:	Is the CDP feature available on this resource
@@ -192,9 +188,6 @@ struct rdt_resource {
 	int			data_width;
 	u32			default_ctrl;
 	const char		*format_str;
-	int			(*parse_ctrlval)(struct rdt_parse_data *data,
-						 struct resctrl_schema *s,
-						 struct rdt_domain *d);
 	struct list_head	evt_list;
 	unsigned long		fflags;
 	bool			cdp_capable;
-- 
2.34.1


  parent reply	other threads:[~2024-04-26 15:09 UTC|newest]

Thread overview: 50+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-26 15:05 [PATCH v2 00/35] x86/resctrl: Move the resctrl filesystem code to /fs/resctrl Dave Martin
2024-04-26 15:08 ` [PATCH v2 01/35] x86/resctrl: Fix allocation of cleanest CLOSID on platforms with no monitors Dave Martin
2024-04-26 15:08   ` [PATCH v2 02/35] x86/resctrl: Add a helper to avoid reaching into the arch code resource list Dave Martin
2024-04-26 15:08   ` Dave Martin [this message]
2024-04-26 15:08   ` [PATCH v2 04/35] x86/resctrl: Add helper for setting CPU default properties Dave Martin
2024-04-26 15:08   ` [PATCH v2 05/35] x86/resctrl: Remove rdtgroup from update_cpu_closid_rmid() Dave Martin
2024-04-26 15:08   ` [PATCH v2 06/35] x86/resctrl: Export resctrl fs's init function Dave Martin
2024-05-23  0:48     ` Shaopeng Tan (Fujitsu)
2024-04-26 15:08   ` [PATCH v2 07/35] x86/resctrl: Wrap resctrl_arch_find_domain() around rdt_find_domain() Dave Martin
2024-04-26 15:08   ` [PATCH v2 08/35] x86/resctrl: Move resctrl types to a separate header Dave Martin
2024-04-26 15:08   ` [PATCH v2 09/35] x86/resctrl: Add a resctrl helper to reset all the resources Dave Martin
2024-04-26 15:08   ` [PATCH v2 10/35] x86/resctrl: Move monitor init work to a resctrl init call Dave Martin
2024-04-26 15:08   ` [PATCH v2 11/35] x86/resctrl: Move monitor exit work to a resctrl exit call Dave Martin
2024-04-26 15:08   ` [PATCH v2 12/35] x86/resctrl: Move max_{name,data}_width into resctrl code Dave Martin
2024-04-26 15:08   ` [PATCH v2 13/35] x86/resctrl: Stop using the for_each_*_rdt_resource() walkers Dave Martin
2024-04-26 15:08   ` [PATCH v2 14/35] x86/resctrl: Export the is_mbm_*_enabled() helpers to asm/resctrl.h Dave Martin
2024-04-26 15:08   ` [PATCH v2 15/35] x86/resctrl: Add resctrl_arch_is_evt_configurable() to abstract BMEC Dave Martin
2024-04-26 15:08   ` [PATCH v2 16/35] x86/resctrl: Change mon_event_config_{read,write}() to be arch helpers Dave Martin
2024-04-26 15:08   ` [PATCH v2 17/35] x86/resctrl: Move mbm_cfg_mask to struct rdt_resource Dave Martin
2024-06-14 13:57     ` James Morse
2024-04-26 15:08   ` [PATCH v2 18/35] x86/resctrl: Allow resctrl_arch_mon_event_config_write() to return an error Dave Martin
2024-04-26 15:08   ` [PATCH v2 19/35] x86/resctrl: Add resctrl_arch_ prefix to pseudo lock functions Dave Martin
2024-04-26 15:08   ` [PATCH v2 20/35] x86/resctrl: Allow an architecture to disable pseudo lock Dave Martin
2024-04-26 15:08   ` [PATCH v2 21/35] x86/resctrl: Make prefetch_disable_bits belong to the arch code Dave Martin
2024-04-26 15:08   ` [PATCH v2 22/35] x86/resctrl: Make resctrl_arch_pseudo_lock_fn() take a plr Dave Martin
2024-06-14 13:58     ` James Morse
2024-04-26 15:08   ` [PATCH v2 23/35] x86/resctrl: Move thread_throttle_mode_init() to be managed by resctrl Dave Martin
2024-04-26 15:08   ` [PATCH v2 24/35] x86/resctrl: Move get_config_index() to a header Dave Martin
2024-04-26 15:08   ` [PATCH v2 25/35] x86/resctrl: Claim get_domain_from_cpu() for resctrl Dave Martin
2024-04-26 15:08   ` [PATCH v2 26/35] x86/resctrl: Describe resctrl's bitmap size assumptions Dave Martin
2024-04-26 15:08   ` [PATCH v2 27/35] x86/resctrl: Rename resctrl_sched_in() to begin with "resctrl_arch_" Dave Martin
2024-04-26 15:08   ` [PATCH v2 28/35] x86/resctrl: Drop __init/__exit on assorted symbols Dave Martin
2024-04-26 15:08   ` [PATCH v2 29/35] fs/resctrl: Add boiler plate for external resctrl code Dave Martin
2024-06-14 13:59     ` James Morse
2024-04-26 15:08   ` [PATCH v2 30/35] x86/resctrl: Squelch whitespace anomalies in resctrl core code Dave Martin
2024-04-26 15:09   ` [PATCH v2 31/35] x86/resctrl: Prefer alloc(sizeof(*foo)) idiom in rdt_init_fs_context() Dave Martin
2024-04-26 15:09   ` [PATCH v2 32/35] x86/resctrl: Relax some asm #includes Dave Martin
2024-04-26 15:09   ` [PATCH v2 33/35] x86/resctrl: Move the filesystem bits to headers visible to fs/resctrl Dave Martin
2024-04-26 15:09   ` [PATCH v2 34/35] x86/resctrl: Move the resctrl filesystem code to /fs/resctrl Dave Martin
2024-04-26 15:09   ` [PATCH v2 35/35] [SQUASHME] " Dave Martin
2024-04-29 23:34 ` [PATCH v2 00/35] " Peter Newman
2024-04-30 16:29   ` Dave Martin
2024-05-08 15:41 ` Reinette Chatre
2024-05-14 15:17   ` Dave Martin
2024-05-14 15:56     ` Reinette Chatre
2024-05-23 15:01       ` Dave Martin
2024-05-23 15:07         ` David Hildenbrand
2024-05-23 15:18           ` Dave Martin
2024-05-23  0:39 ` Shaopeng Tan (Fujitsu)
2024-06-14 13:59   ` James Morse

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240426150904.8854-3-Dave.Martin@arm.com \
    --to=dave.martin@arm.com \
    --cc=Babu.Moger@amd.com \
    --cc=amitsinght@marvell.com \
    --cc=baolin.wang@linux.alibaba.com \
    --cc=bobo.shaobowang@huawei.com \
    --cc=bp@alien8.de \
    --cc=carl@os.amperecomputing.com \
    --cc=david@redhat.com \
    --cc=dfustini@baylibre.com \
    --cc=fenghua.yu@intel.com \
    --cc=hpa@zytor.com \
    --cc=james.morse@arm.com \
    --cc=lcherian@marvell.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=peternewman@google.com \
    --cc=quic_jiles@quicinc.com \
    --cc=reinette.chatre@intel.com \
    --cc=rex.nie@jaguarmicro.com \
    --cc=scott@os.amperecomputing.com \
    --cc=shameerali.kolothum.thodi@huawei.com \
    --cc=tan.shaopeng@fujitsu.com \
    --cc=tglx@linutronix.de \
    --cc=x86@kernel.org \
    --cc=xhao@linux.alibaba.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox