lkml.org 
[lkml]   [2015]   [Nov]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [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?


\
 
 \ /
  Last update: 2015-11-16 10:41    [W:0.462 / U:7.048 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site