Re: [PATCH 2/8] pseries: phyp dump: reserve-release proof-of-concept

If Mike and Paul are okay, then I will leave this bit as is and fix all 
other issues and comments.

Thanks,
Manish



Linas Vepstas wrote:

On 10/03/2008, Michael Ellerman [off-list ref] wrote:

quoted

On Thu, 2008-02-28 at 18:24 -0600, Manish Ahuja wrote:

quoted

 > +
 > +/* Global, used to communicate data between early boot and late boot */
 > +static struct phyp_dump phyp_dump_global;
 > +struct phyp_dump *phyp_dump_info = &phyp_dump_global;

I don't see the point of this. You have a static (ie. non-global) struct
 called phyp_dump_global, then you create a pointer to it and pass that
 around.

I did this. This is a style used to minimize disruption due to future
design changes. Basically, the idea is that, at some later time, for
some unknown reason, we decide that this structure shouldn't
be global, or maybe shouldn't be statically allocated, or maybe
should be per-cpu, or who knows.  By creating a pointer, and
just passing that around, you isolate other code from this change.

I learned this trick after spending too many months of my life hunting
down globals and replacing them by dynamically allocated structs.
Its a long and painful process, on many levels, often requiring major
code restructuring.  Code that touches globals directly is often
poorly thought out, designed.  But going in the opposite direction
is easy: if your code always passes everything it needs as args
to subroutines,  then you are free & clear ... if one of those args
just happens to be a pointer to a global, there's no loss (not even
a performance loss -- the arg passing overhead is about the same
as a global TOC lookup!)

So it may look weird if you're not used to seeing it; but the alternative
is almost always worse.

--linas