Re: [PATCH 05/11] fs/resctrl: Use accurate type for rdt_resource::rid

[Ben Horgan]

Hi Reinette,

On 3/2/26 18:46, Reinette Chatre wrote:
> Every resctrl resource has a unique ID described by enum resctrl_res_level.
> enum resctrl_res_level is used in all resource ID initializations and all
> resource ID comparisons. All functions consuming the resource ID expects an
> enum resctrl_res_level. Of the four structures that contain a resource ID
> (struct mon_data, struct mon_evt, struct rdt_domain_hdr, and struct
>  rdt_resource) only struct rdt_resource does not use enum resctrl_res_level.
> 
> Switch the type of rdt_resource::rid to be enum resctrl_res_level to make
> it obvious what values are valid, match the type everywhere this member is
> used, and obtain benefits from tools that can flag any enum misuse.
> 
> Move define of RDT_NUM_RESOURCES outside the enum to enable tools to catch
> when a switch() on the resource ID does not handle all the resources and
> thus help flag which switch statements need an update when a new resource
> is added.
> 
> Signed-off-by: Reinette Chatre <reinette.chatre@xxxxxxxxx>
> ---
>  include/linux/resctrl.h | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/include/linux/resctrl.h b/include/linux/resctrl.h
> index 006e57fd7ca5..48e95f273fb3 100644
> --- a/include/linux/resctrl.h
> +++ b/include/linux/resctrl.h
> @@ -54,11 +54,11 @@ enum resctrl_res_level {
>  	RDT_RESOURCE_MBA,
>  	RDT_RESOURCE_SMBA,
>  	RDT_RESOURCE_PERF_PKG,
> -
> -	/* Must be the last */
> -	RDT_NUM_RESOURCES,
> +	/* Additions to enum need to update RDT_NUM_RESOURCES. */
>  };>
> +#define RDT_NUM_RESOURCES (RDT_RESOURCE_PERF_PKG + 1)

Would it be clearer to have RDT_RESOURCE_LAST = RDT_RESOURCE_PERF_PKG in the enum
and use RDT_RESOURCE_LAST instead of RDT_RESOURCE_PERF_PKG here?

Thanks,

Ben

> +
>  /**
>   * enum resctrl_conf_type - The type of configuration.
>   * @CDP_NONE:	No prioritisation, both code and data are controlled or monitored.
> @@ -319,7 +319,7 @@ struct resctrl_mon {
>   * @cdp_capable:	Is the CDP feature available on this resource
>   */
>  struct rdt_resource {
> -	int			rid;
> +	enum resctrl_res_level	rid;
>  	bool			alloc_capable;
>  	bool			mon_capable;
>  	enum resctrl_scope	ctrl_scope;