Re: [PATCH] be2iscsi: Use a more current logging style

[Joe Perches]

On Tue, 2016-08-16 at 11:32 +0530, Jitendra Bhivare wrote:
> Thanks Joe for taking this up. It has been pending for long time from our
> side.

Thanks, not a problem, it took ~10 minutes.

There was a bit of an issue about your reply though.

First there was ~50 k of quoted stuff without any content

> [ hundreds and hundreds of quoted lines ]

and then this happened:

> > diff --git a/drivers/scsi/be2iscsi/be_main.h
> b/drivers/scsi/be2iscsi/be_main.h
> > 
> > index aa9c682..7cce6e3 100644
> > --- a/drivers/scsi/be2iscsi/be_main.h
> > +++ b/drivers/scsi/be2iscsi/be_main.h
> > @@ -1081,15 +1081,19 @@ struct hwi_context_memory {
> >  #define BEISCSI_LOG_CONFIG	0x0020	/* CONFIG Code Path */
> >  #define BEISCSI_LOG_ISCSI	0x0040	/* SCSI/iSCSI Protocol related
> Logs */
> > 
> > 
> > -#define __beiscsi_log(phba, level, fmt, ...)				\
> > -	shost_printk(level, phba->shost, fmt, ##__VA_ARGS__)
> > -
> > -#define beiscsi_log(phba, level, mask, prefix, fmt, ...)		\
> > +#define beiscsi_printk(level, phba, mask, fmt, ...)			\
> >  do {									\
> > -	uint32_t log_value = phba->attr_log_enable;			\
> > -	if (((mask) & log_value) || (level[1] <= '3'))			\
> > -		__beiscsi_log(phba, level, prefix "_%d: " fmt,		\
> > -			      __LINE__, ##__VA_ARGS__);			\
> > +	if ((mask) & (phba)->attr_log_enable)				\
> > +		shost_printk(level, phba->shost,			\
> [JB] PCI dev_printk would be more useful with SCSI host_no included by
> default in the message.

This is a good note that seems simple enough, but I almost
missed this.

Given the reply at the top and the _very_ long uncommented
quoted block, I just about assumed it was a useless block
quote that you didn't bother to trim.

Please make it easier to find your replies and notes by
deleting irrelevant quoted stuff.

Also, I think I misread the code.

The original code is <= '3' i.e.: show all KERN_ERR.
That is not correct in the new code.

I don't know the code well and don't have a test bed with
the hardware.

Is it possible for a beiscsi_<level> message to be called
before phba->pcidev is set to a valid value in
beiscsi_hba_alloc?   It appears the code is careful to
only use dev_<level> logging calls before probe.