[PATCH] x86/resctrl: Don't stop walking closids when a locksetup group is found

[PATCH] x86/resctrl: Don't stop walking closids when a locksetup group is found

[James Morse]

When a new control group is created __init_one_rdt_domain() walks all
the other closids to calculate the sets of used and unused bits.

If it discovers a pseudo_locksetup group, it breaks out of the loop.
This means any later closid doesn't get its used bits added to used_b.
These bits will then get set in unused_b, and added to the new
control group's configuration, even if they were marked as exclusive
for a later closid.

When encountering a pseudo_locksetup group, we should continue. This
is because "a resource group enters 'pseudo-locked' mode after the
schemata is written while the resource group is in 'pseudo-locksetup'
mode." When we find a pseudo_locksetup group, its configuration is
expected to be overwritten, we can skip it.

Fixes: dfe9674b04ff6 ("x86/intel_rdt: Enable entering of pseudo-locksetup mode")
Signed-off-by: James Morse <james.morse@arm.com>
---
 arch/x86/kernel/cpu/resctrl/rdtgroup.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
index 333c177a2471..049ccb709957 100644
--- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
+++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
@@ -2541,8 +2541,15 @@ static int __init_one_rdt_domain(struct rdt_domain *d, struct rdt_resource *r,
 	for (i = 0; i < closids_supported(); i++, ctrl++) {
 		if (closid_allocated(i) && i != closid) {
 			mode = rdtgroup_mode_by_closid(i);
-			if (mode == RDT_MODE_PSEUDO_LOCKSETUP)
-				break;
+			if (mode == RDT_MODE_PSEUDO_LOCKSETUP) {
+				/*
+				 * ctrl values for locksetup aren't relevant
+				 * until the schemata is written, and the mode
+				 * becomes RDT_MODE_PSEUDO_LOCKED.
+				 */
+				continue;
+			}
+
 			/*
 			 * If CDP is active include peer domain's
 			 * usage to ensure there is no overlap
-- 
2.20.1

Re: [PATCH] x86/resctrl: Don't stop walking closids when a locksetup group is found

[Reinette Chatre]

Hi James,

On 5/30/2019 8:57 AM, James Morse wrote:
> When a new control group is created __init_one_rdt_domain() walks all
> the other closids to calculate the sets of used and unused bits.
> 
> If it discovers a pseudo_locksetup group, it breaks out of the loop.
> This means any later closid doesn't get its used bits added to used_b.
> These bits will then get set in unused_b, and added to the new
> control group's configuration, even if they were marked as exclusive
> for a later closid.
> 
> When encountering a pseudo_locksetup group, we should continue. This
> is because "a resource group enters 'pseudo-locked' mode after the
> schemata is written while the resource group is in 'pseudo-locksetup'
> mode." When we find a pseudo_locksetup group, its configuration is
> expected to be overwritten, we can skip it.
> 
> Fixes: dfe9674b04ff6 ("x86/intel_rdt: Enable entering of pseudo-locksetup mode")
> Signed-off-by: James Morse <james.morse@arm.com>
> ---
>  arch/x86/kernel/cpu/resctrl/rdtgroup.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> index 333c177a2471..049ccb709957 100644
> --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> @@ -2541,8 +2541,15 @@ static int __init_one_rdt_domain(struct rdt_domain *d, struct rdt_resource *r,
>  	for (i = 0; i < closids_supported(); i++, ctrl++) {
>  		if (closid_allocated(i) && i != closid) {
>  			mode = rdtgroup_mode_by_closid(i);
> -			if (mode == RDT_MODE_PSEUDO_LOCKSETUP)
> -				break;
> +			if (mode == RDT_MODE_PSEUDO_LOCKSETUP) {
> +				/*
> +				 * ctrl values for locksetup aren't relevant
> +				 * until the schemata is written, and the mode
> +				 * becomes RDT_MODE_PSEUDO_LOCKED.
> +				 */
> +				continue;
> +			}
> +
>  			/*
>  			 * If CDP is active include peer domain's
>  			 * usage to ensure there is no overlap
> 

Thank you very much for catching this. The only possible quibbles that
will be guided by the maintainer's style preference are the unnecessary
braces and extra empty line, but this fix is needed. Also, could "Cc:
stable@vger.kernel.org" please be added to the sign-off area?

Acked-by: Reinette Chatre <reinette.chatre@intel.com>

Reinette