Re: [PATCH 39/53] IB/qib: Add qib_trace.c

Ralph Campbell
2009-12-02T00:10:18Z

On Thu, 2009-11-19 at 16:14 -0800, Roland Dreier wrote:
> Seems all this infrastructure could be profitably ripped out and
> replaced with the more powerful and flexible ftrace events stuff...
>
> > +#ifndef ATOMIC_NOTIFIER_INIT
> > +/*
> > + * Some backports don't have this, so define here for now.
> > + * We may not want to keep this for upstream. If we do, we'll
> > + * do this via the backports at that time.
> > + */
> > +#define atomic_notifier_chain_register notifier_chain_register
> > +#define atomic_notifier_chain_unregister notifier_chain_unregister
> > +#endif
>
> Doesn't seem like this was meant to go into the upstream submission?
> Seems like it would be worth going over the patches again and cleaning
> up everything that is solely for compatibility with old kernels.
>
> I don't think random PCI device drivers hooking into the panic notifier
> chain is really a good idea, so probably ripping this out for now is the
> best thing to do.
>
> - R.

I looked at ftrace and I see two problems with using it.

1) It requires CONFIG_TRACING to be set which compiles code -pg
so that function prologue code calls mcount. What Linux
distributions will have this configured as the default?

2) We have to backport the ftrace code to whatever OFED kernels
will be supported for OFED-1.6 which could be painful.

What if we eliminate the notifier_chain_register?