[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <e85e7786-7995-42d5-a5ac-1e08a84492fe@intel.com>
Date: Mon, 8 Apr 2024 20:14:47 -0700
From: Reinette Chatre <reinette.chatre@...el.com>
To: James Morse <james.morse@....com>, <x86@...nel.org>,
<linux-kernel@...r.kernel.org>
CC: Fenghua Yu <fenghua.yu@...el.com>, Thomas Gleixner <tglx@...utronix.de>,
Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>, H Peter Anvin
<hpa@...or.com>, Babu Moger <Babu.Moger@....com>,
<shameerali.kolothum.thodi@...wei.com>, D Scott Phillips OS
<scott@...amperecomputing.com>, <carl@...amperecomputing.com>,
<lcherian@...vell.com>, <bobo.shaobowang@...wei.com>,
<tan.shaopeng@...itsu.com>, <baolin.wang@...ux.alibaba.com>, Jamie Iles
<quic_jiles@...cinc.com>, Xin Hao <xhao@...ux.alibaba.com>,
<peternewman@...gle.com>, <dfustini@...libre.com>, <amitsinght@...vell.com>,
David Hildenbrand <david@...hat.com>, Rex Nie <rex.nie@...uarmicro.com>,
"Dave Martin" <dave.martin@....com>
Subject: Re: [PATCH v1 03/31] x86/resctrl: Move ctrlval string parsing policy
away from the arch code
Hi James,
On 3/21/2024 9:50 AM, James Morse wrote:
> 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 flesystem parts of resctrl. This should prevent any
flesystem -> filesystem
> 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@....com>
> ---
..
> @@ -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;
> +}
This is borderline ... at minimum it expands what fflags means and how it
is intended to be used and that needs to be documented because it reads:
* @fflags: flags to choose base and info files
I am curious why you picked fflags instead of an explicit check against
rid?
Reinette
Powered by blists - more mailing lists