| Message ID | 20241207092136.2488426-1-zengheng4@huawei.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | arm_mpam: Introduce the Narrow-PARTID feature for MPAM driver | expand |
Hi, On Sat, Dec 07, 2024 at 05:21:31PM +0800, Zeng Heng wrote: > The patch set is applied for mpam/snapshot/v6.12-rc1 branch of > https://git.kernel.org/pub/scm/linux/kernel/git/morse/linux.git > repository. > > The narrow-partid feature in MPAM allows for a more efficient use of > PARTIDs by enabling a many-to-one mapping of reqpartids (requested PARTIDs) > to intpartids (internal PARTIDs). This mapping reduces the number of unique > PARTIDs needed, thus allowing more tasks or processes to be monitored and > managed with the available resources. > > For a mixture of MSCs system, for MSCs that do not support narrow-partid, > we use the PARTIDs exceeding the number of closids as reqPARTIDs for > expanding the monitoring groups. Mixed systems still seem not to be handled completely by this series? You do cope with the scenario where some MSCs support Narrowing and some do not, but there is still the problem of incompatible controls. If a non-Narrowing MSC has controls that are not of the "partition bitmap" type, then splitting a resctrl control group across multiple PARTIDs is going to change the hardware regulation behaviour. There does not seem to be any way to work around this be programming different control values (for example). So, there may be over- or under-allocation of resources compared with what the user requests in resctrlfs. So, I think there is still a need to check which controls are present, and either disable the use of non-identity intPARTID<->reqPARTID mappings if incompatible controls are present (or don't expose those controls to resctrl). (If you were not trying to address this issue yet then that is not a problem for an RFC, but it is best to be clear about the limitations...) > In order to keep the existing resctrl API interface, the rmid contains both > req_idx and PMG information instead of PMG only under the MPAM driver. The > req_idx represents the req_idx-th sub-monitoring group under the control > group. The new rmid would be like: > > rmid = (req_idx << shift | pmg). > > The new conversion relationship between closid/rmid and (req)PARTID/PMG is: > > (req)PARTID = (rmid.req_idx * n) + closid, > PMG = rmid.pmg. > > Each intPARTID has m reqPARTIDs, which are used to expand the number of > monitoring groups under the control group. Therefore, the number of > monitoring groups is no longer limited by the range of MPAM PMG, which > enhances the extensibility of the system's monitoring capabilities. > > --- > Compared with v1: > - Rebase this patch set on latest MPAM driver of the v6.12-rc1 branch. > > Compared with v2: > - Refactor closid/rmid pair translation > - Simplify the logic of synchronize configuration > - Remove reqPARTID pool > --- This approach looks reasonable overall, and in this version the changes do seem to be better localised in the mpam_resctrl.c glue code now. I had also been working on a similar approach, so I have posted it for comparison [1] -- though the two approaches are doing pretty much the same thing, some details differ. (Note, I have not addressed PARTID Narrowing at all yet; however, I think more thought is needed for that.) Note: Are there bisection issues with some of the patches in the series? It looks like not all of the ID conversions are applied in the same patch, so I'm wondering whether strange behaviour may be seen at the intermediate commits. Cheers ---Dave [1] [RFC PATCH 0/6] Introduce flexible CLOSID/RMID translation https://lore.kernel.org/lkml/20241212154000.330467-1-Dave.Martin@arm.com/
On 2024/12/13 0:17, Dave Martin wrote: > Hi, > > On Sat, Dec 07, 2024 at 05:21:31PM +0800, Zeng Heng wrote: >> The patch set is applied for mpam/snapshot/v6.12-rc1 branch of >> https://git.kernel.org/pub/scm/linux/kernel/git/morse/linux.git >> repository. >> >> The narrow-partid feature in MPAM allows for a more efficient use of >> PARTIDs by enabling a many-to-one mapping of reqpartids (requested PARTIDs) >> to intpartids (internal PARTIDs). This mapping reduces the number of unique >> PARTIDs needed, thus allowing more tasks or processes to be monitored and >> managed with the available resources. >> >> For a mixture of MSCs system, for MSCs that do not support narrow-partid, >> we use the PARTIDs exceeding the number of closids as reqPARTIDs for >> expanding the monitoring groups. > > Mixed systems still seem not to be handled completely by this series? > > You do cope with the scenario where some MSCs support Narrowing and > some do not, but there is still the problem of incompatible controls. > > If a non-Narrowing MSC has controls that are not of the "partition > bitmap" type, then splitting a resctrl control group across multiple > PARTIDs is going to change the hardware regulation behaviour. There > does not seem to be any way to work around this be programming > different control values (for example). So, there may be over- or > under-allocation of resources compared with what the user requests in > resctrlfs. > > So, I think there is still a need to check which controls are present, > and either disable the use of non-identity intPARTID<->reqPARTID > mappings if incompatible controls are present (or don't expose those > controls to resctrl). > > (If you were not trying to address this issue yet then that is not a > problem for an RFC, but it is best to be clear about the > limitations...) > In the v3, the limitation you mentioned has not yet been handled. V3 has not yet determined how to define this limitation well. Additionally, for v3, I was still uncertain whether a large-scale refactoring was still necessary at that time. In fact, I take this limitation very seriously, and I will try to define the limitation in next version, explaining it separately as a distinct patch. >> In order to keep the existing resctrl API interface, the rmid contains both >> req_idx and PMG information instead of PMG only under the MPAM driver. The >> req_idx represents the req_idx-th sub-monitoring group under the control >> group. The new rmid would be like: >> >> rmid = (req_idx << shift | pmg). >> >> The new conversion relationship between closid/rmid and (req)PARTID/PMG is: >> >> (req)PARTID = (rmid.req_idx * n) + closid, >> PMG = rmid.pmg. >> >> Each intPARTID has m reqPARTIDs, which are used to expand the number of >> monitoring groups under the control group. Therefore, the number of >> monitoring groups is no longer limited by the range of MPAM PMG, which >> enhances the extensibility of the system's monitoring capabilities. >> >> --- >> Compared with v1: >> - Rebase this patch set on latest MPAM driver of the v6.12-rc1 branch. >> >> Compared with v2: >> - Refactor closid/rmid pair translation >> - Simplify the logic of synchronize configuration >> - Remove reqPARTID pool >> --- > > This approach looks reasonable overall, and in this version the changes > do seem to be better localised in the mpam_resctrl.c glue code now. > > I had also been working on a similar approach, so I have posted it for > comparison [1] -- though the two approaches are doing pretty much the > same thing, some details differ. > > (Note, I have not addressed PARTID Narrowing at all yet; however, > I think more thought is needed for that.) > Yes, localize the narrow-partid feature within mpam_resctrl.c file is the biggest restructuring improvement in this version. Of course, thanks for your meticulous review and insightful comments. In the meanwhile, I have roughly read through the patch set you sent out, especially patch 4 (arm_mpam: Introduce flexible CLOSID/RMID translation). If I have any comments worth discussing, I will try to share them. > > Note: Are there bisection issues with some of the patches in the > series? It looks like not all of the ID conversions are applied in the > same patch, so I'm wondering whether strange behaviour may be seen at > the intermediate commits. > The original intention of splitting the patch set was to facilitate review. I will merge the part of patch 5 into patch 2 and consider making the patch set be friendly for bisection issues. Best regards, Zeng Heng
On Fri, Dec 20, 2024 at 06:59:31PM +0800, Zeng Heng wrote: > > > On 2024/12/13 0:17, Dave Martin wrote: > > Hi, > > > > On Sat, Dec 07, 2024 at 05:21:31PM +0800, Zeng Heng wrote: [...] > > > For a mixture of MSCs system, for MSCs that do not support narrow-partid, > > > we use the PARTIDs exceeding the number of closids as reqPARTIDs for > > > expanding the monitoring groups. > > > > Mixed systems still seem not to be handled completely by this series? > > > > You do cope with the scenario where some MSCs support Narrowing and > > some do not, but there is still the problem of incompatible controls. > > > > If a non-Narrowing MSC has controls that are not of the "partition > > bitmap" type, then splitting a resctrl control group across multiple > > PARTIDs is going to change the hardware regulation behaviour. There > > does not seem to be any way to work around this be programming > > different control values (for example). So, there may be over- or > > under-allocation of resources compared with what the user requests in > > resctrlfs. > > > > So, I think there is still a need to check which controls are present, > > and either disable the use of non-identity intPARTID<->reqPARTID > > mappings if incompatible controls are present (or don't expose those > > controls to resctrl). > > > > (If you were not trying to address this issue yet then that is not a > > problem for an RFC, but it is best to be clear about the > > limitations...) > > > > In the v3, the limitation you mentioned has not yet been handled. > V3 has not yet determined how to define this limitation well. Additionally, > for v3, I was still uncertain whether a large-scale > refactoring was still necessary at that time. > > In fact, I take this limitation very seriously, and I will try to define > the limitation in next version, explaining it separately as a distinct > patch. Fair enough. I just wanted to make sure that this issue is not missed (since this is where a lot of the complexity comes from!) [...] > > This approach looks reasonable overall, and in this version the changes > > do seem to be better localised in the mpam_resctrl.c glue code now. > > > > I had also been working on a similar approach, so I have posted it for > > comparison [1] -- though the two approaches are doing pretty much the > > same thing, some details differ. > > > > (Note, I have not addressed PARTID Narrowing at all yet; however, > > I think more thought is needed for that.) > > > > Yes, localize the narrow-partid feature within mpam_resctrl.c file is > the biggest restructuring improvement in this version. Of course, thanks > for your meticulous review and insightful comments. > > In the meanwhile, I have roughly read through the patch set you sent > out, especially patch 4 (arm_mpam: Introduce flexible CLOSID/RMID > translation). If I have any comments worth discussing, I will try to > share them. I would appreciate that, thanks! > > > > > Note: Are there bisection issues with some of the patches in the > > series? It looks like not all of the ID conversions are applied in the > > same patch, so I'm wondering whether strange behaviour may be seen at > > the intermediate commits. > > > > The original intention of splitting the patch set was to facilitate review. > I will merge the part of patch 5 into patch 2 and consider > making the patch set be friendly for bisection issues. > > > Best regards, > Zeng Heng Understood. James may have a view on how important bisectability is, since this is not upstream code -- but it is probably a good idea to maintain full bisectability if at all possible. Cheers ---Dave
The patch set is applied for mpam/snapshot/v6.12-rc1 branch of https://git.kernel.org/pub/scm/linux/kernel/git/morse/linux.git repository. The narrow-partid feature in MPAM allows for a more efficient use of PARTIDs by enabling a many-to-one mapping of reqpartids (requested PARTIDs) to intpartids (internal PARTIDs). This mapping reduces the number of unique PARTIDs needed, thus allowing more tasks or processes to be monitored and managed with the available resources. For a mixture of MSCs system, for MSCs that do not support narrow-partid, we use the PARTIDs exceeding the number of closids as reqPARTIDs for expanding the monitoring groups. In order to keep the existing resctrl API interface, the rmid contains both req_idx and PMG information instead of PMG only under the MPAM driver. The req_idx represents the req_idx-th sub-monitoring group under the control group. The new rmid would be like: rmid = (req_idx << shift | pmg). The new conversion relationship between closid/rmid and (req)PARTID/PMG is: (req)PARTID = (rmid.req_idx * n) + closid, PMG = rmid.pmg. Each intPARTID has m reqPARTIDs, which are used to expand the number of monitoring groups under the control group. Therefore, the number of monitoring groups is no longer limited by the range of MPAM PMG, which enhances the extensibility of the system's monitoring capabilities. --- Compared with v1: - Rebase this patch set on latest MPAM driver of the v6.12-rc1 branch. Compared with v2: - Refactor closid/rmid pair translation - Simplify the logic of synchronize configuration - Remove reqPARTID pool --- Dave Martin (1): arm_mpam: Set INTERNAL as needed when setting MSC controls Zeng Heng (4): arm_mpam: Introduce the definitions of intPARTID and reqPARTID arm_mpam: Read monitor value with new closid/rmid pair arm_mpam: Automatically synchronize the configuration of all sub-monitoring groups arm_mpam: Adapting the closid/rmid matching determination functions arch/arm64/include/asm/mpam.h | 6 +- drivers/platform/arm64/mpam/mpam_devices.c | 61 +++++++-- drivers/platform/arm64/mpam/mpam_internal.h | 5 + drivers/platform/arm64/mpam/mpam_resctrl.c | 142 +++++++++++++++----- 4 files changed, 170 insertions(+), 44 deletions(-) -- 2.25.1