On Thu, Mar 13, 2025 at 03:13:32PM -0500, Moger, Babu wrote: > 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 Once the "mkdir" support described below is implemented users will not need to redefine these legacy event file names. That makes me happy. > > 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. Maybe the one file could look like: # cat mon_L3_assignments mbm_total_bytes: exclusive mbm_local_bytes: shared mbm_read_only: unassigned with new lines appearing when mkdir creates new events, and the obvious write semantics: # echo "mbm_total_bytes: unassigned" > mon_L3_assignments to make updates. > 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 Is there a matching "rmdir" to make this go away again? > -- > Thanks > Babu Moger
