[02/15] qla2xxx: Change post del message from debug level to log level

Message ID 20201201082730.24158-3-njavali@marvell.com
State New
Headers show
Series
  • qla2xxx bug fixes
Related show

Commit Message

Nilesh Javali Dec. 1, 2020, 8:27 a.m.
From: Saurav Kashyap <skashyap@marvell.com>

Change the message debug level.

Signed-off-by: Saurav Kashyap <skashyap@marvell.com>
Signed-off-by: Nilesh Javali <njavali@marvell.com>
---
 drivers/scsi/qla2xxx/qla_gs.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

Comments

Saurav Kashyap Dec. 1, 2020, 4:05 p.m. | #1
Hi Himasnhu,
Comments inline

> -----Original Message-----
> From: Himanshu Madhani <himanshu.madhani@oracle.com>
> Sent: Tuesday, December 1, 2020 9:15 PM
> To: Nilesh Javali <njavali@marvell.com>
> Cc: Martin K . Petersen <martin.petersen@oracle.com>; linux-
> scsi@vger.kernel.org; GR-QLogic-Storage-Upstream <GR-QLogic-Storage-
> Upstream@marvell.com>
> Subject: Re: [PATCH 02/15] qla2xxx: Change post del message from debug level
> to log level
> 
> 
> 
> > On Dec 1, 2020, at 2:27 AM, Nilesh Javali <njavali@marvell.com> wrote:
> >
> > From: Saurav Kashyap <skashyap@marvell.com>
> >
> > Change the message debug level.
> >
> > Signed-off-by: Saurav Kashyap <skashyap@marvell.com>
> > Signed-off-by: Nilesh Javali <njavali@marvell.com>
> > ---
> > drivers/scsi/qla2xxx/qla_gs.c | 8 ++++----
> > 1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/scsi/qla2xxx/qla_gs.c b/drivers/scsi/qla2xxx/qla_gs.c
> > index e28c4b7ec55f..391ac75e3de3 100644
> > --- a/drivers/scsi/qla2xxx/qla_gs.c
> > +++ b/drivers/scsi/qla2xxx/qla_gs.c
> > @@ -3558,10 +3558,10 @@ void qla24xx_async_gnnft_done(scsi_qla_host_t
> *vha, srb_t *sp)
> > 					if (fcport->flags & FCF_FCP2_DEVICE)
> > 						fcport->logout_on_delete = 0;
> >
> > -					ql_dbg(ql_dbg_disc, vha, 0x20f0,
> > -					    "%s %d %8phC post del sess\n",
> > -					    __func__, __LINE__,
> > -					    fcport->port_name);
> > +					ql_log(ql_log_warn, vha, 0x20f0,
> > +					       "%s %d %8phC post del sess\n",
> > +					       __func__, __LINE__,
> > +					       fcport->port_name);
> >
> >
> 	qlt_schedule_sess_for_deletion(fcport);
> > 					continue;
> > --
> > 2.19.0.rc0
> >
> 
> I am okay with the change just curious, Would it not flood message file for
> large number of sessions?

This was added mainly for help in debugging, if debug is not enabled. Sometimes we get logs
where it's hard to tell what happened to particular session. Moreover session deletion is not
very common scenario.

Thanks,
~Saurav
> 
> --
> Himanshu Madhani	 Oracle Linux Engineering
Himanshu Madhani Dec. 1, 2020, 4:15 p.m. | #2
> On Dec 1, 2020, at 10:05 AM, Saurav Kashyap <skashyap@marvell.com> wrote:
> 
> Hi Himasnhu,
> Comments inline
> 
>> -----Original Message-----
>> From: Himanshu Madhani <himanshu.madhani@oracle.com>
>> Sent: Tuesday, December 1, 2020 9:15 PM
>> To: Nilesh Javali <njavali@marvell.com>
>> Cc: Martin K . Petersen <martin.petersen@oracle.com>; linux-
>> scsi@vger.kernel.org; GR-QLogic-Storage-Upstream <GR-QLogic-Storage-
>> Upstream@marvell.com>
>> Subject: Re: [PATCH 02/15] qla2xxx: Change post del message from debug level
>> to log level
>> 
>> 
>> 
>>> On Dec 1, 2020, at 2:27 AM, Nilesh Javali <njavali@marvell.com> wrote:
>>> 
>>> From: Saurav Kashyap <skashyap@marvell.com>
>>> 
>>> Change the message debug level.
>>> 
>>> Signed-off-by: Saurav Kashyap <skashyap@marvell.com>
>>> Signed-off-by: Nilesh Javali <njavali@marvell.com>
>>> ---
>>> drivers/scsi/qla2xxx/qla_gs.c | 8 ++++----
>>> 1 file changed, 4 insertions(+), 4 deletions(-)
>>> 
>>> diff --git a/drivers/scsi/qla2xxx/qla_gs.c b/drivers/scsi/qla2xxx/qla_gs.c
>>> index e28c4b7ec55f..391ac75e3de3 100644
>>> --- a/drivers/scsi/qla2xxx/qla_gs.c
>>> +++ b/drivers/scsi/qla2xxx/qla_gs.c
>>> @@ -3558,10 +3558,10 @@ void qla24xx_async_gnnft_done(scsi_qla_host_t
>> *vha, srb_t *sp)
>>> 					if (fcport->flags & FCF_FCP2_DEVICE)
>>> 						fcport->logout_on_delete = 0;
>>> 
>>> -					ql_dbg(ql_dbg_disc, vha, 0x20f0,
>>> -					    "%s %d %8phC post del sess\n",
>>> -					    __func__, __LINE__,
>>> -					    fcport->port_name);
>>> +					ql_log(ql_log_warn, vha, 0x20f0,
>>> +					       "%s %d %8phC post del sess\n",
>>> +					       __func__, __LINE__,
>>> +					       fcport->port_name);
>>> 
>>> 
>> 	qlt_schedule_sess_for_deletion(fcport);
>>> 					continue;
>>> --
>>> 2.19.0.rc0
>>> 
>> 
>> I am okay with the change just curious, Would it not flood message file for
>> large number of sessions?
> 
> This was added mainly for help in debugging, if debug is not enabled. Sometimes we get logs
> where it's hard to tell what happened to particular session. Moreover session deletion is not
> very common scenario.
> 

In that case, I would also prefer to see message coming out from qlt_schedule_sess_for_deletion(), because that’s where you can track that session is scheduled for deletion. 

I would prefer this message also changed to ql_log_warn. 

        ql_dbg(ql_dbg_disc, sess->vha, 0xe001,
            "Scheduling sess %p for deletion %8phC\n",
            sess, sess->port_name);

> Thanks,
> ~Saurav
>> 
>> --
>> Himanshu Madhani	 Oracle Linux Engineering
> 

--
Himanshu Madhani	 Oracle Linux Engineering
Saurav Kashyap Dec. 2, 2020, 7:44 a.m. | #3
Hi Himanshu,

> -----Original Message-----

> From: Himanshu Madhani <himanshu.madhani@oracle.com>

> Sent: Tuesday, December 1, 2020 9:46 PM

> To: Saurav Kashyap <skashyap@marvell.com>

> Cc: Nilesh Javali <njavali@marvell.com>; Martin K . Petersen

> <martin.petersen@oracle.com>; linux-scsi@vger.kernel.org; GR-QLogic-

> Storage-Upstream <GR-QLogic-Storage-Upstream@marvell.com>

> Subject: [EXT] Re: [PATCH 02/15] qla2xxx: Change post del message from

> debug level to log level

> 

> External Email

> 

> ----------------------------------------------------------------------

> 

> 

> > On Dec 1, 2020, at 10:05 AM, Saurav Kashyap <skashyap@marvell.com>

> wrote:

> >

> > Hi Himasnhu,

> > Comments inline

> >

> >> -----Original Message-----

> >> From: Himanshu Madhani <himanshu.madhani@oracle.com>

> >> Sent: Tuesday, December 1, 2020 9:15 PM

> >> To: Nilesh Javali <njavali@marvell.com>

> >> Cc: Martin K . Petersen <martin.petersen@oracle.com>; linux-

> >> scsi@vger.kernel.org; GR-QLogic-Storage-Upstream <GR-QLogic-Storage-

> >> Upstream@marvell.com>

> >> Subject: Re: [PATCH 02/15] qla2xxx: Change post del message from debug

> level

> >> to log level

> >>

> >>

> >>

> >>> On Dec 1, 2020, at 2:27 AM, Nilesh Javali <njavali@marvell.com> wrote:

> >>>

> >>> From: Saurav Kashyap <skashyap@marvell.com>

> >>>

> >>> Change the message debug level.

> >>>

> >>> Signed-off-by: Saurav Kashyap <skashyap@marvell.com>

> >>> Signed-off-by: Nilesh Javali <njavali@marvell.com>

> >>> ---

> >>> drivers/scsi/qla2xxx/qla_gs.c | 8 ++++----

> >>> 1 file changed, 4 insertions(+), 4 deletions(-)

> >>>

> >>> diff --git a/drivers/scsi/qla2xxx/qla_gs.c b/drivers/scsi/qla2xxx/qla_gs.c

> >>> index e28c4b7ec55f..391ac75e3de3 100644

> >>> --- a/drivers/scsi/qla2xxx/qla_gs.c

> >>> +++ b/drivers/scsi/qla2xxx/qla_gs.c

> >>> @@ -3558,10 +3558,10 @@ void

> qla24xx_async_gnnft_done(scsi_qla_host_t

> >> *vha, srb_t *sp)

> >>> 					if (fcport->flags & FCF_FCP2_DEVICE)

> >>> 						fcport->logout_on_delete = 0;

> >>>

> >>> -					ql_dbg(ql_dbg_disc, vha, 0x20f0,

> >>> -					    "%s %d %8phC post del sess\n",

> >>> -					    __func__, __LINE__,

> >>> -					    fcport->port_name);

> >>> +					ql_log(ql_log_warn, vha, 0x20f0,

> >>> +					       "%s %d %8phC post del sess\n",

> >>> +					       __func__, __LINE__,

> >>> +					       fcport->port_name);

> >>>

> >>>

> >> 	qlt_schedule_sess_for_deletion(fcport);

> >>> 					continue;

> >>> --

> >>> 2.19.0.rc0

> >>>

> >>

> >> I am okay with the change just curious, Would it not flood message file for

> >> large number of sessions?

> >

> > This was added mainly for help in debugging, if debug is not enabled.

> Sometimes we get logs

> > where it's hard to tell what happened to particular session. Moreover session

> deletion is not

> > very common scenario.

> >

> 

> In that case, I would also prefer to see message coming out from

> qlt_schedule_sess_for_deletion(), because that’s where you can track that

> session is scheduled for deletion.

> 

> I would prefer this message also changed to ql_log_warn.

> 

>         ql_dbg(ql_dbg_disc, sess->vha, 0xe001,

>             "Scheduling sess %p for deletion %8phC\n",

>             sess, sess->port_name);


Make sense, will update it in next version.

Thanks,
~Saurav
> 

> > Thanks,

> > ~Saurav

> >>

> >> --

> >> Himanshu Madhani	 Oracle Linux Engineering

> >

> 

> --

> Himanshu Madhani	 Oracle Linux Engineering

Patch

diff --git a/drivers/scsi/qla2xxx/qla_gs.c b/drivers/scsi/qla2xxx/qla_gs.c
index e28c4b7ec55f..391ac75e3de3 100644
--- a/drivers/scsi/qla2xxx/qla_gs.c
+++ b/drivers/scsi/qla2xxx/qla_gs.c
@@ -3558,10 +3558,10 @@  void qla24xx_async_gnnft_done(scsi_qla_host_t *vha, srb_t *sp)
 					if (fcport->flags & FCF_FCP2_DEVICE)
 						fcport->logout_on_delete = 0;
 
-					ql_dbg(ql_dbg_disc, vha, 0x20f0,
-					    "%s %d %8phC post del sess\n",
-					    __func__, __LINE__,
-					    fcport->port_name);
+					ql_log(ql_log_warn, vha, 0x20f0,
+					       "%s %d %8phC post del sess\n",
+					       __func__, __LINE__,
+					       fcport->port_name);
 
 					qlt_schedule_sess_for_deletion(fcport);
 					continue;