Messages in this thread Patch in this message | | | Date | Mon, 14 Aug 2023 17:50:41 -0700 | Subject | Re: [PATCH v5 04/24] x86/resctrl: Move rmid allocation out of mkdir_rdt_prepare() | From | Fenghua Yu <> |
| |
Hi, James,
On 7/28/23 09:42, James Morse wrote: > RMID are allocated for each monitor or control group directory, because > each of these needs its own RMID. For control groups, > rdtgroup_mkdir_ctrl_mon() later goes on to allocate the CLOSID. > > MPAM's equivalent of RMID is not an independent number, so can't be > allocated until the CLOSID is known. An RMID allocation for one CLOSID > may fail, whereas another may succeed depending on how many monitor > groups a control group has. > > The RMID allocation needs to move to be after the CLOSID has been > allocated. > > Move the RMID allocation out of mkdir_rdt_prepare() to occur in its caller, > after the mkdir_rdt_prepare() call. This allows the RMID allocator to > know the CLOSID. > > Tested-by: Shaopeng Tan <tan.shaopeng@fujitsu.com> > Signed-off-by: James Morse <james.morse@arm.com> > --- > Changes since v2: > * Moved kernfs_activate() later to preserve atomicity of files being visible > --- > arch/x86/kernel/cpu/resctrl/rdtgroup.c | 35 +++++++++++++++++++------- > 1 file changed, 26 insertions(+), 9 deletions(-) > > diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c > index e7178bbbd30f..7c5cfb373d03 100644 > --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c > +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c > @@ -3189,6 +3189,12 @@ static int mkdir_rdt_prepare_rmid_alloc(struct rdtgroup *rdtgrp) > return 0; > } > > +static void mkdir_rdt_prepare_rmid_free(struct rdtgroup *rgrp) > +{ > + if (rdt_mon_capable) > + free_rmid(rgrp->closid, rgrp->mon.rmid); > +} > + > static int mkdir_rdt_prepare(struct kernfs_node *parent_kn, > const char *name, umode_t mode, > enum rdt_group_type rtype, struct rdtgroup **r) > @@ -3254,12 +3260,6 @@ static int mkdir_rdt_prepare(struct kernfs_node *parent_kn, > goto out_destroy; > } > > - ret = mkdir_rdt_prepare_rmid_alloc(rdtgrp); > - if (ret) > - goto out_destroy; > - > - kernfs_activate(kn); > - > /* > * The caller unlocks the parent_kn upon success. > */ > @@ -3278,7 +3278,6 @@ static int mkdir_rdt_prepare(struct kernfs_node *parent_kn, > static void mkdir_rdt_prepare_clean(struct rdtgroup *rgrp) > { > kernfs_remove(rgrp->kn); > - free_rmid(rgrp->closid, rgrp->mon.rmid); > rdtgroup_remove(rgrp); > } > > @@ -3300,12 +3299,21 @@ static int rdtgroup_mkdir_mon(struct kernfs_node *parent_kn, > prgrp = rdtgrp->mon.parent; > rdtgrp->closid = prgrp->closid; > > + ret = mkdir_rdt_prepare_rmid_alloc(rdtgrp); > + if (ret) { > + mkdir_rdt_prepare_clean(rdtgrp); > + goto out_unlock; > + } > + > + kernfs_activate(rdtgrp->kn); > + > /* > * Add the rdtgrp to the list of rdtgrps the parent > * ctrl_mon group has to track. > */ > list_add_tail(&rdtgrp->mon.crdtgrp_list, &prgrp->mon.crdtgrp_list); > > +out_unlock: > rdtgroup_kn_unlock(parent_kn); > return ret; > } > @@ -3336,10 +3344,17 @@ static int rdtgroup_mkdir_ctrl_mon(struct kernfs_node *parent_kn, > ret = 0; > > rdtgrp->closid = closid; > - ret = rdtgroup_init_alloc(rdtgrp); > - if (ret < 0) > + > + ret = mkdir_rdt_prepare_rmid_alloc(rdtgrp); > + if (ret) > goto out_id_free;
Is it better to change "out_id_free" to "out_closid_free"? It's not confused to name it "out_id_free" because only closid is freed. But this patch introduces new "rmid" free. So it's better to rename the label to "out_closid_free" and it matches the following "out_rmid_free" as well.
> > + kernfs_activate(rdtgrp->kn); > + > + ret = rdtgroup_init_alloc(rdtgrp); > + if (ret < 0) > + goto out_rmid_free; > + > list_add(&rdtgrp->rdtgroup_list, &rdt_all_groups); > > if (rdt_mon_capable) { > @@ -3358,6 +3373,8 @@ static int rdtgroup_mkdir_ctrl_mon(struct kernfs_node *parent_kn, > > out_del_list: > list_del(&rdtgrp->rdtgroup_list); > +out_rmid_free: > + mkdir_rdt_prepare_rmid_free(rdtgrp); > out_id_free:
s/out_id_free/out_closid_free/?
> closid_free(closid); > out_common_fail:
Thanks.
-Fenghua
| |