Re: [PATCH 01/15] qla2xxx: Set the SCSI command result before calling the command done

[Giridhar Malavali <gmalavali@xxxxxxxxxxx>]

On 3/28/19, 10:36 AM, "linux-scsi-owner@xxxxxxxxxxxxxxx on behalf of Bart Van Assche" <linux-scsi-owner@xxxxxxxxxxxxxxx on behalf of bvanassche@xxxxxxx> wrote:

    On Thu, 2019-03-28 at 10:09 -0700, Himanshu Madhani wrote:
    > From: Giridhar Malavali <gmalavali@xxxxxxxxxxx>
    > 
    > This patch sets SCSI cmd->result before scsi_done() is called.
    > 
    > Signed-off-by: Giridhar Malavali <gmalavali@xxxxxxxxxxx>
    > Signed-off-by: Himanshu Madhani <hmadhani@xxxxxxxxxxx>
    > ---
    >  drivers/scsi/qla2xxx/qla_os.c | 3 +-
    >  1 file changed, 1 insertion(), 2 deletions(-)
    > 
    > diff --git a/drivers/scsi/qla2xxx/qla_os.c b/drivers/scsi/qla2xxx/qla_os.c
    > index a58a2885fb70..34db83b6f932 100644
    > --- a/drivers/scsi/qla2xxx/qla_os.c
    > ﯦｶ뢯窻㾱쬢ﺩ嫛ᱱﺩ媢윻 窪嚶읍_sp_compl(void *ptr, int res)
    >  	srb_t *sp = ptr;
    >  	struct scsi_cmnd *cmd = GET_CMD_SP(sp);
    >  
    > -	cmd->result = res;
    > -
    >  	if (atomic_read(&sp->ref_count) == 0) {
    >  		ql_dbg(ql_dbg_io, sp->vha, 0x3015,
    >  		    "SP reference-count to ZERO -- sp=%p cmd=%p.n",
    > @@ -779,6 ﮪ嚶읍_sp_compl(void *ptr, int res)
    >  		return;
    >  
    >  	sp->free(sp);
    > 	cmd->result = res;
    >  	cmd->scsi_done(cmd);
    
    Hi Giridhar,
    
    A patch description should not only explain what is changed but also why a
    change has been made. What is the reason that you want to make this change?
  
Sorry for the missing detailed description. We have a race between the abort handler and completion handler (refer to patch #14 in this series) where the command result is set by both the handlers. The scsi done is called only after refcount on SRB structure goes to zero. The abort handler sets the result prematurely even if the refcount has not gone to zero. So, setting of the result is moved below reflecting the latest status when we are sure about calling scsi_done. 

-- Giri



  
    Thanks,
    
    Bart.