Re: [PATCH 3/3] x86: oprofile: fix oprofile_arch_init behaviour onfailure

From: Robert Richter
Date: Mon Aug 30 2010 - 05:26:00 EST


On 29.08.10 14:52:01, Will Deacon wrote:
> The OProfile driver no longer calls oprofile_arch_exit when
> oprofile_arch_init return failure.
>
> This patch fixes the x86 implementation of oprofile_arch_init
> to ensure that op_nmi_exit is called if necessary.
>
> Cc: Robert Richter <robert.richter@xxxxxxx>
> Cc: Matt Fleming <matt@xxxxxxxxxxxxxxxxx>
> Cc: Peter Zijlstra <peterz@xxxxxxxxxxxxx>
> Cc: Ingo Molnar <mingo@xxxxxxx>
> Signed-off-by: Will Deacon <will.deacon@xxxxxxx>
> ---
> arch/x86/oprofile/init.c | 26 +++++++++++++++-----------
> 1 files changed, 15 insertions(+), 11 deletions(-)
>

> int __init oprofile_arch_init(struct oprofile_operations *ops)
> {
> int ret;
>
> - ret = -ENODEV;
> -
> -#ifdef CONFIG_X86_LOCAL_APIC
> ret = op_nmi_init(ops);
> -#endif
> -#ifdef CONFIG_X86_IO_APIC
> if (ret < 0)
> +#ifdef CONFIG_X86_IO_APIC
> ret = op_nmi_timer_init(ops);
> +#else
> + return ret;
> #endif
> +
> ops->backtrace = x86_backtrace;
>
> + if (ret < 0)
> + op_nmi_exit();
> +

I don't see why we have to do this. All init functions above clean up
properly on failure. If op_nmi_init() succeeds we don't call
op_nmi_timer_init(), so we don't need to free it it either.

-Robert

> return ret;
> }

--
Advanced Micro Devices, Inc.
Operating System Research Center

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/