[Cbe-oss-dev] [RFC, PATCH 2/2] OProfile for Cell: Initial profiling support

[Arnd Bergmann]

On Friday 10 November 2006 02:17, Maynard Johnson wrote:
> Add PPU event-based and cycle-based profiling support to Oprofile for Cell.

Christopher told me about a bug you're facing because you pass static data
from a module to rtas:

> +
> +/* pm_signal, need per node copy for smt safeness since multiple cpus
> + * are using this structure in the RTAS calls */
> +static struct pm_signal pm_signal[NR_NODES][OP_MAX_COUNTER];

This is where it's defined. The address will be allocated with vmalloc,
in case you build oprofile as a module.

> +static int
> +rtas_ibm_cbe_perftools(int subfunc, int passthru,
> +                      void *address, unsigned long length)
> +{
> +       u64 paddr = __pa(address);
> +
> +       return rtas_call(pm_rtas_token, 5, 1, NULL, subfunc, passthru,
> +                        paddr >> 32, paddr & 0xffffffff, length);
> +}

Here, you call __pa on the argument, which is only well-defined for
addresses that don't come from vmalloc.

> +static void pm_rtas_reset_signals(u32 node)
> +{
> +       int ret;
> +
> +       pm_signal[node][0].cpu = node;  // fw expects physical cpu #
> +       passthru[node] = PASSTHRU_DISABLE;
> +
> +       ret = rtas_ibm_cbe_perftools(SUBFUNC_RESET, passthru[node],
> +                                    pm_signal[node],
> +                                    sizeof(struct pm_signal));
> +
> +       if (ret)
> +               printk(KERN_WARNING "%s: rtas returned: %d\n",
> +                      __FUNCTION__, ret);
> +}
> +
> +static void pm_rtas_activate_signals(u32 node, u32 count)
> +{
> +       int ret;
> +
> +       if (passthru[node] == PASSTHRU_DISABLE) {
> +               passthru[node] = PASSTHRU_ENABLE;
> +       }
> +
> +       pm_signal[node][0].cpu = node;  // fw expects physical cpu #
> +
> +       ret = rtas_ibm_cbe_perftools(SUBFUNC_ACTIVATE, passthru[node],
> +                                    pm_signal[node],
> +                                    count * sizeof(struct pm_signal));
> +
> +       if (ret)
> +               printk(KERN_WARNING "%s: rtas returned: %d\n",
> +                      __FUNCTION__, ret);
> +}

And these two functions pass the data incorrectly. The easiest
solution would be to have a copy of the pm_signal structure that you
pass on the stack and copy between that and the global copy.

	Arnd <><