Re: [PATCH v9 8/9] x86/resctrl: Display CLOSID for resource group

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Fenghua,

On 9/11/2023 11:08 AM, Fenghua Yu wrote:
> On 9/7/23 16:51, Babu Moger wrote:

...
>> @@ -1863,6 +1879,13 @@ static struct rftype res_common_files[] = {
>>           .seq_show    = rdtgroup_size_show,
>>           .fflags        = RFTYPE_CTRL_BASE,
>>       },
>> +    {
>> +        .name        = "ctrl_hw_id",
>> +        .mode        = 0444,
>> +        .kf_ops        = &rdtgroup_kf_single_ops,
>> +        .seq_show    = rdtgroup_closid_show,
> 
> Is it better to rename "rdtgroup_closid_show" as arch neutral name
> "rdtgroup_ctrl_hw_id_show"? So the name is arch neutral and reflects
> the ctrl_hw_id. So this can eventually go to generic fs code without
> renaming it. The getting closid implementation in the function will
> be arch specific.

This is not so obvious to me. We have to draw the line somewhere. This
series draws the line between the kernel and user interface. That is,
from user perspective it is "ctrl_hw_id" and from kernel perspective
it is "closid".

I think renaming rdtgroup_closid_show() to rdtgroup_ctrl_hw_id_show() may
be mixing up the two because the function called rdtgroup_ctrl_hw_id_show() 
would operate on rdtgroup->closid - so in a sense renaming the function
to rdtgroup_ctrl_hw_id_show() would move the name further from what the
function does. Would that mean that we need to refactor the whole kernel
the naming to match between kernel space and user space by changing
rdtgroup->closid to rdtgroup->ctrl_hw_id? That escalates quite far and
wide and I wonder if it is not just simpler to keep the naming
boundary between kernel space and user space.

Reinette



[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux FS]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux