Re: [PATCH v7 4/8] x86/resctrl: Add comments on RFTYPE flags hierarchy

[Reinette Chatre]

Hi Babu,

On 8/17/2023 10:07 AM, Moger, Babu wrote:
> On 8/17/23 10:37, Reinette Chatre wrote:
>> On 8/17/2023 7:20 AM, Moger, Babu wrote:

>>> + *
>>> + *             --> RFTYPE_MON (Files for all monitoring resources)
>>> + *                 directory: L3_MON
>>
>> Should this not be below RFTYPE_RES_CACHE? 
> 
> This is kind of odd. I know why you are saying it. Wouldn't it confuse the
> user? Then, it feels like mon_features and num_rmids don't belong L3_MON.
> 

This is exactly the confusion that I attempted to highlight in my
first response to this patch. The same issue is present for
"num_closids" (which is currently treated differently ... at least
these need to be consistent). How can it be made obvious that
these files are present in all resource sub-directories while
also capturing the hierarchy of the RFTYPE flags? I could not
find a clear way and that is why I ended up removing the directories
in my earlier proposal and just stick to documenting the file flags
that only applies to files anyway. 

What do you think of something like below? It has duplication
but may be less confusing while still capturing the flags
accurately?

	--> RFTYPE_MON (Files for all monitoring resources)
	    Directory: L3_MON
	    Files: mon_features, num_rmids

	    --> RFTYPE_RES_CACHE (Files for cache monitoring resources)
		Directory: L3_MON
		Files: max_threshold_occupancy, mbm_total_bytes_config,
		       mbm_local_bytes_config

	--> RFTYPE_CTRL (Files for all control resources)
	    Directories: L2, L3, MB, SMBA
	    File: num_closids

	    --> RFTYPE_CTRL (Files for all control resources)
		Directories: L2, L3
		Files: bit_usage, cbm_mask, min_cbm_bits,
		       shareable_bits
	
	    --> RFTYPE_RES_MB (Files for memory control resources)
		Directories: MB, SMBA
		Files: bandwidth_gran, delay_linear,
		       min_bandwidth, thread_throttle_mode

	
>>> + *                               Files: bit_usage, cbm_mask, min_cbm_bits,
>>> + *                                      shareable_bits
>>
>> The extra indent is not clear to me. Did you do it to represent
>> a hierarchy or just to line up the ":"?
> 
> This is to line up with :. I can fix it.
> 
> Just wondering how do you notice these tabs? My linux diff does not show
> any difference. Are you using any utilities to see these tabs?

My editor is configured to visualize tabs and trailing spaces. In
this case it was just how my email client displayed it though.

Reinette