Re: [patch 1/8] Immediate Values - Global Modules List and Module Mutex

[Rusty Russell <rusty@rustcorp.com.au>]

On Tue, 2007-09-18 at 09:41 -0400, Mathieu Desnoyers wrote:
> * Rusty Russell (rusty@rustcorp.com.au) wrote:
> > On Fri, 2007-09-14 at 11:32 -0400, Mathieu Desnoyers wrote:
> > > * Rusty Russell (rusty@rustcorp.com.au) wrote:
> > > > Alternatively, if you called it "immediate_init" then the semantics
> > > > change slightly, but are more obvious (ie. only use this when the value
> > > > isn't being accessed yet).  But it can't be __init then anyway.
> > > > 
> > > 
> > > I think your idea is good. immediate_init() could be used to update the
> > > immediate values at boot time _and_ at module load time, and we could
> > > use an architecture specific arch_immediate_update_init() to support it.
> > 
> > Right.
> > 
> > > As for "when" to use this, it should be used at boot time when
> > > interrupts are still disabled, still running in UP. It can also be used
> > > at module load time before any of the module code is executed, as long
> > > as the module code pages are writable (which they always are, for
> > > now..). Therefore, the flag seems inappropriate for module load
> > > arch_immediate_update_init. It cannot be put in __init section neither
> > > though if we use it like this.
> > 
> > I think from a user's POV it would be nice to have a 1:1 mapping with
> > normal initialization semantics (ie. it will work as long as you don't
> > access this value until initialized).  And I think this would be the
> > case.  eg:
> > 
> >         int foo_func(void)
> >         {
> >         	if (immediate_read(&some_immediate))
> >         		return 0;
> >         	...
> >         }
> >         
> >         int some_init(void)
> >         {
> >         	immediate_init(some_immediate, 0);
> >         	register_foo(foo_func);
> >         	...
> >         }
> > 
> 
> There are other considerations that differs between the boot-time case
> and the general "init" case: the write-protection flag must be
> cleared-saved/restored when the kernel is running to patch read-only
> text, but we don't want to modify cr0 at early boot on i386 because
> paravirt is not executed yet (at boot time, pages are not
> write-protected yet).
> 
> And I am not sure that it buys us anything to create an immediate_init()
> when we can do exactly the same job with immediate_set. Yes, it might be
> a bit slower, but we are not on a fast path.

Good points.  Well I'd say hiding it all behind a friendly
"immediate_set()" interface is the best option then.

> > OK, but can you justify the use of immediates within the nmi or mce
> > handlers?  They don't strike me as useful candidates for optimization.
> 
> Yes, immediate values are used by the Linux Kernel Markers, which
> instrument many code paths, including functions called from nmi and mce
> contexts (including printk).

Is this really worth worrying about?  Isn't there already a problem with
printk() in nmi?

Cheers,
Rusty.