[PATCH 2/3] powerpc: Instrument Hypervisor Calls: add wrappers

[Arnd Bergmann]

On Wednesday 14 June 2006 05:52, Mike Kravetz wrote:

>                         unsigned long *out2,            R9
>                         unsigned long *out3);           R10
>   */
> +#ifdef CONFIG_HCALL_STATS
> +_GLOBAL(plpar_hcall_base)
> +#else
>  _GLOBAL(plpar_hcall)
> +#endif
>         HMT_MEDIUM
>  
>         mfcr    r0

The assembly files are already hard to read. Can we do this without
an extra #ifdef in there for every call?
Moreover, when using ctags with your version, looking for plpar_hcall
will find the instrumented version only, which may be extra confusing
when trying to work out the call chain.

> diff -Naupr linux-2.6.17-rc6/arch/powerpc/platforms/pseries/hvCall_inst.c linux-2.6.17-rc6.work/arch/powerpc/platforms/pseries/hvCall_inst.c
> --- linux-2.6.17-rc6/arch/powerpc/platforms/pseries/hvCall_inst.c       1970-01-01 00:00:00.000000000 +0000
> +++ linux-2.6.17-rc6.work/arch/powerpc/platforms/pseries/hvCall_inst.c  2006-06-13 21:39:24.000000000 +0000
...
> +extern long plpar_hcall_base (unsigned long opcode,
> +                 unsigned long arg1,
> +                 unsigned long arg2,
> +                 unsigned long arg3,
> +                 unsigned long arg4,
> +                 unsigned long *out1,
> +                 unsigned long *out2,
> +                 unsigned long *out3);

At the same time, the declarations should go to the header file, as they
are an interface between two files.

> +extern long plpar_hcall_norets_base(unsigned long opcode, ...);
> +
> +long plpar_hcall_norets_C(unsigned long opcode,
> +                               unsigned long arg1,
> +                               unsigned long arg2,
> +                               unsigned long arg3,
> +                               unsigned long arg4,
> +                               unsigned long arg5,
> +                               unsigned long arg6)
> +{
> +       long rc;
> +       unsigned long t_before = mfspr(SPRN_PURR);
> +
> +       rc = plpar_hcall_norets_base(opcode, arg1, arg2, arg3, arg4, arg5,
> +                                    arg6);
> +
> +       update_stats(opcode, t_before);
> +       return rc;
> +}

Maybe a little comment hinting about the magic going on underneath this?

> diff -Naupr linux-2.6.17-rc6/include/asm-powerpc/hvcall.h linux-2.6.17-rc6.work/include/asm-powerpc/hvcall.h
> --- linux-2.6.17-rc6/include/asm-powerpc/hvcall.h       2006-06-13 21:38:45.000000000 +0000
> +++ linux-2.6.17-rc6.work/include/asm-powerpc/hvcall.h  2006-06-13 21:39:24.000000000 +0000
> @@ -292,6 +292,13 @@ long plpar_hcall_9arg_9ret(unsigned long
>                            unsigned long *out8,
>                            unsigned long *out9);
>  
> +#ifdef CONFIG_HCALL_STATS
> +struct hcall_stats {
> +       unsigned long   num_calls;
> +       unsigned long   total_time;
> +};
> +#endif /* CONFIG_HCALL_STATS */
> +

That really shouldn't need #ifdef. You could move it to hvCall_inst.c
though, since it appears to be used only in there (patch 3/3 hasn't
come through yet, don't know if it's used elsewhere in that).

	Arnd <><