Hi Reinette, On Thu, Mar 13, 2025 at 10:22 PM Reinette Chatre <reinette.chatre@xxxxxxxxx> wrote: > > Hi Babu, > > On 3/13/25 1:13 PM, Moger, Babu wrote: > > On 3/13/25 11:08, Reinette Chatre wrote: > >> On 3/12/25 11:14 AM, Moger, Babu wrote: > >>> On 3/12/25 12:14, Reinette Chatre wrote: > >>>> On 3/12/25 9:03 AM, Moger, Babu wrote: > >>>>> On 3/12/25 10:07, Reinette Chatre wrote: > > > > Here are the steps. Just copying steps from Peters proposal. > > https://lore.kernel.org/lkml/CALPaoCiii0vXOF06mfV=kVLBzhfNo0SFqt4kQGwGSGVUqvr2Dg@xxxxxxxxxxxxxx/ > > Thank you very much for detailing the steps. It is starting the fall into place > for me. > > > > > > > 1. Mount the resctrl > > mount -t resctrl resctrl /sys/fs/resctrl > > I assume that on ABMC system the plan remains to have ABMC enabled by default, which > will continue to depend on BMEC. > > How would the existing BMEC implementation be impacted in this case? > > Without any changes to BMEC support the mbm_total_bytes_config and mbm_local_bytes_config > files will remain and user space may continue to use them to change the event > configurations with confusing expecations/results on an ABMC system. > > One possibility may be that a user may see below on ABMC system even if BMEC is supported: > # cat /sys/fs/resctrl/info/L3_MON/mon_features > llc_occupancy > mbm_total_bytes > mbm_local_bytes > > With the above a user cannot be expected to want to interact with mbm_total_bytes_config > and mbm_local_bytes_config, which may be the simplest to do. How about making mbm_local_bytes and mbm_total_bytes always be configured using mbm_{local,total}_bytes_config and only allowing the full ABMC configurability on user-defined configurations. This could resolve the issue of backwards compatibility with the BMEC files and remove the need for the user opting-in to ABMC mode. It will be less clean implementation-wise, since there will be two classes of event configuration to deal with, but I think it seems logical from the user's side. > > To follow that, we should also consider how "mon_features" will change with this > implementation. > > > > > 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 > > Looks good. Here we could perhaps start nitpicking about naming and line separation. > I think it may be easier if the fields are separated by comma, but more on that > below ... > > > > > # 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 > > We need to be clear on how user space interacts with this file. For example, > can user space "append" configurations? Specifically, if the file has > contents like your earlier example: > # cat info/L3_MON/counter_configs/mbm_local_bytes/event_filter > LclFill > LclNTWr > LclSlowFill > > Should above be created with (note "append" needed for second and third): > 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 > > Is it possible to set multiple configurations in one write like below? > echo "LclFill,LclNTWr,LclSlowFill" > info/L3_MON/counter_configs/mbm_local_bytes/event_filter > > (note above where it may be easier for user space to use comma (or some other field separator) > when providing multiple configurations at a time, with this, to match, having output in > commas may be easier since it makes user interface copy&paste easier) > > If file has content like: > # cat info/L3_MON/counter_configs/mbm_local_bytes/event_filter > LclNTWr > LclSlowFill > > What is impact of the following: > echo LclFill > info/L3_MON/counter_configs/mbm_local_bytes/event_filter > > Is it (append): > # cat info/L3_MON/counter_configs/mbm_local_bytes/event_filter > LclFill > LclNTWr > LclSlowFill > > or (overwrite): > # cat info/L3_MON/counter_configs/mbm_local_bytes/event_filter > LclFill > > I do think the interface will be more intuitive it if follows regular file > operations wrt "append" and such. I have not looked into how kernfs supports > "append". I expect specifying counter_configs to be a rare or one-time operation, so I think ease-of-use is the only concern. I think multiple, appending writes is the most straightforward to implement and invoke (for a shell user), but I think commas are easy enough to support as well, even though it would look better when reading back to see the entries on separate lines. I believe you can inspect the file descriptor's flags from the kernfs_open_file reference: of->file->f_flags & O_APPEND I haven't tried this, though. -Peter
