linux-kernel - Re: [PATCH v1 01/14] fs/resctrl: group the resource control types for schemata list

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CALPaoChguTrJPtgtxXkNd2sJdFLajWq-CcKeCd__pfuUye+43w@mail.gmail.com>
Date: Wed, 17 Jan 2024 11:21:22 -0800
From: Peter Newman <peternewman@...gle.com>
To: Amit Singh Tomar <amitsinght@...vell.com>
Cc: linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org, 
	fenghua.yu@...el.com, reinette.chatre@...el.com, james.morse@....com, 
	gcherian@...vell.com, robh@...nel.org, dfustini@...libre.com, 
	jonathan.cameron@...wei.com
Subject: Re: [PATCH v1 01/14] fs/resctrl: group the resource control types for
 schemata list

Hi Amit,

On Wed, Jan 17, 2024 at 6:14 AM Amit Singh Tomar <amitsinght@...vell.com> wrote:
> diff --git a/include/linux/resctrl.h b/include/linux/resctrl.h
> index 3ad308e9e226..125c4b0c2ff7 100644
> --- a/include/linux/resctrl.h
> +++ b/include/linux/resctrl.h
> @@ -249,6 +249,7 @@ struct resctrl_schema {
>         struct list_head                list;
>         char                            name[8];
>         enum resctrl_conf_type          conf_type;
> +       enum resctrl_ctrl_type          ctrl_type;

I don't see a difference between a conf_type and a ctrl_type other
than conf_type being used for CDP and ctrl_type being used for DSPRI.

>         struct rdt_resource             *res;
>         u32                             num_closid;
>  };
> diff --git a/include/linux/resctrl_types.h b/include/linux/resctrl_types.h
> index 3897de9c4ecb..b9268ec3ba71 100644
> --- a/include/linux/resctrl_types.h
> +++ b/include/linux/resctrl_types.h
> @@ -57,6 +57,12 @@ enum resctrl_res_level {
>         RDT_NUM_RESOURCES,
>  };
>
> +enum resctrl_ctrl_type {
> +       SCHEMA_BASIC = 0,
> +       SCHEMA_DSPRI,
> +       SCHEMA_NUM_CTRL_TYPE
> +};

Rather than enumerate every control type on all implementations, I
would prefer the approach taken in the MPAM resctrl_arch_rmid_read(),
where the MPAM driver would embed the resctrl_schema into a private
structure it defines and use container_of() in
resctrl_arch_update_domains(), which should take a resctrl_schema
parameter instead of an rdt_resource. Like resctrl_arch_rmid_read(),
the containing structure would direct the values to the appropriate
hardware.

I would like to see the MPAM driver owning the enumeration of this
feature and the common FS code just understanding the control as a
value that is passed back to the driver if its value is within a
stated acceptable range.

I think there's still a lot of work that needs to be done in the
overall design of resctrl to provide general support for controls and
monitors which do not exist in RDT and these patches provide useful
information to help us understand the issues.

Thanks!
-Peter