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

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

Hi Tony,

On 6/5/25 5:09 PM, Luck, Tony wrote:
> On Tue, Jun 03, 2025 at 09:15:02PM -0700, Reinette Chatre wrote:
>> Hi Tony,
>>
>> On 5/21/25 3:50 PM, Tony Luck wrote:
>>> Creation of all files in the resctrl file system is under control of
>>> the file system layer.
>>>
>>> But some resources may need to add a file to the info/{resource}
>>> directory for debug purposes.
>>>
>>> Add a new rdt_resource::info_file field for the resource to specify
>>> show() and/or write() operations. These will be called with the
>>> rdtgroup_mutex held.
>>>
>>> Architecture can note the file is only for debug using by setting
>>> the rftype::flags RFTYPE_DEBUG bit.
>>
>> This needs to change. This punches a crater through the separation
>> between fs and arch that we worked hard to achieve. Please make an attempt
>> to do so as I am sure you can.
> 
> The file I want to create here amy only be of interest in debugging
> the telemetry h/w interface. So my next choice is debugfs.

I believe we can make either work but debugfs does indeed sound most
appropriate. I am not familiar with customs surrounding kernel support for
hardware debug interfaces though, so setting that aside for now.

> 
> But creation of the debugfs "resctrl" directory is done by file
> system code and the debugfs_resctrl variable is only marked "extern" by
> fs/resctrl/internal.h, so currently not accessible to architecture code.

It would be best to stay this way.

> 
> Is that a deliberate choice? Would it be OK to make that visible to
> architecture code to create files in /sys/kernel/debug/resctrl?

It should not be necessary to give an arch total control over
resctrl fs's debugfs.

> 
> Or should I add my file in a new /sys/kernel/debug/x86/resctrl
> directory?

Since there is already a resctrl debugfs I think it will be less confusing
to keep all resctrl debug together within /sys/kernel/debug/resctrl.
There should be some bounds on what an arch can do here though.
There is already support for debugfs via the pseudo-locking
work where /sys/kernel/debug/resctrl contains directories with name
of resource group to provide debug for specific resource group. Giving
an arch total freedom on what can be created in /sys/kernel/debug/resctrl
thus runs the risk of exposing to corner cases where name of arch
debug cannot match resource group names and with ordering of these
directory creations it can become tricky.

With /sys/kernel/debug/resctrl potentially mirroring /sys/fs/resctrl to
support various debugging scenarios there may later be resource level
debugging for which a "/sys/kernel/debug/resctrl/info/<resource>/<debugfile>" can
be used. Considering this it looks to me as though one possible boundary could
be to isolate arch specific debug to, for example, a new directory named
"/sys/kernel/debug/resctrl/info/arch_debug_name_tbd/". By placing the
arch debug in a sub-directory named "info" it avoids collision with resource
group names with naming that also avoids collision with resource names since
all these names are controlled by resctrl fs. 

To support this an architecture can request resctrl fs to create such directory
after resctrl_init() succeeds. Since it is custom to ignore errors for
debugfs dir creation the call is not expected to interfere with initialization
and existing arch initialization should not be impacted with this call
being after resctrl__init().

For example, for x86 it could be something like:

	struct dentry *arch_priv_debug_fs_dir;

	resctrl_arch_late_init(void) {

		...
		ret = resctrl_init();
		if (ret) {
			cpuhp_remove_state(state);
			return ret;
		}
		rdt_online = state;

		arch_priv_debug_fs_dir = resctrl_create_arch_debugfs(); /* all names up for improvement */
		...

	}

Note that the architecture does not pick the directory name.
On the resctrl fs side, resctrl_create_arch_debugfs() creates
"/sys/kernel/debug/resctrl/info/arch_debug_name_tbd", passing the 
dentry back to the arch and with this gives the arch flexibility to create
new directories and files to match its debug requirements.
The arch has flexibility to manage the lifetimes of files/directories of
its debugfs area while resctrl fs still has the final control 
with the debugfs_remove_recursive() of the top level /sys/kernel/debug/resctrl
on its exit.

What do you think?

It will need more work to support an arch-specific debug that is
connected to a resource or resource group, but this does not seem to be the
goal here. Even so, with a design like this it keeps the door open for
future resctrl fs and/or arch debug associated with resources and
resource groups.

Reinette