On Tue, 2007-09-11 at 10:27 -0400, Mathieu Desnoyers wrote:
> * Rusty Russell ([EMAIL PROTECTED]) wrote:
> > On Mon, 2007-09-10 at 20:45 -0400, Mathieu Desnoyers wrote:
> > > Code patching of _live_ SMP code is allowed. This is why I went through
> > > all this trouble on i386.
> >
> > Oh, I was pretty sure it wasn't. OK.
> >
> > So now why three versions of immediate_set()? And why are you using my
> > lock for exclusion? Against what?
> >
>
> If we need to patch code at boot time, when interrupts are still
> disabled (it happens when we parse the kernel arguments for instance),
> we cannot afford to use IPIs to call sync_core() on each cpu, using
> breakpoints/notifier chains could be tricky (because we are very early
> at boot and alternatives or paravirt may not have been applied yet).
Hi Mathieu,
Sure, but why is that the caller's problem? immediate_set() isn't
fastpath, so why not make it do an "if (early_boot)" internally?
> _immediate_set() has been introduced because of the way immediate values
> are used by markers: the linux kernel markers already hold the module
> mutex when they need to update the immediate values. Taking the mutex
> twice makes no sence, so _immediate_set() is used when the caller
> already holds the module mutex.
> Why not just have one immediate_set() which iterates through and fixes
> > up all the references?
>
> (reasons explained above)
>
> > It can use an internal lock if you want to avoid
> > concurrent immediate_set() calls.
> >
>
> An internal lock won't protect against modules load/unload race. We have
> to iterate on the module list.
Sure, but it seems like that's fairly easy to do within module.c:
/* This updates all the immediates even though only one might have
* changed. But it's so rare it's not worth optimizing. */
void module_update_immediates(void)
{
mutex_lock(&module_mutex);
list_for_each_entry(mod, &modules, list)
update_immediates(mod->immediate, mod->num_immediate);
mutex_unlock(&module_mutex);
}
Then during module load you do:
update_immediates(mod->immediate, mod->num_immediate);
Your immediate_update() just becomes:
update_immediates(__start___immediate,
__stop___immediate - __start___immediate);
module_update_immediates();
update_immediates() can grab the immediate_mutex if you want.
> > Why is it easier to patch the sites now than later? Currently it's just
> > churn. You could go back and find them when this mythical patch gets
> > merged into this mythical future gcc version. It could well need a
> > completely different macro style, like "cond_imm(var, code)".
>
> Maybe you're right. My though was that if we have a way to express a
> strictly boolean if() statement that can later be optimized further by
> gcc using a jump rather than a conditionnal branch and currently emulate
> it by using a load immediate/test/branch, we might want to do so right
> now so we don't have to do a second code transition from
> if (immediate_read(&var)) to immediate_if (&var) later. But you might be
> right in that the form could potentially change anyway when the
> implementation would come, although I don't see how.
I was thinking that we might find useful specific cases before we get
GCC support, which archs can override with tricky asm if they wish.
Cheers,
Rusty.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
