Messages in this thread | | | Date | Thu, 7 Dec 2023 17:34:19 -0600 | Subject | Re: [PATCH 00/15] x86/resctrl : Support AMD QoS RMID Pinning feature | From | "Moger, Babu" <> |
| |
Hi Reinette,
On 12/7/2023 5:26 PM, Reinette Chatre wrote: > Hi Babu, > > On 12/7/2023 3:07 PM, Moger, Babu wrote: >> On 12/7/2023 1:29 PM, Reinette Chatre wrote: >>> On 12/7/2023 8:12 AM, Moger, Babu wrote: >>>> On 12/6/23 12:49, Reinette Chatre wrote: >>>>> On 12/6/2023 7:40 AM, Moger, Babu wrote: >>>>>> On 12/5/23 17:17, Reinette Chatre wrote: >>>>>>> On 11/30/2023 4:57 PM, Babu Moger wrote: >>>>>>>> b. Mount with ABMC support >>>>>>>> #umount /sys/fs/resctrl/ >>>>>>>> #mount -o abmc -t resctrl resctrl /sys/fs/resctrl/ >>>>>>>> >>>>>>> hmmm ... so this requires the user to mount resctrl, determine if the >>>>>>> feature is supported, unmount resctrl, remount resctrl with feature enabled. >>>>>>> Could you please elaborate what prevents this feature from being enabled >>>>>>> without needing to remount resctrl? >>>>>> Spec says >>>>>> "Enabling ABMC: ABMC is enabled by setting L3_QOS_EXT_CFG.ABMC_En=1 (see >>>>>> Figure 19-7). When the state of ABMC_En is changed, it must be changed to >>>>>> the updated value on all logical processors in the QOS Domain. >>>>>> Upon transitions of the ABMC_En the following actions take place: >>>>>> All ABMC assignable bandwidth counters are reset to 0. >>>>>> The L3 default mode bandwidth counters are reset to 0. >>>>>> The L3_QOS_ABMC_CFG MSR is reset to 0." >>>>>> >>>>>> So, all the monitoring group counters will be reset. >>>>>> >>>>>> It is technically possible to enable without remount. But ABMC mode >>>>>> requires few new files(in each group) which I added when mounted with "-o >>>>>> abmc". Thought it is a better option. >>>>>> >>>>>> Otherwise we need to add these files when ABMC is supported(not when >>>>>> enabled). Need to add another file in /sys/fs/resctrl/info/L3_MON to >>>>>> enable the feature on the fly. >>>>>> >>>>>> Both are acceptable options. Any thoughts? >>>>> The new resctrl files in info/ could always be present. For example, >>>>> user space may want to know how many counters are available before >>>>> enabling the feature. >>>>> >>>>> It is not yet obvious to me that this feature requires new files >>>>> in monitor groups. >>>> There are two MBM events(total and local) in each group. >>>> We should provide an interface to assign each event independently. >>>> User can assign only one event in a group. We should also provide an >>>> option assign both the events in the group. This needs to be done at >>>> resctrl group level. >>> Understood. I would like to start by considering how (if at all) existing >>> files may be used, thus my example of using mbm_total_bytes, before adding >>> more files. >>> >>> >>> ... >>> >>>>>>>> #cat /sys/fs/resctrl/mon_data/mon_L3_00/mbm_local_bytes >>>>>>>> Unavailable >>>>>>> I believe that "Unavailable" already has an accepted meaning within current >>>>>>> interface and is associated with temporary failure. Even the AMD spec states "This >>>>>>> is generally a temporary condition and subsequent reads may succeed". In the >>>>>>> scenario above there is no chance that this counter would produce a value later. >>>>>>> I do not think it is ideal to overload existing interface with different meanings >>>>>>> associated with a new hardware specific feature ... something like "Disabled" seems >>>>>>> more appropriate. >>>>>> Hardware still reports it as unavailable. Also, there are some error cases >>>>>> hardware can report unavailable. We may not be able to differentiate that. >>>>> This highlights that this resctrl feature is currently latched to AMD's >>>>> ABMC. I do not think we should require that this resctrl feature is backed >>>>> by hardware that can support reads of counters that are disabled. A counter >>>>> read really only needs to be sent to hardware if it is enabled. >>>>> >>>>>>> Considering this we may even consider using these files themselves as a >>>>>>> way to enable the counters if they are disabled. For example, just >>>>>>> "echo 1 > /sys/fs/resctrl/mon_data/mon_L3_00/mbm_total_bytes" can be used >>>>>> I am not sure about this. This is specific to domain 0. This group can >>>>>> have cpus from multiple domains. I think we should have the interface for >>>>>> all the domains(not for specific domain). >>>>> Are the ABMC registers not per CPU? This is unclear to me at this time >>>>> since changelog of patch #13 states it is per-CPU but yet the code >>>>> uses smp_call_function_any(). >>>> Here are the clarifications from hardware engineer about this. >>>> >>>> # While configuring the counter, should we have to write (L3_QOS_ABMC_CFG) >>>> on all the logical processors in a domain? >>>> >>>> No. In order to configure a specific counter, you only need to write it >>>> on a single logical processor in a domain. Configuring the actual ABMC >>>> counter is a side-effect of the write to this register. And the actual >>>> ABMC counter configuration is a global state. >>>> >>>> "Each logical processor implements a separate copy of these registers" >>>> identifies that if you write a 5 to L3_QOS_ABMC_CFG on C0T0, you will not >>>> read a 5 from the L3_QOS_ABMC_CFG register on C1T0. >>> Thank you for this information. Would reading L3_QOS_ABMC_DSC register on >>> C1T0 return the configuration written to L3_QOS_ABMC_CFG on C0T0 ? >> Yes. Because the counter counter configuration is global. Reading L3_QOS_ABMC_DSC will return the configuration of the counter specified by >> >> QOS_ABMC_CFG[CtrID]. > > To confirm, when you say "global" you mean within a domain?
Yes. That is correct.
> >>> Even so, you do confirm that the counter configuration is per domain. If I >>> understand correctly the implementation in this series assumes the counters >>> are programmed identically on all domains, but theoretically the system can support >>> domains with different counter configurations. For example, if a resource group >>> is limited to CPUs in one domain it would be unnecessary to consume the other >>> domain's counters. >> Yes. It is programmed on all the domains. Separating the domain >> configuration will require more changes. I am not planning to address >> in this series. > That may be ok. The priority is to consider how users want to interact with this > feature and create a suitable interface to support this. This version may not > separate domain configuration, but we do not want to create an the interface that > prevents such an enhancement in the future. Especially since it is already known > that hardware supports it.
Yes. Understood.
Thanks
Babu
| |