Received-SPF: pass (google.com: domain of linux-kernel+bounces-29360-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Precedence: bulk
MIME-Version: 1.0
References: <20240117141405.3063506-1-amitsinght@marvell.com> <20240117141405.3063506-2-amitsinght@marvell.com>
In-Reply-To: <20240117141405.3063506-2-amitsinght@marvell.com>
From: Peter Newman <peternewman@google.com>
Date: Wed, 17 Jan 2024 11:21:22 -0800
Message-ID: <CALPaoChguTrJPtgtxXkNd2sJdFLajWq-CcKeCd__pfuUye+43w@mail.gmail.com>
Subject: Re: [PATCH v1 01/14] fs/resctrl: group the resource control types for
 schemata list
To: Amit Singh Tomar <amitsinght@marvell.com>
Cc: linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, 
	fenghua.yu@intel.com, reinette.chatre@intel.com, james.morse@arm.com, 
	gcherian@marvell.com, robh@kernel.org, dfustini@baylibre.com, 
	jonathan.cameron@huawei.com
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

Hi Amit,

On Wed, Jan 17, 2024 at 6:14=E2=80=AFAM Amit Singh Tomar <amitsinght@marvel=
l.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 =3D 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