Mailing List Archive

[PATCH v3 06/19] x86/resctrl: Allow the allocator to check if a CLOSID can allocate clean RMID
MPAM's PMG bits extend its PARTID space, meaning the same PMG value can be
used for different control groups.

This means once a CLOSID is allocated, all its monitoring ids may still be
dirty, and held in limbo.

Add a helper to allow the CLOSID allocator to check if a CLOSID has dirty
RMID values. This behaviour is enabled by a kconfig option selected by
the architecture, which avoids a pointless search for x86.

Tested-by: Shaopeng Tan <tan.shaopeng@fujitsu.com>
Signed-off-by: James Morse <james.morse@arm.com>

---
Changes since v1:
* Removed superflous IS_ENABLED().

Changes since v2:
* Reworded comment over resctrl_closid_is_dirty() to reflect this is all RMID.
---
arch/x86/kernel/cpu/resctrl/internal.h | 1 +
arch/x86/kernel/cpu/resctrl/monitor.c | 36 ++++++++++++++++++++++++++
arch/x86/kernel/cpu/resctrl/rdtgroup.c | 17 +++++++-----
3 files changed, 47 insertions(+), 7 deletions(-)

diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
index e11d9ce943d3..87545e4beb70 100644
--- a/arch/x86/kernel/cpu/resctrl/internal.h
+++ b/arch/x86/kernel/cpu/resctrl/internal.h
@@ -534,6 +534,7 @@ int rdtgroup_pseudo_lock_create(struct rdtgroup *rdtgrp);
void rdtgroup_pseudo_lock_remove(struct rdtgroup *rdtgrp);
struct rdt_domain *get_domain_from_cpu(int cpu, struct rdt_resource *r);
int closids_supported(void);
+bool resctrl_closid_is_dirty(u32 closid);
void closid_free(int closid);
int alloc_rmid(u32 closid);
void free_rmid(u32 closid, u32 rmid);
diff --git a/arch/x86/kernel/cpu/resctrl/monitor.c b/arch/x86/kernel/cpu/resctrl/monitor.c
index ca58a433c668..a2ae4be4b2ba 100644
--- a/arch/x86/kernel/cpu/resctrl/monitor.c
+++ b/arch/x86/kernel/cpu/resctrl/monitor.c
@@ -363,6 +363,42 @@ static struct rmid_entry *resctrl_find_free_rmid(u32 closid)
return ERR_PTR(-ENOSPC);
}

+/**
+ * resctrl_closid_is_dirty - Determine if all RMID associated with this CLOSID
+ * are available.
+ * @closid: The CLOSID that is being queried.
+ *
+ * MPAM's equivalent of RMID are per-CLOSID, meaning a freshly allocated CLOSID
+ * may not be able to allocate clean RMID. To avoid this the allocator will
+ * only return clean CLOSID. This is enough for now as it allows MPAM systems
+ * to use resctrl. This suffers from the problem that there may be no CLOSID
+ * where all the RMID are clean, causing the CLOSID allocation to fail.
+ * This can be improved (once MPAM support is upstream) to return the cleanest
+ * CLOSID where PMG=0 is clean. This would allow the CLOSID allocation to
+ * succeed, but subsequent monitor-group allocations may fail.
+ */
+bool resctrl_closid_is_dirty(u32 closid)
+{
+ struct rmid_entry *entry;
+ int i;
+
+ lockdep_assert_held(&rdtgroup_mutex);
+
+ if (!IS_ENABLED(CONFIG_RESCTRL_RMID_DEPENDS_ON_CLOSID))
+ return false;
+
+ for (i = 0; i < resctrl_arch_system_num_rmid_idx(); i++) {
+ entry = &rmid_ptrs[i];
+ if (entry->closid != closid)
+ continue;
+
+ if (entry->busy)
+ return true;
+ }
+
+ return false;
+}
+
/*
* For MPAM the RMID value is not unique, and has to be considered with
* the CLOSID. The (CLOSID, RMID) pair is allocated on all domains, which
diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
index bcd27610bb77..e741bc47bae9 100644
--- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
+++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
@@ -93,7 +93,7 @@ void rdt_last_cmd_printf(const char *fmt, ...)
* - Our choices on how to configure each resource become progressively more
* limited as the number of resources grows.
*/
-static int closid_free_map;
+static unsigned long closid_free_map;
static int closid_free_map_len;

int closids_supported(void)
@@ -119,14 +119,17 @@ static void closid_init(void)

static int closid_alloc(void)
{
- u32 closid = ffs(closid_free_map);
+ u32 closid;

- if (closid == 0)
- return -ENOSPC;
- closid--;
- closid_free_map &= ~(1 << closid);
+ for_each_set_bit(closid, &closid_free_map, closid_free_map_len) {
+ if (resctrl_closid_is_dirty(closid))
+ continue;

- return closid;
+ clear_bit(closid, &closid_free_map);
+ return closid;
+ }
+
+ return -ENOSPC;
}

void closid_free(int closid)
--
2.39.2
Re: [PATCH v3 06/19] x86/resctrl: Allow the allocator to check if a CLOSID can allocate clean RMID [ In reply to ]
Hi James,

On 3/20/2023 10:26 AM, James Morse wrote:

...

> +/**
> + * resctrl_closid_is_dirty - Determine if all RMID associated with this CLOSID
> + * are available.
> + * @closid: The CLOSID that is being queried.
> + *
> + * MPAM's equivalent of RMID are per-CLOSID, meaning a freshly allocated CLOSID
> + * may not be able to allocate clean RMID. To avoid this the allocator will
> + * only return clean CLOSID. This is enough for now as it allows MPAM systems
> + * to use resctrl. This suffers from the problem that there may be no CLOSID
> + * where all the RMID are clean, causing the CLOSID allocation to fail.
> + * This can be improved (once MPAM support is upstream) to return the cleanest
> + * CLOSID where PMG=0 is clean. This would allow the CLOSID allocation to

Why does PMG=0 have to be the clean ID?

I am wondering about the use cases here. When a new CLOSID needs to be allocated,
would it not be useful to instead have a utility that returns the "cleanest" CLOSID?
Instead of picking an available CLOSID and then always have to check if it is
"dirty or not", why not have a utility that picks the CLOSID with the most
available PMGs?

Reinette
Re: [PATCH v3 06/19] x86/resctrl: Allow the allocator to check if a CLOSID can allocate clean RMID [ In reply to ]
Hi Reinette,

On 01/04/2023 00:21, Reinette Chatre wrote:
> On 3/20/2023 10:26 AM, James Morse wrote:
>> +/**
>> + * resctrl_closid_is_dirty - Determine if all RMID associated with this CLOSID
>> + * are available.
>> + * @closid: The CLOSID that is being queried.
>> + *
>> + * MPAM's equivalent of RMID are per-CLOSID, meaning a freshly allocated CLOSID
>> + * may not be able to allocate clean RMID. To avoid this the allocator will
>> + * only return clean CLOSID. This is enough for now as it allows MPAM systems
>> + * to use resctrl. This suffers from the problem that there may be no CLOSID
>> + * where all the RMID are clean, causing the CLOSID allocation to fail.
>> + * This can be improved (once MPAM support is upstream) to return the cleanest
>> + * CLOSID where PMG=0 is clean. This would allow the CLOSID allocation to

> Why does PMG=0 have to be the clean ID?

True, there ends up being a second search anyway.


> I am wondering about the use cases here. When a new CLOSID needs to be allocated,
> would it not be useful to instead have a utility that returns the "cleanest" CLOSID?

It would, but this is a trade off between churn and features, I'm trying to do the minimum
to get feature parity for supporting MPAM by keeping any additional code that x86 doesn't
use small and simple. Improvements that only affect MPAM can be kicked down the road.

But as we're discussing it...


> Instead of picking an available CLOSID and then always have to check if it is
> "dirty or not", why not have a utility that picks the CLOSID with the most
> available PMGs?

I think an extra array to keep track of this is simplest as it avoids a complex walk of
the rmid_ptrs[] array looking for the global minimum across a number of entries. I think
it would be two additional patches. I'll include this in the next version.


Thanks,

James