Re: [PATCH v5 1/2] fs/resctrl: Report invalid domain ID when parsing io_alloc

[Reinette Chatre]

Hi Aaron,

typo in subject: "io_alloc" -> "io_alloc_cbm"

On 2/10/26 1:07 PM, Aaron Tomlin wrote:
> The last_cmd_status file is intended to report details about the most
> recent resctrl filesystem operation, specifically to aid in diagnosing
> failures.
> 
> However, when parsing io_alloc_cbm, if a user provides a domain ID that
> does not exist in the resource, the operation fails with -EINVAL without
> updating last_cmd_status. This results in inconsistent behaviour where
> the system call reports an error, but the status file may misleadingly

"reports" -> "returns"

> report "ok" (retained from a previous successful operation), leaving the

This can be made specific:
"the status file may misleadingly report "ok"" -> "last_cmd_status
misleadingly reports"

Drop "(retained from a previous successful operation)" since it is not
accurate. Note that resctrl_io_alloc_cbm_write() starts with rdt_last_cmd_clear()
that clears the last_cmd_status buffer and rdt_last_cmd_status_show()
always prints "ok" when the buffer is empty.

> user unaware that the failure was caused by an invalid domain ID.
> 
> Update resctrl_io_alloc_parse_line() to explicitly write an error
> message to last_cmd_status when the target domain ID cannot be found.

Drop "Update resctrl_io_alloc_parse_line() to explicitly" that can be seen from
the patch.

> 
> Suggested-by: Reinette Chatre <reinette.chatre@xxxxxxxxx>
> Fixes: 28fa2cce7a83 ("fs/resctrl: Introduce interface to modify io_alloc capacity bitmasks")

Please place the "Fixes:" tag first. For reference, "Ordering of commit tags"
in Documentation/process/maintainer-tip.rst.

> Signed-off-by: Aaron Tomlin <atomlin@xxxxxxxxxxx>
> ---
>  fs/resctrl/ctrlmondata.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/fs/resctrl/ctrlmondata.c b/fs/resctrl/ctrlmondata.c
> index b2d178d3556e..b96b661626c2 100644
> --- a/fs/resctrl/ctrlmondata.c
> +++ b/fs/resctrl/ctrlmondata.c
> @@ -907,6 +907,7 @@ static int resctrl_io_alloc_parse_line(char *line,  struct rdt_resource *r,
>  		}
>  	}
>  
> +	rdt_last_cmd_printf("Invalid domain %lu\n", dom_id);
>  	return -EINVAL;
>  }
>  

Thank you very much for adding the fix. The actual change looks good to me,
just a couple of nits on the changelog.

Reinette