On Mon, Nov 16, 2015 at 09:04:53AM +0100, SF Markus Elfring wrote: > From: Markus Elfring <elfring@xxxxxxxxxxxxxxxxxxxxx> > 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@xxxxxxxxxxxxxxxxxxxxx> > --- > 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? -- To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
