Messages in this thread | | | Date | Thu, 27 Apr 2023 16:40:00 -0700 | Subject | Re: [PATCH v3 11/19] x86/resctrl: Allow arch to allocate memory needed in resctrl_arch_rmid_read() | From | Reinette Chatre <> |
| |
Hi James,
On 4/27/2023 7:19 AM, James Morse wrote: > On 01/04/2023 00:27, Reinette Chatre wrote: >> On 3/20/2023 10:26 AM, James Morse wrote:
...
>>> #include <linux/module.h> >>> #include <linux/sizes.h> >>> #include <linux/slab.h> >>> @@ -271,7 +272,7 @@ static void smp_call_rmid_read(void *_arg) >>> >>> int resctrl_arch_rmid_read(struct rdt_resource *r, struct rdt_domain *d, >>> u32 closid, u32 rmid, enum resctrl_event_id eventid, >>> - u64 *val) >>> + u64 *val, int ignored) >>> { >>> struct rdt_hw_resource *hw_res = resctrl_to_arch_res(r); >>> struct rdt_hw_domain *hw_dom = resctrl_to_arch_dom(d); >>> @@ -317,9 +318,14 @@ void __check_limbo(struct rdt_domain *d, bool force_free) >>> u32 idx_limit = resctrl_arch_system_num_rmid_idx(); >>> struct rmid_entry *entry; >>> u32 idx, cur_idx = 1; >>> + int arch_mon_ctx; >>> bool rmid_dirty; >>> u64 val = 0; >>> >>> + arch_mon_ctx = resctrl_arch_mon_ctx_alloc(r, QOS_L3_OCCUP_EVENT_ID); >>> + if (arch_mon_ctx < 0) >>> + return; > >> The vision for this is not clear to me. When I read that context needs to be allocated >> I expect it to return a pointer to some new context, not an int. What would the >> "context" consist of? > > It might just need a different name. > > For MPAM, this is allocating a monitor, which is the hardware that does the counting in > the cache or the memory controller. The number of monitors is an implementation choice, > and may not match the number of CLOSID/RMID that are in use. There aren't guaranteed to be > enough to allocate one for every control or monitor group up front. > > The int being returned is the allocated monitor's index. It identifies which monitor needs > programming to read the provided CLOSID/RMID, and the counter register to read with the value.
I see.
> > I can allocate memory for an int if you think that is clearer. > (I was hoping to leave that for whoever needs something bigger than a pointer)
I'd rather not complicate it in this way.
>>> diff --git a/include/linux/resctrl.h b/include/linux/resctrl.h >>> index ff7452f644e4..03e4f41cd336 100644 >>> --- a/include/linux/resctrl.h >>> +++ b/include/linux/resctrl.h >>> @@ -233,6 +233,7 @@ void resctrl_offline_domain(struct rdt_resource *r, struct rdt_domain *d); >>> * @rmid: rmid of the counter to read. >>> * @eventid: eventid to read, e.g. L3 occupancy. >>> * @val: result of the counter read in bytes. >>> + * @arch_mon_ctx: An allocated context from resctrl_arch_mon_ctx_alloc(). >>> * > >> Could this description be expanded to indicate what this context is used for? > > Sure, > "An architecture specific value from resctrl_arch_mon_ctx_alloc(), for MPAM this > identifies the hardware monitor allocated for this read request".
This helps. Thank you.
Reinette
| |