Plucking out just the big, direction change, comment for discussion (which may make several of the code comments moot). > I needed to refresh my understanding of this work by re-reading the previous discussions. > You mentioned in [2]: > I tried out some code to make the event runtime selectable via a r/w file in the > resctrl/info directories. But that got complicated because of the amount of state > that needs to be updated when switching events. > > Could you please clarify which state you referred to? I wonder if it may be the > struct mbm_state state maintained by mbm_bw_count()? mbm_bw_count() is lightweight > and I see no problem with it being called for all supported MBM events when > the software controller is enabled. With state for all supported events always available > it seems simpler to runtime switch between which events guide the software controller? > > Thinking about it more, it seems possible for the user to use different > MBM events to guide the software controller for different resource groups. > > If it is possible to do runtime switching in this way I do think it will simplify this > implementation while not requiring the user to remount resctrl to make changes. You > mentioned [3] that "a separate patch series" may be coming to address this but doing this > now seems simpler while avoiding any future work as well as confusing duplicate ABI > ... unless you were referring to other issues that needs to be addressed separately? Yes, the state maintained by mbm_bw_count() was the piece that worried me. After a user switch to a different event there would be no bandwidth data until two updates passed by (one to get a baseline, second to compute bandwidth). So update_mba_bw() would need to be aware of this liminal period to avoid making updates with no data to back them up. Your solution is elegant. The cost to maintain bandwidth data for each event is indeed very low. So there are no weird transition cases. update_mba_bw() can immediately compare bandwidth for the new event against the target bandwidth and make appropriate adjustments. This requires a new file in each CTRL_MON directory when mba_sc is enabled so the user can make their selection. Note that technically it would be possible to make a different selection for each domain. But that seems like an option without an obvious use case and would just complicate the syntax of the new file. Maybe name this new file "mba_sc_event"[1] with contents that match the names of the mbm_monitor events as listed in /sys/fs/resctrl/info/L3_MON/mon_features? So default state when resctrl is mounted with the software controller enabled would have: $ cat /sys/fs/resctrl/mba_sc_event mbm_local_bytes User could switch to total with # echo mbm_total_bytes > /sys/fs/resctrl/mba_sc_event On systems where mbm_local_bytes is not supported default would be mbm_total_bytes. New CTRL_MON directories would also default to mbm_local_bytes if it is supported. -Tony [1] Other suggestions welcome.
