Re: [PATCH v3 4/7] x86/resctrl: Re-arrange RFTYPE flags based on hierarchy

[Reinette Chatre]

Hi Babu,

On 3/21/2023 8:54 AM, Moger, Babu wrote:
> Hi Reinette,
> To be honest, I had tough time understanding these flags. Also, I need to
> add more files in the future. So, I am trying make these these things
> clear before I do those changes.
> 
> These flags decoding is pretty confusing. Also, there are some flags which
> are duplicate. Not really required.
> 
> For example:
>   In group structure, we have control group or mon group. We just need two
> bits here. The code uses combination of 3 flags here.
> #define RFTYPE_BASE                     BIT(1)
> #define RFTYPE_CTRL                     BIT(4)
> #define RFTYPE_MON                      BIT(5)

Two bits are used to distinguish between files being for control or
monitoring groups respectively.

The third bit you list is used to indicate where the file is located. The
RFTYPE_BASE is the bit used to specify that the file is located within
a resource group, as opposed to RFTYPE_INFO that specifies that the file
is located within the info directory.


> Also, the flag RFTYPE_MON again used in creation on info directory.
> Basically, very confusing to add anything new.

The RFTYPE_MON indicates a monitoring file. Whether it is in the info
directory or as part of a monitoring group is specified using RFTYPE_INFO
or RFTYPE_BASE respectively.

A file can be specific to a monitoring or control group by setting
the RFTYPE_CTRL or RFTYPE_MON bits, but if it does not then control and
monitoring groups will have the file. For example, the "tasks" and "cpus"
files.
 
> I will try to minimize the changes in the next version but still make it
> clear.

ok

> 
> 
> On 3/15/23 13:37, Reinette Chatre wrote:
>> Hi Babu,
>>
>> On 3/2/2023 12:24 PM, Babu Moger wrote:
>>> RESCTRL filesystem has two main components:
>>> a. info (Details on resources and monitoring)
>>> b. base (Details on CONTROL and MON groups)
>>>
>>> The rftype flags can be renamed accordingly for better understanding.
>>> For example:
>>> RFTYPE_INFO     : Files with these flags go in info directory
>>
>> This is not a rename but the current name.
> 
> Agree. I am giving some example here. I may not need to change the text here.
>>
>>> RFTYPE_INFO_MON : Files with these flags go in info/L3_MON
>>
>> How does this improve the current RFTYPE_MON_INFO?
> 
> RFTYPE_INFO_MON -> info/L3_MON.
> 
> I tried to improve some readability based on hierarchy. Basically, looking
> at the flags we know exaclty where these files land.

It is not clear to me how switching around terms in the flag
accomplishes this. The meaning ends up the same.

>>> @@ -3218,7 +3218,7 @@ static int mkdir_rdt_prepare(struct kernfs_node *parent_kn,
>>>  	if (rtype == RDTCTRL_GROUP)
>>>  		fflags = RFTYPE_BASE | RFTYPE_CTRL;
>>>  	else
>>> -		fflags = RFTYPE_BASE | RFTYPE_MON;
>>> +		fflags = RFTYPE_BASE;
>>>  
>>
>> Is this intended?
> 
> Yes. We don't need this extra flag (RFTYPE_MON) here.

It is not used, but it reflects the resctrl design in support
of adding monitoring files. Future additions of files need not think
how such integration needs to happen since it has been solved and
is supported.

Reinette