Re: [PATCH v5 27/29] fs/resctrl: Add file system mechanism for architecture info file

[Reinette Chatre <reinette.chatre@intel.com>]

On 6/9/25 4:34 PM, Luck, Tony wrote:
> Reinette,
> 
> Trimming to focus on why I was confused by your message.
> 
>>> One possibility, that supports intended use while keeping the door open to support
>>> future resctrl fs use of the debugfs, could be  a new resctrl fs function,
>>> for example resctrl_create_mon_resource_debugfs(struct rdt_resource *r), that will initialize
>>> rdt_resource::arch_debug_info(*) to point to the dentry of newly created
>>> /sys/kernel/debug/resctrl/info/<rdt_resource::name>_MON/arch_debug_name_TBD *if*
>>> the associated resource is capable of monitoring 
> 
> What exactly is this dentry pointing to? I was mistakenly of the impression that it was a directory.

Yes, it has been directory since https://lore.kernel.org/lkml/9eb9a466-2895-405a-91f7-cda75e75f7ae@intel.com/
If your impression was indeed that it was a directory then why did your patch not
create a directory?

I am now going to repeat what I said in https://lore.kernel.org/lkml/9eb9a466-2895-405a-91f7-cda75e75f7ae@intel.com/

> 
> Now I think that you intend it to be a single file with a name chosen by filesystem code.
> 
> Is that right?
Not what I have been saying, no.

> 
> If so, there needs to be "umode_t mode" and "struct file_operations *fops" arguments
> for architecture to say whether this file is readable, writeable, and most importantly
> to specify the architecture functions to be called when the user accesses this file.
> 
> With added "mode" and "fops" arguments this proposal meets my needs.
> 
> Choosing the exact string for the "arch_debug_name_TBD" file name that

This should be a directory, a directory owned by the arch where it can create
debug infrastructure required by arch. The directory name chosen and
assigned by resctrl fs, while arch has freedom to create more directories
and add files underneath it. Goal is to isolate all arch specific debug to
a known location.

Again, we need to prepare for resctrl fs to potentially use debugfs for its own
debug and when it does this the expectation is that the layout will mirror
/sys/fs/resctrl. Creating a directory /sys/kernel/debug/resctrl/info/<rdt_resource::name>_MON
and then handing it off to the arch goes *against* this. It gives arch
control over a directory that should be owned by resctrl fs.

What I have been trying to propose is that resctrl fs create a directory
/sys/kernel/debug/resctrl/info/<rdt_resource::name>_MON/arch_debug_name_TBD and hand
a dentry pointer to it to the arch where it can do what is needed to support its debugging needs.
Isn't this exactly what I wrote in the snippet above? Above you respond with
statement that you were under impression that it was a directory ... and then
send a patch that does something else. I am so confused. Gaslighting is
beneath you.

> will be given to any other users needs some thought. I was planning on
> simply "status" since the information that I want to convey is read-only
> status about each of the telemetry collection aggregators. But that feels
> like it might be limiting if a future use includes any control options by
> providing a writable file.

The file containing the debug information for this feature will be added within
the directory that we are talking about here.

Reinette