Re: [PATCH 3/7 v4] CPER: Adjust code flow of some functions

["Chen, Gong" <gong.chen@linux.intel.com>]

[-- Attachment #1: Type: text/plain, Size: 1507 bytes --]

On Thu, May 22, 2014 at 12:52:42PM +0200, Borislav Petkov wrote:
> > > > +const char *cper_mem_err_type_str(unsigned int etype)
> > > >  {
> > > > -	if (mem->validation_bits & CPER_MEM_VALID_ERROR_STATUS)
> > > > -		printk("%s""error_status: 0x%016llx\n", pfx, mem->error_status);
> > > > -	if (mem->validation_bits & CPER_MEM_VALID_PA)
> > > > -		printk("%s""physical_address: 0x%016llx\n",
> > > > -		       pfx, mem->physical_addr);
> > > > -	if (mem->validation_bits & CPER_MEM_VALID_PA_MASK)
> > > > -		printk("%s""physical_address_mask: 0x%016llx\n",
> > > 
> > > The physical address mask is still not part of the tracepoint as a u8 as
> > > we talked.
> > > 
> > I thought out discussion is only for trace part. But it is OK to me to
> > make whole style aligned.
> 
> No, I'm not talking about style - I'm talking about adding the physical address
> mask to the tracepoint call:

If so, it has been there already. Maybe you should check patch 5/7. I merge
pa/pa_mask into pa_info as a whole to avoid too much calculation/logic in
trace.

> +TRACE_EVENT(extlog_mem_event,
> +       TP_PROTO(u32 etype,
> +                const uuid_le *fru_id,
> +                u64 error_count,
> 
> Btw, is that the error_count we're reporting?? You surely can't claim
> that we'll ever report 2^64-1 errors, right?
> 
> I'd make that u32 and I'd call it
> 
> 		u32 error_number;
Fine.
> 
> as it is a counter we're incrementing.
> 
> +                u32 severity,
Fine.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]