Re: [PATCH v1 03/31] x86/resctrl: Move ctrlval string parsing policy away from the arch code

Reinette Chatre
2024-04-09T03:14:53Z

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@arm.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