> -----Original Message----- > From: Joe Perches [mailto:joe@xxxxxxxxxxx] > Sent: Tuesday, August 16, 2016 3:57 PM > To: Jitendra Bhivare; Christophe JAILLET; Jayamohan Kallickal; Ketan Mukadam > Cc: Bart Van Assche; James E.J. Bottomley; Martin K. Petersen; linux- > scsi@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx > Subject: Re: [PATCH] be2iscsi: Use a more current logging style > > 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. [JB] KERN_ERR messages need to be logged irrespective of the masks. I understand, that in some places, mask is unnecessarily passed. I had made sure to call __beiscsi_log in some places. Can we please keep it that way? So beiscsi_err calls dev_err directly or is replaced with dev_err. It's safe to assume pcidev will be valid for all beiscsi_log calls. Will test your change on my setup before ack'ing. Actually, we too wanted to get rid of BC_/BM_... line# way and replace with ABCD = error identifier. A <category> B <subcategory> CD <error code> But that will be substantial change with some testing requirements. For now, this looks good. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
