Re: [PATCH v6 15/24] x86/resctrl: Allow arch to allocate memory needed in resctrl_arch_rmid_read()

[James Morse <james.morse@arm.com>]

Hi Babu,

On 05/10/2023 22:46, Moger, Babu wrote:
> On 9/14/2023 12:21 PM, James Morse wrote:
>> Depending on the number of monitors available, Arm's MPAM may need to
>> allocate a monitor prior to reading the counter value. Allocating a
>> contended resource may involve sleeping.
>>
>> add_rmid_to_limbo() calls resctrl_arch_rmid_read() for multiple domains,
>> the allocation should be valid for all domains.
>>
>> __check_limbo() and mon_event_count() each make multiple calls to
>> resctrl_arch_rmid_read(), to avoid extra work on contended systems,
>> the allocation should be valid for multiple invocations of
>> resctrl_arch_rmid_read().
>>
>> Add arch hooks for this allocation, which need calling before
>> resctrl_arch_rmid_read(). The allocated monitor is passed to
>> resctrl_arch_rmid_read(), then freed again afterwards. The helper
>> can be called on any CPU, and can sleep.

>>   diff --git a/include/linux/resctrl.h b/include/linux/resctrl.h
>> index f7311102e94c..5e4b4df9610b 100644
>> --- a/include/linux/resctrl.h
>> +++ b/include/linux/resctrl.h
>> @@ -235,6 +235,9 @@ 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 architecture specific value from
>> + *            resctrl_arch_mon_ctx_alloc(), for MPAM this identifies
>> + *            the hardware monitor allocated for this read request.
>>    *
>>    * Some architectures need to sleep when first programming some of the counters.
>>    * (specifically: arm64's MPAM cache occupancy counters can return 'not ready'
>> @@ -248,7 +251,7 @@ void resctrl_offline_domain(struct rdt_resource *r, struct
>> rdt_domain *d);
>>    */
>>   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, void *arch_mon_ctx);

> Just wondering.. Have you thought about passing rmid_read structure to this function?

I did, but I'd prefer to leave that as private to resctrl as the proposed PMU driver ends
up using this API too.


> Because most of the information is already inside the rmid_read structure. We can avoid
> passing 7 parameters.

We'd end up passing all these parameters via memory ... but the compiler knows when it has
to do this, and when it doesn't. For example on aarch64 the compiler knows it can pass all
seven of these arguments in registers. On x86_64 it looks like 6 arguments can be passed,
and the last one is never used on x86 so it never needs reading off the stack. (all this
feels like micro-optimisation!)


Thanks,

James