Hi Reinette, On 3/13/25 11:08, Reinette Chatre wrote: > Hi Babu, > > On 3/12/25 11:14 AM, Moger, Babu wrote: >> Hi Reinette, >> >> On 3/12/25 12:14, Reinette Chatre wrote: >>> Hi Babu, >>> >>> On 3/12/25 9:03 AM, Moger, Babu wrote: >>>> Hi Reinette, >>>> >>>> On 3/12/25 10:07, Reinette Chatre wrote: >>>>> Hi Babu, >>>>> .. >>>>>> We can add the mkdir support later. That way we can provide basic ABMC >>>>>> support without too much code complexity with mkdir support. >>>>> >>>>> This is not clear to me how you envision the "first phase". Is it what you >>>>> proposed above, for example: >>>>> #echo "LclFill, LclNTWr, RmtFill" > >>>>> /sys/fs/resctrl/info/L3_MON/counter_configs/mbm_local_bytes >>>>> >>>>> In above the counter configuration name is a file. >>>> >>>> Yes. That is correct. >>>> >>>> There will be two configuration files by default when resctrl is mounted >>>> when ABMC is enabled. >>>> /sys/fs/resctrl/info/L3_MON/counter_configs/mbm_total_bytes >>>> /sys/fs/resctrl/info/L3_MON/counter_configs/mbm_local_bytes >>>> >>>>> >>>>> How could mkdir support be added to this later if there are already files present? >>>> >>>> We already have these directories when resctrl is mounted. >>>> /sys/fs/resctrl/test/mon_data/mon_L3_00/mbm_total_bytes >>>> /sys/fs/resctrl/test/mon_data/mon_L3_00/mbm_local_bytes >>>> /sys/fs/resctrl/test/mon_data/mon_L3_01/mbm_total_bytes >>>> /sys/fs/resctrl/test/mon_data/mon_L3_01/mbm_local_bytes >>>> >>>> We dont need "mkdir" support for default configurations. >>> >>> I was referring to the "mkdir" support for additional configurations that >>> I understood you are thinking about adding later. For example, >>> (copied from Peter's message >>> https://lore.kernel.org/lkml/CALPaoCiii0vXOF06mfV=kVLBzhfNo0SFqt4kQGwGSGVUqvr2Dg@xxxxxxxxxxxxxx/): >>> >>> >>> # mkdir info/L3_MON/counter_configs/mbm_local_bytes >>> # echo LclFill > info/L3_MON/counter_configs/mbm_local_bytes/event_filter >>> # echo LclNTWr > info/L3_MON/counter_configs/mbm_local_bytes/event_filter >>> # echo LclSlowFill > info/L3_MON/counter_configs/mbm_local_bytes/event_filter >>> # cat info/L3_MON/counter_configs/mbm_local_bytes/event_filter >>> LclFill >>> LclNTWr >>> LclSlowFill >>> >>> Any "later" work needs to be backward compatible with the first phase. >> >> Actually, we dont need extra file "event_filter". >> This was discussed here. >> https://lore.kernel.org/lkml/CALPaoChLL8p49eANYgQ0dJiFs7G=223fGae+LJyx3DwEhNeR8A@xxxxxxxxxxxxxx/ > > I undestand from that exchange that it is possible to read/write from > an *existing* kernfs file but it is not obvious to me how that file is > planned to be created. My bad.. I misspoke here. We need "event_filter" file under each configuration. > > My understanding of the motivation behind support for "mkdir" is to enable > user space to create custom counter configurations. > That is correct. > I understand that ABMC support aims to start with existing mbm_total_bytes/mbm_local_bytes > configurations but I believe the consensus is that custom configurations need > to be supported in the future. > If resctrl starts with support where counter configuration as > managed with a *file*, for example: > /sys/fs/resctrl/info/L3_MON/counter_configs/mbm_total_bytes > how will user space create future custom configurations? > As I understand that is only possible with mkdir. > >> >> # echo LclFill > info/L3_MON/counter_configs/mbm_local_bytes >> # echo LclNTWr > info/L3_MON/counter_configs/mbm_local_bytes >> # echo LclSlowFill > info/L3_MON/counter_configs/mbm_local_bytes >> # cat info/L3_MON/counter_configs/mbm_local_bytes >> LclFill >> LclNTWr >> LclSlowFill >> >> In the future, we can add mkdir support. >> >> # mkdir info/L3_MON/counter_configs/mbm_read_only > > This is exactly my concern. resctrl should not start with a user space where > a counter configuration is a file (mbm_local_bytes/mbm_total_bytes) and then > switch user space interface to have counter configuration be done with > directories. > >> # echo LclFill > info/L3_MON/counter_configs/mbm_read_only >> # cat info/L3_MON/counter_configs/mbm_read_only >> LclFill > > ... wait ... user space writes to the directory? > My bad. This is wrong. Let me rewrite the steps below. > > >> >> #echo mbm_read_only > test/mon_data/mon_L3_00/assign_exclusive >> >> Which would result in the creation of test/mon_data/mon_L3_*/mbm_read_only >> >> So, there is not breakage of backword compatibility. > > The way I understand it I am seeing many incompatibilities. Perhaps I am missing > something. Could you please provide detailed steps of how first phase and > second phase would look? No. You didn't miss anything. I misspoke on few steps. Here are the steps. Just copying steps from Peters proposal. https://lore.kernel.org/lkml/CALPaoCiii0vXOF06mfV=kVLBzhfNo0SFqt4kQGwGSGVUqvr2Dg@xxxxxxxxxxxxxx/ 1. Mount the resctrl mount -t resctrl resctrl /sys/fs/resctrl 2. When ABMC is supported two default configurations will be created. a. info/L3_MON/counter_configs/mbm_total_bytes/event_filter b. info/L3_MON/counter_configs/mbm_local_bytes/event_filter These files will be populated with default total and local events # cat info/L3_MON/counter_configs/mbm_total_bytes/event_filter VictimBW RmtSlowFill RmtNTWr RmtFill LclFill LclNTWr LclSlowFill # cat info/L3_MON/counter_configs/mbm_local_bytes/event_filter LclFill, LclNTWr LclSlowFill 3. Users will have options to update the event configuration. echo LclFill > info/L3_MON/counter_configs/mbm_local_bytes/event_filter 4. As usual the events can be read from the mon_data directories. #mkdir /sys/fs/resctrl/test #cd /sys/fs/resctr/test #cat test/mon_data/mon_data/mon_L3_00/mbm_tota_bytes 101010 #cat test/mon_data/mon_data/mon_L3_00/mbm_local_bytes 32323 5. There will be 3 files created in each group's mon_data directory when ABMC is supported. a. test/mon_data/mon_L3_00/assign_exclusive b. test/mon_data/mon_L3_00/assign_shared c. test/mon_data/mon_L3_00/unassign 6. Events can be assigned/unassigned by these commands # echo mbm_total_bytes > test/mon_data/mon_L3_00/assign_exclusive # echo mbm_local_bytes > test/mon_data/mon_L3_01/assign_exclusive # echo mbm_local_bytes > test/mon_data/mon_L3_01/unassign Note: I feel 3 files are excessive here. We can probably achieve everything in just one file. Not sure about mbm_assign_control interface as there are concerns with group listing holding the lock for long. ----------------------------------------------------------------------- Second phase, we can add support for "mkdir" 1. mkdir info/L3_MON/counter_configs/mbm_read_only 2. mkdir option will create "event_filter" file. info/L3_MON/counter_configs/mbm_read_only/event_filter 3. Users can modify event configuration. echo LclFill > info/L3_MON/counter_configs/mbm_read_only/event_filter 4. Users can assign the events echo mbm_read_only > test/mon_data/mon_L3_00/assign_exclusive 5. Events can be read in test/mon_data/mon_data/mon_L3_00/mbm_read_only -- Thanks Babu Moger
