[PATCH v2 2/3] fs/resctrl: Return -EINVAL for a missing seq_show implementation

Aaron Tomlin posted 3 patches 1 month, 3 weeks ago
[PATCH v2 2/3] fs/resctrl: Return -EINVAL for a missing seq_show implementation
Posted by Aaron Tomlin 1 month, 3 weeks ago
The rdtgroup_seqfile_show() function, which is the sequence file handler
for reading data from resctrl files, previously returned 0 (success) if
the file's associated rftype did not define a .seq_show implementation.

This behavior is incorrect and confusing, as a read operation that
does not define a display function should be treated as an error.

This patch change the function to return -EINVAL if the file type
handler (i.e., rft->seq_show) is NULL, providing proper feedback that
the operation is invalid for that file.

Signed-off-by: Aaron Tomlin <atomlin@atomlin.com>
---
 fs/resctrl/rdtgroup.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/resctrl/rdtgroup.c b/fs/resctrl/rdtgroup.c
index 8e39dfda56bc..e1dc63b2218f 100644
--- a/fs/resctrl/rdtgroup.c
+++ b/fs/resctrl/rdtgroup.c
@@ -314,7 +314,8 @@ static int rdtgroup_seqfile_show(struct seq_file *m, void *arg)
 
 	if (rft->seq_show)
 		return rft->seq_show(of, m, arg);
-	return 0;
+
+	return -EINVAL;
 }
 
 static ssize_t rdtgroup_file_write(struct kernfs_open_file *of, char *buf,
-- 
2.51.0
Re: [PATCH v2 2/3] fs/resctrl: Return -EINVAL for a missing seq_show implementation
Posted by Reinette Chatre 1 month, 3 weeks ago
Hi Aaron,

How is this change required to support the feature of enabling user to
set CBM across domains?

On 12/15/25 3:02 PM, Aaron Tomlin wrote:
> The rdtgroup_seqfile_show() function, which is the sequence file handler
> for reading data from resctrl files, previously returned 0 (success) if
> the file's associated rftype did not define a .seq_show implementation.
> 
> This behavior is incorrect and confusing, as a read operation that
> does not define a display function should be treated as an error.

Why should it be treated as an error? Not having rftype::seq_show() set when
user is intended to be able to see data when reading from the file is a kernel
bug. Otherwise it seems fine to return nothing when there is nothing to show
and doing so be successful.

Reinette
Re: [PATCH v2 2/3] fs/resctrl: Return -EINVAL for a missing seq_show implementation
Posted by Aaron Tomlin 1 month, 3 weeks ago
On Tue, Dec 16, 2025 at 09:02:27PM -0800, Reinette Chatre wrote:
> Hi Aaron,

Hi Reinette,

Thank you for your enquiry regarding the necessity of this specific change.

> How is this change required to support the feature of enabling user to
> set CBM across domains?

To be quite candid, this particular patch does not constitute a strict
prerequisite for the core functionality. The primary feature can, in
technical terms, operate without it.

However, it is important to note that this change was proposed as part of
the broader patchset.

> On 12/15/25 3:02 PM, Aaron Tomlin wrote:
> > The rdtgroup_seqfile_show() function, which is the sequence file handler
> > for reading data from resctrl files, previously returned 0 (success) if
> > the file's associated rftype did not define a .seq_show implementation.
> > 
> > This behavior is incorrect and confusing, as a read operation that
> > does not define a display function should be treated as an error.
> 
> Why should it be treated as an error? Not having rftype::seq_show() set when
> user is intended to be able to see data when reading from the file is a kernel
> bug. Otherwise it seems fine to return nothing when there is nothing to show
> and doing so be successful.

To provide some context on my initial reasoning, I found it somewhat
unorthodox to attempt to access an interface that effectively does not
exist (in this case, a missing seq_show callback) without the kernel
returning an explicit error, such as -EINVAL. My instinct was to signal
that the operation was invalid rather than allowing a silent, successful
"empty" read.

However, I take your point entirely. If the intention is that a lack of
data should simply result in a successful return with no output, then the
current behaviour is indeed acceptable.

I shall revert this change from the series.


Kind rgards,
-- 
Aaron Tomlin