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

From: Joe Perches
Date: Tue Aug 16 2016 - 06:27:15 EST


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.