Re: [PATCH v4 3/7] x86/resctrl: Change monitor code to use rdt_mondomain
From: Reinette Chatre <reinette.chatre@intel.com>
Date: 2023-08-11 17:31:31
Also in:
linux-patches, lkml
Hi Tony, On 7/22/2023 12:07 PM, Tony Luck wrote:
A few functions need to be duplicated to provide versions to operate on control and monitor domains respectively. But most of the changes are just fixing argument and return value types.
Could you please add some context in support of this change? I do not think "duplicated" is appropriate though. Functions are not duplicated but instead made to be dedicated to either control or monitoring domains, no? ...
quoted hunk ↗ jump to hunk
diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c index 274605aaa026..0161362b0c3e 100644 --- a/arch/x86/kernel/cpu/resctrl/core.c +++ b/arch/x86/kernel/cpu/resctrl/core.c@@ -393,9 +393,12 @@ void rdt_ctrl_update(void *arg) * id is found in a domain, return the domain. Otherwise, if requested by * caller, return the first domain whose id is bigger than the input id. * The domain list is sorted by id in ascending order. + * + * N.B. Returned value may be either a pointer to "struct rdt_domain" or + * to "struct rdt_mondomain" depending on which domain list is scanned. */ -struct rdt_domain *rdt_find_domain(struct list_head *h, int id, - struct list_head **pos) +void *rdt_find_domain(struct list_head *h, int id, + struct list_head **pos) { struct rdt_domain *d; struct list_head *l;
I do not think that void pointers should be passed around. How about two new functions dedicated to the different domain types with the void pointer handling contained in a static function? For example, static void *__rdt_find_domain(struct list_head *h, int id, struct list_head **pos) struct rdt_mondomain *rdt_find_mondomain(struct rdt_resource *r, int id, struct list_head **pos) struct rdt_domain *rdt_find_ctrldomain(struct rdt_resource *r, int id, struct list_head **pos) rdt_find_mondomain() and rdt_find_ctrldomain() would be what callers use while they can be wrappers of __rdt_find_domain().
quoted hunk ↗ jump to hunk
@@ -434,10 +437,15 @@ static void setup_default_ctrlval(struct rdt_resource *r, u32 *dc) } static void domain_free(struct rdt_hw_domain *hw_dom) +{ + kfree(hw_dom->ctrl_val); + kfree(hw_dom); +} + +static void mondomain_free(struct rdt_hw_mondomain *hw_dom) { kfree(hw_dom->arch_mbm_total); kfree(hw_dom->arch_mbm_local); - kfree(hw_dom->ctrl_val); kfree(hw_dom); }@@ -467,7 +475,7 @@ static int domain_setup_ctrlval(struct rdt_resource *r, struct rdt_domain *d) * @num_rmid: The size of the MBM counter array * @hw_dom: The domain that owns the allocated arrays */ -static int arch_domain_mbm_alloc(u32 num_rmid, struct rdt_hw_domain *hw_dom) +static int arch_domain_mbm_alloc(u32 num_rmid, struct rdt_hw_mondomain *hw_dom) { size_t tsize;@@ -539,8 +547,8 @@ static void domain_add_cpu_mon(int cpu, struct rdt_resource *r) { int id = get_cpu_cacheinfo_id(cpu, r->mon_scope); struct list_head *add_pos = NULL; - struct rdt_hw_domain *hw_dom; - struct rdt_domain *d; + struct rdt_hw_mondomain *hw_mondom; + struct rdt_mondomain *d; int err;
Please ensure that reverse fir tree order is maintained in all these changes. Reinette