Re: [PATCH 20/21] amd64_edac: add DRAM error injection logic using sysfs

[Doug Thompson <norsk5@yahoo.com>]

--- On Thu, 4/30/09, Ingo Molnar <mingo@elte.hu> wrote:

> From: Ingo Molnar <mingo@elte.hu>
> Subject: Re: [PATCH 20/21] amd64_edac: add DRAM error injection logic using sysfs
> To: "Doug Thompson" <norsk5@yahoo.com>
> Cc: "Borislav Petkov" <borislav.petkov@amd.com>, akpm@linux-foundation.org, greg@kroah.com, tglx@linutronix.de, hpa@zytor.com, dougthompson@xmission.com, linux-kernel@vger.kernel.org
> Date: Thursday, April 30, 2009, 2:34 AM
> 
> * Doug Thompson <norsk5@yahoo.com>
> wrote:
> 
> > 
> > I believe I failed to reply to ALL and replied only to
> the sender
> > 
> > doug t
> > 
> > --- On Wed, 4/29/09, Ingo Molnar <mingo@elte.hu>
> wrote:
> > 
> > > From: Ingo Molnar <mingo@elte.hu>
> > > Subject: Re: [PATCH 20/21] amd64_edac: add DRAM
> error injection logic using sysfs
> > > To: "Borislav Petkov" <borislav.petkov@amd.com>
> > > Cc: akpm@linux-foundation.org,
> greg@kroah.com, tglx@linutronix.de,
> hpa@zytor.com, dougthompson@xmission.com,
> linux-kernel@vger.kernel.org
> > > Date: Wednesday, April 29, 2009, 12:17 PM
> > > 
> > > * Borislav Petkov <borislav.petkov@amd.com>
> > > wrote:
> > > 
> > > > From: Doug Thompson <dougthompson@xmission.com>
> > > > 
> > > > Signed-off-by: Doug Thompson <dougthompson@xmission.com>
> > > > Signed-off-by: Borislav Petkov <borislav.petkov@amd.com>
> > > > ---
> > > >  drivers/edac/amd64_edac.c |  287
> > > +++++++++++++++++++++++++++++++++++++++++++++
> > > >  1 files changed, 287 insertions(+), 0
> > > deletions(-)
> > > > 
> > > > diff --git a/drivers/edac/amd64_edac.c
> > > b/drivers/edac/amd64_edac.c
> > > > index b1a7e8c..4d1076f 100644
> > > > --- a/drivers/edac/amd64_edac.c
> > > > +++ b/drivers/edac/amd64_edac.c
> > > > @@ -4621,3 +4621,290 @@ static ssize_t
> > > amd64_hole_show(struct mem_ctl_info *mci, char
> *data)
> > > >  
> > > >  #endif    /* DEBUG */
> > > >  
> > > > +#ifdef
> CONFIG_EDAC_AMD64_OPTERON_ERROR_INJECTION
> > > 
> > > this should be in a separate .c file under
> > > drivers/edac/amd64/. 
> > > 
> > > Introducing large #ifdef sections like that is
> not very
> > > clean. The 
> > > amd64_edac.c file is _way_ too large at more than
> 5000
> > > lines of 
> > > code.
> > > 
> > >     Ingo
> > 
> > If we broke this into a separate files, then there
> would be TWO 
> > (2) files: 1 for the source code of the routines and a
> 1 for the 
> > table entries which reference those routines. Is that
> then 
> > acceptable as well?
> > 
> > Same pattern applies to the DEBUG functions Info
> refers to in 
> > another thread: 2 separate files would be required as
> well.
> > 
> > 2 files for Error Injection code
> > 2 files for DEBUG controls
> > 1 files for text mapping
> > 
> > and I assume all these would be included via an
> #include statement 
> > at their appropriate locations
> 
> A Makefile might be more natural i think - that way the
> #ifdef turns 
> into a makefile rule?
> 
>     Ingo
> 

OK, yes for the separate and standalone functions themselves, but NOT for the function pointer table entries, which also share the table with the DEBUG function pointers and possibly function pointers to code that is not bracketed by #ifdefs 

The function pointer table itself is/can be composed of several entries, not just of the injection or debug kind. In this instance, that is what is there, but I would prefer to allow for future additions not tied to injection and/or debug origins nor of #ifdef bracketing

doug t