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

[Manish Ahuja <ahuja@austin.ibm.com>]

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 <michael@ellerman.id.au> wrote:
>> On Thu, 2008-02-28 at 18:24 -0600, Manish Ahuja wrote:
> 
>>  > +
>>  > +/* 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