Hi Babu,
On 7/10/25 10:16 AM, Babu Moger wrote:
> The functions parse_cbm() and parse_bw() require mode and CLOSID to
No need to say "function" when using (). Also, drop parse_bw(), since it
does not validate CBMs.
> validate the Capacity Bit Mask (CBM). It is passed through struct
"passed through" -> "passed via"?
> rdtgroup in rdt_parse_data.
"rdt_parse_data" -> "struct rdt_parse_data"
>
> This can be simplified by passing the mode and closid directly, instead of
closid -> CLOSID
> through the rdtgroup struct. Doing so also facilitates calling parse_cbm()
> to verify the CBM within the io_alloc feature, since io_alloc does not
> have rdtgroup context.
Above notes that "simplification" is the primary motivation but I do not think
this change qualifies as a "simplification". How about second paragraph changed
to something like:
The io_alloc feature also uses CBMs to indicate which portions of
cache are allocated for I/O traffic. The CBMs are provided by
user space and need to be validated the same as CBMs provided for
general (CPU) cache allocation. parse_cbm() cannot be used as-is
since io_alloc does not have rdtgroup context.
Pass the mode and CLOSID directly to parse_cbm() via struct rdt_parse_data
instead of through the rdtgroup struct to facilitate calling parse_cbm() to
verify the CBM of the io_alloc feature.
(please feel free to improve)
>
> Signed-off-by: Babu Moger <babu.moger@xxxxxxx>
> ---
> ---
> fs/resctrl/ctrlmondata.c | 29 +++++++++++++----------------
> fs/resctrl/internal.h | 6 ++++++
> 2 files changed, 19 insertions(+), 16 deletions(-)
>
> diff --git a/fs/resctrl/ctrlmondata.c b/fs/resctrl/ctrlmondata.c
> index e78828b0408a..5c16557fb7a8 100644
> --- a/fs/resctrl/ctrlmondata.c
> +++ b/fs/resctrl/ctrlmondata.c
> @@ -23,11 +23,6 @@
>
> #include "internal.h"
>
> -struct rdt_parse_data {
> - struct rdtgroup *rdtgrp;
> - char *buf;
> -};
> -
This patch is only about replacing rdtgroup with mode and CLOSID, there is no
motivation for relocating the structure declaration. This looks to be a change
needed by following patch but is another change that becomes unnecessary if
the io_alloc code, specifically resctrl_io_alloc_cbm_write() and
resctrl_io_alloc_parse_line() from next patch, are moved to ctrlmondata.c.
...
> @@ -171,7 +167,7 @@ static int parse_cbm(struct rdt_parse_data *data, struct resctrl_schema *s,
> * Cannot set up more than one pseudo-locked region in a cache
> * hierarchy.
> */
> - if (rdtgrp->mode == RDT_MODE_PSEUDO_LOCKSETUP &&
> + if (mode == RDT_MODE_PSEUDO_LOCKSETUP &&
> rdtgroup_pseudo_locked_in_hierarchy(d)) {
> rdt_last_cmd_puts("Pseudo-locked region in hierarchy\n");
> return -EINVAL;
> @@ -180,9 +176,9 @@ static int parse_cbm(struct rdt_parse_data *data, struct resctrl_schema *s,
> if (!cbm_validate(data->buf, &cbm_val, r))
> return -EINVAL;
>
> - if ((rdtgrp->mode == RDT_MODE_EXCLUSIVE ||
> - rdtgrp->mode == RDT_MODE_SHAREABLE) &&
> - rdtgroup_cbm_overlaps_pseudo_locked(d, cbm_val)) {
> + if ((mode == RDT_MODE_EXCLUSIVE ||
> + mode == RDT_MODE_SHAREABLE) &&
> + rdtgroup_cbm_overlaps_pseudo_locked(d, cbm_val)) {
Please fix alignment.
> rdt_last_cmd_puts("CBM overlaps with pseudo-locked region\n");
> return -EINVAL;
> }
Reinette
