Messages in this thread | | | Subject | Re: [PATCH v4 03/21] x86/resctrl: Add domain online callback for resctrl work | From | Xin Hao <> | Date | Tue, 3 May 2022 15:59:26 +0800 |
| |
On 4/17/22 12:06 AM, Xin Hao wrote: > > > On 4/12/22 8:44 PM, James Morse wrote: >> Because domains are exposed to user-space via resctrl, the filesystem >> must update its state when CPU hotplug callbacks are triggered. >> >> Some of this work is common to any architecture that would support >> resctrl, but the work is tied up with the architecture code to >> allocate the memory. >> >> Move domain_setup_mon_state(), the monitor subdir creation call and the >> mbm/limbo workers into a new resctrl_online_domain() call. These bits >> are not specific to the architecture. Grouping them in one function >> allows that code to be moved to /fs/ and re-used by another architecture. >> >> Reviewed-by: Jamie Iles<quic_jiles@quicinc.com> >> Tested-by: Xin Hao<xhao@linux.alibaba.com> >> Reviewed-by: Shaopeng Tan<tan.shaopeng@fujitsu.com> >> Tested-by: Shaopeng Tan<tan.shaopeng@fujitsu.com> >> Tested-by: Cristian Marussi<cristian.marussi@arm.com> >> Signed-off-by: James Morse<james.morse@arm.com> >> --- >> Changes since v2: >> * Fixed kfree(d) rebase artefact. >> >> Changes since v1: >> * Capitalisation >> * Removed inline comment >> * Added to the commit message >> --- >> arch/x86/kernel/cpu/resctrl/core.c | 57 ++++------------------ >> arch/x86/kernel/cpu/resctrl/internal.h | 2 - >> arch/x86/kernel/cpu/resctrl/rdtgroup.c | 65 ++++++++++++++++++++++++-- >> include/linux/resctrl.h | 1 + >> 4 files changed, 69 insertions(+), 56 deletions(-) >> >> diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c >> index 2f87177f1f69..25f30148478b 100644 >> --- a/arch/x86/kernel/cpu/resctrl/core.c >> +++ b/arch/x86/kernel/cpu/resctrl/core.c >> @@ -443,42 +443,6 @@ static int domain_setup_ctrlval(struct rdt_resource *r, struct rdt_domain *d) >> return 0; >> } >> >> -static int domain_setup_mon_state(struct rdt_resource *r, struct rdt_domain *d) >> -{ >> - size_t tsize; >> - >> - if (is_llc_occupancy_enabled()) { >> - d->rmid_busy_llc = bitmap_zalloc(r->num_rmid, GFP_KERNEL); >> - if (!d->rmid_busy_llc) >> - return -ENOMEM; >> - INIT_DELAYED_WORK(&d->cqm_limbo, cqm_handle_limbo); >> - } >> - if (is_mbm_total_enabled()) { >> - tsize = sizeof(*d->mbm_total); >> - d->mbm_total = kcalloc(r->num_rmid, tsize, GFP_KERNEL); >> - if (!d->mbm_total) { >> - bitmap_free(d->rmid_busy_llc); >> - return -ENOMEM; >> - } >> - } >> - if (is_mbm_local_enabled()) { >> - tsize = sizeof(*d->mbm_local); >> - d->mbm_local = kcalloc(r->num_rmid, tsize, GFP_KERNEL); >> - if (!d->mbm_local) { >> - bitmap_free(d->rmid_busy_llc); >> - kfree(d->mbm_total); >> - return -ENOMEM; >> - } >> - } >> - >> - if (is_mbm_enabled()) { >> - INIT_DELAYED_WORK(&d->mbm_over, mbm_handle_overflow); >> - mbm_setup_overflow_handler(d, MBM_OVERFLOW_INTERVAL); >> - } >> - >> - return 0; >> -} >> - >> /* >> * domain_add_cpu - Add a cpu to a resource's domain list. >> * >> @@ -498,6 +462,7 @@ static void domain_add_cpu(int cpu, struct rdt_resource *r) >> struct list_head *add_pos = NULL; >> struct rdt_hw_domain *hw_dom; >> struct rdt_domain *d; >> + int err; >> >> d = rdt_find_domain(r, id, &add_pos); >> if (IS_ERR(d)) { >> @@ -527,21 +492,15 @@ static void domain_add_cpu(int cpu, struct rdt_resource *r) >> return; >> } >> >> - if (r->mon_capable && domain_setup_mon_state(r, d)) { >> - kfree(hw_dom->ctrl_val); >> - kfree(hw_dom->mbps_val); >> - kfree(hw_dom); >> - return; >> - } >> - >> list_add_tail(&d->list, add_pos); >> >> - /* >> - * If resctrl is mounted, add >> - * per domain monitor data directories. >> - */ >> - if (static_branch_unlikely(&rdt_mon_enable_key)) >> - mkdir_mondata_subdir_allrdtgrp(r, d); >> + err = resctrl_online_domain(r, d); >> + if (err) { >> + list_del(&d->list); >> + kfree(hw_dom->ctrl_val); >> + kfree(hw_dom->mbps_val); >> + kfree(hw_dom); >> + } >> } >> >> static void domain_remove_cpu(int cpu, struct rdt_resource *r) >> diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h >> index 8828b5c1b6d2..be48a682dbdb 100644 >> --- a/arch/x86/kernel/cpu/resctrl/internal.h >> +++ b/arch/x86/kernel/cpu/resctrl/internal.h >> @@ -524,8 +524,6 @@ void mon_event_count(void *info); >> int rdtgroup_mondata_show(struct seq_file *m, void *arg); >> void rmdir_mondata_subdir_allrdtgrp(struct rdt_resource *r, >> unsigned int dom_id); >> -void mkdir_mondata_subdir_allrdtgrp(struct rdt_resource *r, >> - struct rdt_domain *d); >> void mon_event_read(struct rmid_read *rr, struct rdt_resource *r, >> struct rdt_domain *d, struct rdtgroup *rdtgrp, >> int evtid, int first); >> diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c >> index 651ff5288e85..53bdc07f9dac 100644 >> --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c >> +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c >> @@ -2565,16 +2565,13 @@ static int mkdir_mondata_subdir(struct kernfs_node *parent_kn, >> * Add all subdirectories of mon_data for "ctrl_mon" groups >> * and "monitor" groups with given domain id. >> */ >> -void mkdir_mondata_subdir_allrdtgrp(struct rdt_resource *r, >> - struct rdt_domain *d) >> +static void mkdir_mondata_subdir_allrdtgrp(struct rdt_resource *r, >> + struct rdt_domain *d) >> { >> struct kernfs_node *parent_kn; >> struct rdtgroup *prgrp, *crgrp; >> struct list_head *head; >> >> - if (!r->mon_capable) >> - return; >> - >> list_for_each_entry(prgrp, &rdt_all_groups, rdtgroup_list) { >> parent_kn = prgrp->mon.mon_data_kn; >> mkdir_mondata_subdir(parent_kn, d, r, prgrp); >> @@ -3236,6 +3233,64 @@ static int __init rdtgroup_setup_root(void) >> return ret; >> } >> >> +static int domain_setup_mon_state(struct rdt_resource *r, struct rdt_domain *d) >> +{ >> + size_t tsize; >> + >> + if (is_llc_occupancy_enabled()) { >> + d->rmid_busy_llc = bitmap_zalloc(r->num_rmid, GFP_KERNEL); >> + if (!d->rmid_busy_llc) >> + return -ENOMEM; >> + } >> + if (is_mbm_total_enabled()) { >> + tsize = sizeof(*d->mbm_total); >> + d->mbm_total = kcalloc(r->num_rmid, tsize, GFP_KERNEL); >> + if (!d->mbm_total) { >> + bitmap_free(d->rmid_busy_llc); >> + return -ENOMEM; >> + } >> + } >> + if (is_mbm_local_enabled()) { >> + tsize = sizeof(*d->mbm_local); >> + d->mbm_local = kcalloc(r->num_rmid, tsize, GFP_KERNEL); >> + if (!d->mbm_local) { >> + bitmap_free(d->rmid_busy_llc); >> + kfree(d->mbm_total); >> + return -ENOMEM; >> + } >> + } >> + >> + return 0; >> +} >> + >> +int resctrl_online_domain(struct rdt_resource *r, struct rdt_domain *d) >> +{ >> + int err; >> + >> + lockdep_assert_held(&rdtgroup_mutex); >> + >> + if (!r->mon_capable) >> + return 0; > > This return value "0" may not quite correct, in code where it is > called, if err = 0, i think if mon_capable is not supported, the > hw_dom->mbps_val also need to be released? if I get this wrong,please > let me know. > > + err = resctrl_online_domain(r, d); > + if (err) { > + list_del(&d->list); > + kfree(hw_dom->ctrl_val); > + kfree(hw_dom->mbps_val); > + kfree(hw_dom); > + } >> + >> + err = domain_setup_mon_state(r, d); >> + if (err) >> + return err; >> + >> + if (is_mbm_enabled()) { >> + INIT_DELAYED_WORK(&d->mbm_over, mbm_handle_overflow); >> + mbm_setup_overflow_handler(d, MBM_OVERFLOW_INTERVAL); >> + } >> + >> + if (is_llc_occupancy_enabled()) >> + INIT_DELAYED_WORK(&d->cqm_limbo, cqm_handle_limbo); >> + >> + /* If resctrl is mounted, add per domain monitor data directories. */ >> + if (static_branch_unlikely(&rdt_mon_enable_key)) >> + mkdir_mondata_subdir_allrdtgrp(r, d); >> + >> + return 0; >> +} >> + >> /* >> * rdtgroup_init - rdtgroup initialization >> * >> diff --git a/include/linux/resctrl.h b/include/linux/resctrl.h >> index 8180c539800d..d512455b4c3a 100644 >> --- a/include/linux/resctrl.h >> +++ b/include/linux/resctrl.h >> @@ -192,5 +192,6 @@ u32 resctrl_arch_get_num_closid(struct rdt_resource *r); >> int resctrl_arch_update_domains(struct rdt_resource *r, u32 closid); >> u32 resctrl_arch_get_config(struct rdt_resource *r, struct rdt_domain *d, >> u32 closid, enum resctrl_conf_type type); >> +int resctrl_online_domain(struct rdt_resource *r, struct rdt_domain *d); >> >> #endif /* _RESCTRL_H */ > -- > Best Regards! > Xin Hao
-- Best Regards! Xin Hao
| |