From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753435AbcHQDun (ORCPT ); Tue, 16 Aug 2016 23:50:43 -0400 Received: from mail-ua0-f182.google.com ([209.85.217.182]:33662 "EHLO mail-ua0-f182.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751284AbcHQDul (ORCPT ); Tue, 16 Aug 2016 23:50:41 -0400 From: Jitendra Bhivare References: <1471191843.4075.39.camel@perches.com> <1471343225.4075.153.camel@perches.com> In-Reply-To: <1471343225.4075.153.camel@perches.com> MIME-Version: 1.0 X-Mailer: Microsoft Outlook 14.0 Thread-Index: AQHX6pvz/xpP32XrUT6+Qe8Qa6SidgJt00hMApmME0MB4OAgVaAJUetg Date: Wed, 17 Aug 2016 09:20:39 +0530 Message-ID: <6716f28ca4aa8580e87f6ca68ba5199e@mail.gmail.com> Subject: RE: [PATCH] be2iscsi: Use a more current logging style To: Joe Perches , Christophe JAILLET , Jayamohan Kallickal , Ketan Mukadam Cc: Bart Van Assche , "James E.J. Bottomley" , "Martin K. Petersen" , linux-scsi@vger.kernel.org, linux-kernel@vger.kernel.org Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from quoted-printable to 8bit by mail.home.local id u7H3onHB013964 > -----Original Message----- > From: Joe Perches [mailto:joe@perches.com] > 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@vger.kernel.org; linux-kernel@vger.kernel.org > 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_ 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_ 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 B CD But that will be substantial change with some testing requirements. For now, this looks good.