Messages in this thread Patch in this message | | | Date | Tue, 10 Jan 2023 17:57:39 +0000 | Subject | Re: [PATCH 03/18] x86/resctrl: Create helper for RMID allocation and mondata dir creation | From | James Morse <> |
| |
Hi Fenghua,
On 06/01/2023 03:23, Yu, Fenghua wrote: >> James Morse <james.morse@arm.com> writes: >> >> 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 are 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. >> >> To make a subsequent change that does this easier to read, move the RMID >> allocation and mondata dir creation to a helper.
>> diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c >> b/arch/x86/kernel/cpu/resctrl/rdtgroup.c >> index 9ce4746778f4..841294ad6263 100644 >> --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c >> +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c >> @@ -2868,6 +2868,30 @@ static int rdtgroup_init_alloc(struct rdtgroup *rdtgrp) >> return 0; >> } >> >> +static int mkdir_rdt_prepare_rmid_alloc(struct rdtgroup *rdtgrp) { >> + int ret; >> + >> + if (!rdt_mon_capable) >> + return 0; >> + >> + ret = alloc_rmid(); >> + if (ret < 0) { >> + rdt_last_cmd_puts("Out of RMIDs\n"); >> + return ret; >> + } >> + rdtgrp->mon.rmid = ret; >> + >> + ret = mkdir_mondata_all(rdtgrp->kn, rdtgrp, &rdtgrp- >>> mon.mon_data_kn); >> + if (ret) { >> + rdt_last_cmd_puts("kernfs subdir error\n");
>> + free_rmid(rdtgrp->closid, rdtgrp->mon.rmid);
^^^^^^^^^
>> + return ret; >> + } >> + >> + return 0; >> +} >> + >> static int mkdir_rdt_prepare(struct kernfs_node *parent_kn, >> const char *name, umode_t mode, >> enum rdt_group_type rtype, struct rdtgroup **r) >> @@ -2933,20 +2957,10 @@ static int mkdir_rdt_prepare(struct kernfs_node >> *parent_kn, >> goto out_destroy; >> } >> >> - if (rdt_mon_capable) { >> - ret = alloc_rmid(); >> - if (ret < 0) { >> - rdt_last_cmd_puts("Out of RMIDs\n"); >> - goto out_destroy; >> - } >> - rdtgrp->mon.rmid = ret; >> + ret = mkdir_rdt_prepare_rmid_alloc(rdtgrp); >> + if (ret) >> + goto out_destroy; >> >> - ret = mkdir_mondata_all(kn, rdtgrp, &rdtgrp- >>> mon.mon_data_kn); >> - if (ret) { >> - rdt_last_cmd_puts("kernfs subdir error\n"); >> - goto out_idfree; >> - } >> - } >> kernfs_activate(kn); >> >> /* >> @@ -2954,8 +2968,6 @@ static int mkdir_rdt_prepare(struct kernfs_node >> *parent_kn, >> */ >> return 0; >> >> -out_idfree: >> - free_rmid(rdtgrp->closid, rdtgrp->mon.rmid); >> out_destroy: >> kernfs_put(rdtgrp->kn); >> kernfs_remove(rdtgrp->kn);
> Why not free allocated rmid? Rmid leak without freed.
It's just moved into the helper.
This free_rmid() is here because this function used to allocate the rmid and the mondata as separate steps. Now a helper does that in one go - and cleans up after itself.
If mkdir_rdt_prepare_rmid_alloc() fails, it didn't allocate an rmid .. if it secretly did, it free()s it itself, see the ^-lighted code above.
Thanks,
James
| |