Re: [PATCH EDACv16 1/2] edac: Change internal representation to work with layers

[Borislav Petkov <bp@amd64.org>]

On Fri, Apr 27, 2012 at 04:24:28PM -0300, Mauro Carvalho Chehab wrote:
> Em 27-04-2012 15:11, Luck, Tony escreveu:
> >>>> +	for (i = 0; i < dimm->mci->n_layers; i++) {
> >>>> +		printk(KERN_CONT "%d", dimm->location[i]);
> >>>> +		if (i < dimm->mci->n_layers - 1)
> >>>> +			printk(KERN_CONT ".");
> >>>> +	}
> >>>> +	printk(KERN_CONT "\n");
> >>>
> >>> This looks hacky but I don't have a good suggestion what to do instead
> >>> here. Maybe snprintf into a complete string which you can issue with
> >>> debugf4()...
> >>
> >> This is not hacky. There are several places at the Kernel doing loops like
> >> that. Look, for example, at lib/hexdump.c (without KERN_CONT, as this
> >> macro was added later - probably to avoid checkpatch.pl complains).
> > 
> > There is some benefit to "one printk == one output line" ... it means
> > that console output will not be (as) jumbled if multiple cpus are
> > printk'ing at the same time.
> 
> Ok, but this message only appears when all the conditions below are met:
> 	- the driver is compiled with EDAC_DEBUG;
> 	- the edac_core is modprobed with edac_debug_level=4;
> 	- during the driver modprobe, when the EDAC driver is being registered.

That means nothing.

> Even on several-core machines, those messages won't mangle, in practice.
> 
> Let's not over-design a simple debug message.

No, let's design a simple debug message correctly, regardless of when it
appears.

> 
> Regards,
> Mauro
> 

-- 
Regards/Gruss,
Boris.

Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
GM: Alberto Bozzo
Reg: Dornach, Landkreis Muenchen
HRB Nr. 43632 WEEE Registernr: 129 19551