Messages in this thread | | | Date | Thu, 8 Sep 2022 18:01:04 +0100 | Subject | Re: [PATCH v6 04/21] x86/resctrl: Group struct rdt_hw_domain cleanup | From | James Morse <> |
| |
Hi Hao Xin,
On 07/09/2022 07:28, haoxin wrote: > > 在 2022/9/2 下午11:48, James Morse 写道: >> domain_add_cpu() and domain_remove_cpu() need to kfree() the child >> arrays that were allocated by domain_setup_ctrlval(). >> >> As this memory is moved around, and new arrays are created, adjusting >> the error handling cleanup code becomes noisier. >> >> To simplify this, move all the kfree() calls into a domain_free() helper. >> This depends on struct rdt_hw_domain being kzalloc()d, allowing it to >> unconditionally kfree() all the child arrays.
>> diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c >> index 25f30148478b..e37889f7a1a5 100644 >> --- a/arch/x86/kernel/cpu/resctrl/core.c >> +++ b/arch/x86/kernel/cpu/resctrl/core.c >> @@ -414,6 +414,13 @@ void setup_default_ctrlval(struct rdt_resource *r, u32 *dc, u32 *dm) >> } >> } >> +static void domain_free(struct rdt_hw_domain *hw_dom)
> add inline ?
It's best to let the compiler decide this. As this is in a C file, and is declared static, the compiler is free to duplicate and inline this function as it sees fit. The inline keyword would only be needed if this were in a header file.
Looking at the built object file - the compiler chose not to duplicate this into the two callers, presumably because of the size of the function.
Unless its relied on for correctness, or is a performance sensitive path, its best to let the compiler make its own decision here.
Thanks,
James
>> +{ >> + kfree(hw_dom->ctrl_val); >> + kfree(hw_dom->mbps_val); >> + kfree(hw_dom); >> +} >> + >> static int domain_setup_ctrlval(struct rdt_resource *r, struct rdt_domain *d) >> { >> struct rdt_hw_resource *hw_res = resctrl_to_arch_res(r);
| |