Re: [PATCH v13 13/25] arm,x86,fs/resctrl: Allocate maximum needed rmid_ptrs[]

From: Moger, Babu

Date: Wed Oct 07 2026 - 10:14:15 EST

Hi Tony,

On 10/6/2026 5:07 PM, Luck, Tony wrote:

Hi Babu,

On Tue, Oct 06, 2026 at 02:53:49PM -0500, Babu Moger wrote:

Hi Tony,

... snip ...

int setup_rmid_lru_list(void)
{
- struct rmid_entry *entry = NULL;
- u32 idx_limit;
- u32 idx;
+ struct rmid_entry *entry;
+ u32 cur_idx_limit;
+ u32 rsvd_idx;
int i;
if (!resctrl_mon_capable())
return 0;
/*
- * Called on every mount, but the number of RMIDs cannot change
- * after the first mount, so keep using the same set of rmid_ptrs[]
- * until resctrl_exit(). Note that the limbo handler continues to
- * access rmid_ptrs[] after resctrl is unmounted.
+ * Allocate the largest number of RMIDs that this system will ever
+ * need. These cannot be freed until resctrl_exit() because the limbo
+ * handler continues to access rmid_ptrs[] after resctrl is unmounted.
*/
- if (rmid_ptrs)
- return 0;
+ if (!rmid_ptrs) {
+ num_rmid_ptrs = resctrl_arch_system_max_rmid_idx();
+ rmid_ptrs = kzalloc_objs(struct rmid_entry, num_rmid_ptrs);
+ if (!rmid_ptrs) {
+ num_rmid_ptrs = 0;
+ return -ENOMEM;
+ }
- idx_limit = resctrl_arch_system_num_rmid_idx();
- rmid_ptrs = kzalloc_objs(struct rmid_entry, idx_limit);
- if (!rmid_ptrs)
- return -ENOMEM;
+ for (i = 0; i < num_rmid_ptrs; i++) {
+ entry = &rmid_ptrs[i];
+ INIT_LIST_HEAD(&entry->list);

[1]. All the nodes are initialized. Basically, it is pointing to itself now.

This is setting the ->next and ->prev fields to point to the entry. So
each of them is an empty list. Maybe I should just delete this as it is
pointless. Later on these objects are added to the free list with:

list_add_tail(&entry->list, &rmid_free_lru);

but if you dig into that code you'll see that the ->next and ->prev
fields are never read (not even in all the debug/sanity routines). Both
fields are overwritten to add them to the free list.

Note that this seems to be a common pattern. Code does
INIT_LIST_HEAD(&x->list) and then immediately uses list_add()
or list_add_tail() to put "x" onto a list.

- for (i = 0; i < idx_limit; i++) {
- entry = &rmid_ptrs[i];
- INIT_LIST_HEAD(&entry->list);
+ resctrl_arch_rmid_idx_decode(i, &entry->closid, &entry->rmid);
+ }
+ }
- resctrl_arch_rmid_idx_decode(i, &entry->closid, &entry->rmid);
- list_add_tail(&entry->list, &rmid_free_lru);
+ /* Find how many RMIDs are available for this mount */
+ cur_idx_limit = resctrl_arch_system_num_rmid_idx();
+ if (cur_idx_limit > num_rmid_ptrs) {
+ pr_warn_once("RMID count %u exceeds allocation. Limit to %u\n",
+ cur_idx_limit, num_rmid_ptrs);
+ cur_idx_limit = num_rmid_ptrs;
}
+ INIT_LIST_HEAD(&rmid_free_lru);

Initializing the list head without first cleaning up existing entries could
be problematic here.

I don't see a problem. Any entries on the list are left floating, but
they are all in the rmid_ptrs[] so easy to access them again.
rmid_free_list is now an empty list having discarded all entries that
were on it. Sure, those floating entries have stale pointers sitting in
their ->next and ->prev fields. But those pointers are not going to be
used by any code.

The first mount works correctly, and INIT_LIST_HEAD() is not needed in that
case because the list head is already statically initialized.

However, on a subsequent mount, the list entries remain linked from the
previous mount through the setup performed in [2] below.

INIT_LIST_HEAD() only reinitializes the list head itself. It does not
traverse the list and detach or reinitialize existing entries as was done in
[1].

I think you need to replace

INIT_LIST_HEAD(&rmid_free_lru);

to

while (!list_empty(&rmid_free_lru))
list_del_init(rmid_free_lru.next);

That would set up all those floating entries each as an empty list. But
doing that doesn't matter. No code is going to look at the ->next or
->prev fields.

That is correct. It may not be an issue in this case. The only concern is that the pointers are being overwritten without being cleaned up first. I agree that it may not cause a problem at this point, though I'm not sure whether list debugging would report any issues.

I'll leave it to you.

Thanks,
Babu