RE: [PATCH 02/16] qla2xxx: Implement ref count for srb

[Saurav Kashyap <skashyap@marvell.com>]

Hi Himanshu,

> -----Original Message-----
> From: Himanshu Madhani <himanshu.madhani@oracle.com>
> Sent: Monday, January 3, 2022 6:03 AM
> To: Nilesh Javali <njavali@marvell.com>
> Cc: Martin Petersen <martin.petersen@oracle.com>; linux-
> scsi@vger.kernel.org; GR-QLogic-Storage-Upstream <GR-QLogic-Storage-
> Upstream@marvell.com>
> Subject: Re: [PATCH 02/16] qla2xxx: Implement ref count for srb
> 
> 
> 
> > On Dec 23, 2021, at 11:06 PM, Nilesh Javali <njavali@marvell.com> wrote:
> >
> > From: Saurav Kashyap <skashyap@marvell.com>
> >
> > Fix race between timeout handler and completion handler by introducing
> > a reference counter. One reference is taken for the normal code path
> > (the 'good case') and one for the timeout path.
> >
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Daniel Wagner <dwagner@suse.de>
> > Signed-off-by: Saurav Kashyap <skashyap@marvell.com>
> > Signed-off-by: Nilesh Javali <njavali@marvell.com>
> > ---
> > drivers/scsi/qla2xxx/qla_bsg.c    |  6 ++-
> > drivers/scsi/qla2xxx/qla_def.h    |  5 ++
> > drivers/scsi/qla2xxx/qla_edif.c   |  3 +-
> > drivers/scsi/qla2xxx/qla_gbl.h    |  1 +
> > drivers/scsi/qla2xxx/qla_gs.c     | 85 +++++++++++++++++++++----------
> > drivers/scsi/qla2xxx/qla_init.c   | 70 +++++++++++++++++--------
> > drivers/scsi/qla2xxx/qla_inline.h |  2 +
> > drivers/scsi/qla2xxx/qla_iocb.c   | 41 +++++++++++----
> > drivers/scsi/qla2xxx/qla_mbx.c    |  4 +-
> > drivers/scsi/qla2xxx/qla_mid.c    |  4 +-
> > drivers/scsi/qla2xxx/qla_mr.c     |  4 +-
> > drivers/scsi/qla2xxx/qla_os.c     | 14 +++--
> > drivers/scsi/qla2xxx/qla_target.c |  4 +-
> > 13 files changed, 173 insertions(+), 70 deletions(-)
> >
> > diff --git a/drivers/scsi/qla2xxx/qla_bsg.c b/drivers/scsi/qla2xxx/qla_bsg.c
> > index 9da8034ccad4..c2f00f076f79 100644
> > --- a/drivers/scsi/qla2xxx/qla_bsg.c
> > +++ b/drivers/scsi/qla2xxx/qla_bsg.c
> > @@ -29,7 +29,8 @@ void qla2x00_bsg_job_done(srb_t *sp, int res)
> > 	    "%s: sp hdl %x, result=%x bsg ptr %p\n",
> > 	    __func__, sp->handle, res, bsg_job);
> >
> > -	sp->free(sp);
> > +	/* ref: INIT */
> 
> IMO, There is no need for this comment spread in this patch. Please explain If
> you think there is need for comment.

<SK> Thanks for the review. The sp reference can be taken and released on various paths. These comments make 
life simpler during some ref issue and also make code more understandable. For various scenarios, this comments
helps in determining final reference count and check if its released properly or not.

Thanks,
~Saurav