Re: [RFC PATCH] x86: explicit call to mmiotrace in do_page_fault()

[Pekka Paalanen]

On Sat, 9 Feb 2008 10:01:33 -0800
Arjan van de Ven <arjan@xxxxxxxxxxxxx> wrote:

> >  	default n
> >  	help
> >  	  This will build a kernel module called mmiotrace.
> > +	  Making this a built-in is heavily discouraged.
> 
> why is this? Wouldn't it be nice if distros just shipped with this in their kernel by default
> for example?

It's currently not too nice as a built-in. Originally I did have it depend
on 'm' but maybe Ingo removed it? It's not depending on 'm' anymore in
x86/mm git tree Kconfig.

By default it reserves 32 MB/cpu of relay buffers during module init.
It's this big based on experimentation with the nvidia proprietary driver.
There is no other way than unloading the module to free the buffers, or
even to clear them. This means that every time you start the user space
logger application, it gets old data first. Also poking the module
parameters via /sys is not allowed.

So it's not actually broken as far as I know, it's just not nice IMHO.
And I've never tried it as built-in. I don't think anyone ever has.
I do have a plan to make it a nicer built-in, but it's a long way.

> > +#ifdef CONFIG_MMIOTRACE_HOOKS
> > +static pf_handler_func mmiotrace_pf_handler; /* protected by RCU */
> > +static DEFINE_SPINLOCK(mmiotrace_handler_lock);
> >  
> > +int mmiotrace_register_pf(pf_handler_func new_pfh)
> >  {
> > +	int ret = 0;
> >  	unsigned long flags;
> > +	spin_lock_irqsave(&mmiotrace_handler_lock, flags);
> > +	if (mmiotrace_pf_handler)
> > +		ret = -EBUSY;
> > +	else
> > +		mmiotrace_pf_handler = new_pfh;
> > +	spin_unlock_irqrestore(&mmiotrace_handler_lock, flags);
> > +	return ret;
> >  }
> > +EXPORT_SYMBOL_GPL(mmiotrace_register_pf);
> 
> why not use a weak function instead? should be a ton simpler.

Ookay, this is the first time I hear about weak functions, and
that I could accomplish something like this with them.
I'll be googling around. These are completely new beasts to me.

Thanks for the keywords, finally I have a clue to follow.

-- 
Pekka Paalanen
http://www.iki.fi/pq/
--
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/