[LinuxBIOS] LNXI Merge: lnxi-patch-06/16

[Eric W. Biederman]

Hamish Guthrie <hamish at prodigi.ch> writes:

> Eric,
>
> I totally agree that constructive criticism is in order. Please look at the
> original version of the code I submitted a few months ago, shortly after the
> cpu_relax issue first came into being:

Thank you.

> I do not want to harp on ad infinitum on this, but, there are a number of people
> in the embedded space using LinuxBIOS, and generally there are very few SMP
> devices in the embedded space. I would also just like to ask how many devices
> are out there using LinuxBIOS with SMP? 

With dual cores becoming the path to performance you can't make a new
system that isn't SMP.  From the cluster side most interesting
commodity are dual processor SMP.

> I am sure that I could multiply that
> number by 10 000 to get to the number of embedded uniprocessor devices using
> LinuxBIOS. Right now, I am in the process of rolling out some replacement
> firmware for some 120 000 devices previously running WinCE, which are now
> running Linux with LinuxBIOS as the loader.

Cool.  I like the thought of 100 million devices running LinuxBIOS :)

> Do we need to get to the worst-case scenario and fork LinuxBIOS for
> uniprocessor/SMP> - I would like to think not.

Agreed.

My impression is usually the SMP/Desktop work uses the same hardware as
the embedded systems but a couple of years sooner.  There is also the challenge
that a lot of embedded systems are single shot deals many times embedded
developers don't have a long term interest in the free software project
they work with.  So my impression is that in a 5 years or so we will start
seeing multi core embedded systems.

The most maintainable fix for something like cpu_relax() is to have
one definition that compiles SMP and another definition that works
non-smp.

#ifdef CONFIG_SMP
static inline void cpu_relax(void) 
{
        __asm__ __volatile__ ("rep ; nop");
}
#else
static inline void cpu_relax(void)
{
        /* do nothing */
}
#endif

Although in this particular case I don't know it even makes sense to bracket
cpu_relax();


My point being that putting the #ifdef code in header files or in the
definition of the function being called it is frequently more
maintainable, than having it in the inline body of the function.  Then
so long as calling cpu_relax() is a reasonable thing to do you don't
have to worry about it in that code.

For this particular piece of code cpu_relax() largely overkill because we don't
do much on secondary processors.

With the growing developer base the conflicts between different
developers have become a problem, this is not isolated to embedded
work/SMP work conflicting.  Getting a functioning code review and
release process in place is one of the things we need to talk about at
the LinuxBIOS summit. 

I know Jason tested the build on a least one uniprocessor system but
that may not have tested this code path.

Jason before committing will you please double check that cpu_relax()
has a definition of CONFIG_SMP is 0 or not set?

Eric