Messages in this thread | | | Date | Mon, 16 Nov 2015 10:17:58 +0100 | From | Peter Zijlstra <> | Subject | Re: [PATCH] x86: AMD-perf_event: Delete unnecessary checks before the function call "free_percpu" |
| |
On Mon, Nov 16, 2015 at 09:04:53AM +0100, SF Markus Elfring wrote: > From: Markus Elfring <elfring@users.sourceforge.net> > Date: Mon, 16 Nov 2015 09:00:20 +0100 > > The free_percpu() function tests whether its argument is NULL and then > returns immediately. Thus the test around the calls is not needed. > > This issue was detected by using the Coccinelle software. > > Signed-off-by: Markus Elfring <elfring@users.sourceforge.net> > --- > arch/x86/kernel/cpu/perf_event_amd_uncore.c | 6 ++---- > 1 file changed, 2 insertions(+), 4 deletions(-) > > diff --git a/arch/x86/kernel/cpu/perf_event_amd_uncore.c b/arch/x86/kernel/cpu/perf_event_amd_uncore.c > index cc6cedb..240ecee 100644 > --- a/arch/x86/kernel/cpu/perf_event_amd_uncore.c > +++ b/arch/x86/kernel/cpu/perf_event_amd_uncore.c > @@ -588,11 +588,9 @@ fail_online: > fail_l2: > if (cpu_has_perfctr_nb) > perf_pmu_unregister(&amd_nb_pmu); > - if (amd_uncore_l2) > - free_percpu(amd_uncore_l2); > + free_percpu(amd_uncore_l2); > fail_nb: > - if (amd_uncore_nb) > - free_percpu(amd_uncore_nb); > + free_percpu(amd_uncore_nb);
So I'm really in two minds about such patches; yes its correct. But at the same time; this isn't a performance critical piece of code and the additional condition isn't hurting anything.
Furthermore, I find the explicit test conceptually easier than remembering that kfree() works this way (while many other resource freeing functions do not).
And in error paths -- which aren't our best code by far -- obvious safe is far preferred to clever.
Ingo, preference?
| |