Hi Reinette,
On 6/20/2025 10:53 AM, Reinette Chatre wrote:
Hi Babu,
On 6/19/25 11:41 AM, Moger, Babu wrote:
On 6/17/25 22:59, Reinette Chatre wrote:
On 6/11/25 2:23 PM, Babu Moger wrote:
...
+ */
+static int resctrl_io_alloc_closid_get(struct rdt_resource *r)
+{
+ int num_closids = closids_supported();
+
+ if (resctrl_arch_get_cdp_enabled(r->rid))
+ num_closids *= 2;
+
+ if (num_closids != resctrl_arch_get_num_closid(r))
+ return -ENOSPC;
+
+ return closids_supported() - 1;
+}
resctrl_io_alloc_closid_get() seems to be trying to do two things:
- determine what the io_alloc_closid is
- make sure the io_alloc_closid is supported
I think this should be split into two functions. Once the
io_alloc_closid is determined to be supported and io_alloc
enabled then there is no reason to keep checking if it is
supported whenever the io_alloc_closid is queried.
How about simplifying this to:
/*
* note how this returns u32 that will eliminate
* unnecessary error checking in usages where io_alloc_closid
* needs to be determined after an resctrl_arch_get_io_alloc_enabled(r)
* already confirmed io_alloc is enabled
* function comment could note that this returns the CLOSID
* required by io_alloc but not whether the CLOSID can
* be supported, for this resctrl_io_alloc_closid_supported() should
* be used.
* Can also note that returned value will always be valid if
* resctrl_arch_get_io_alloc_enabled(r) is true.
*/
u32 resctrl_io_alloc_closid(struct rdt_resource *r) {
if (resctrl_arch_get_cdp_enabled(r->rid))
return resctrl_arch_get_num_closid(r)/2 - 1
else
return resctrl_arch_get_num_closid(r) -1
}
/*
* note how below already makes resctrl's io_alloc implementation
* more generic
*/
resctrl_io_alloc_closid_supported(u32 io_alloc_closid) {
return io_alloc_closid < closids_supported()
}
Sure.
Changed the check to
return io_alloc_closid == (closids_supported() -1)
resctrl_io_alloc_closid_supported() is not intended to reflect what the
value is but just check if provided value is supported. By changing the
check to above resctrl_io_alloc_closid_supported() does two things again
(what the move to new functions aimed to avoid): checking that the CLOSID
is supported while requiring that it is the highest supported CLOSID.
What issue(s) do you see with using "io_alloc_closid < closids_supported()"
as the check?
I don't see any issue. It should be fine. Will test and verify it.
thanks
Babu
