Re: [PATCH 8/9] perf/breakpoint: Split breakpoint "check" and "commit"

[Frederic Weisbecker]

On Wed, May 09, 2018 at 11:17:03AM +0200, Peter Zijlstra wrote:
> On Sun, May 06, 2018 at 09:19:54PM +0200, Frederic Weisbecker wrote:
> >  arch/arm/include/asm/hw_breakpoint.h     |  5 ++++-
> >  arch/arm/kernel/hw_breakpoint.c          | 22 +++-------------------
> >  arch/arm64/include/asm/hw_breakpoint.h   |  5 ++++-
> >  arch/arm64/kernel/hw_breakpoint.c        | 22 +++-------------------
> >  arch/powerpc/include/asm/hw_breakpoint.h |  5 ++++-
> >  arch/powerpc/kernel/hw_breakpoint.c      | 22 +++-------------------
> >  arch/sh/include/asm/hw_breakpoint.h      |  5 ++++-
> >  arch/sh/kernel/hw_breakpoint.c           | 22 +++-------------------
> >  arch/x86/include/asm/hw_breakpoint.h     |  5 ++++-
> >  arch/x86/kernel/hw_breakpoint.c          | 23 +++--------------------
> >  arch/xtensa/include/asm/hw_breakpoint.h  |  5 ++++-
> >  arch/xtensa/kernel/hw_breakpoint.c       | 22 +++-------------------
> 
> Because of those ^,
> 
> >  kernel/events/hw_breakpoint.c            | 11 ++++++-----
> 
> would it not make sense to have a prelimenary patch doing something
> like:
> 
> __weak int hw_breakpoint_arch_check(struct perf_event *bp)
> {
> 	return arch_validate_hwbkpt_settings(bp);
> }

So eventually I fear I can't do that, due to linking order.

Say I convert x86 to implement hw_breakpoint_arch_check(), so I
remove arch_validate_hwbkpt_settings(). On build time, the weak version
is still compiled and can't find a declaration for arch_validate_hwbkpt_settings().

I tried to keep the declaration while the definition has been removed but
it seems the weak version is linked first before it gets later replaced by
the overriden arch version. So I get a build error.

I could keep arch_validate_hwbkpt_settings() around on all archs and remove it in
the end with the weak version but that would defeat the purpose of removing
the mid-state in the current patch.