lkml.org 
[lkml]   [2021]   [Nov]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 5.15 573/917] scsi: pm80xx: Fix lockup in outbound queue management
    Date
    From: Ajish Koshy <Ajish.Koshy@microchip.com>

    [ Upstream commit b27a40534ef76a22628a5c12f98ea489823a8ba5 ]

    Commit 1f02beff224e ("scsi: pm80xx: Remove global lock from outbound queue
    processing") introduced a lock per outbound queue. Prior to that change the
    driver was using a global lock for all outbound queues.

    While processing the I/O responses and events the driver takes the outbound
    queue spinlock and is supposed to release it in pm8001_ccb_task_free_done()
    before calling command done(). Since the older code was using a global
    lock, pm8001_ccb_task_free_done() was releasing the global spin lock. The
    change that split the lock per outbound queue did not consider this and
    pm8001_ccb_task_free_done() was still releasing the global lock.

    Link: https://lore.kernel.org/r/20210906170404.5682-3-Ajish.Koshy@microchip.com
    Fixes: 1f02beff224e ("scsi: pm80xx: Remove global lock from outbound queue processing")
    Acked-by: Jack Wang <jinpu.wang@ionos.com>
    Signed-off-by: Ajish Koshy <Ajish.Koshy@microchip.com>
    Signed-off-by: Viswas G <Viswas.G@microchip.com>
    Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
    Signed-off-by: Sasha Levin <sashal@kernel.org>
    ---
    drivers/scsi/pm8001/pm8001_sas.h | 3 +-
    drivers/scsi/pm8001/pm80xx_hwi.c | 53 ++++++++++++++++++++++++++------
    2 files changed, 45 insertions(+), 11 deletions(-)

    diff --git a/drivers/scsi/pm8001/pm8001_sas.h b/drivers/scsi/pm8001/pm8001_sas.h
    index 62d08b535a4b6..e18f2b60371db 100644
    --- a/drivers/scsi/pm8001/pm8001_sas.h
    +++ b/drivers/scsi/pm8001/pm8001_sas.h
    @@ -457,6 +457,7 @@ struct outbound_queue_table {
    __le32 producer_index;
    u32 consumer_idx;
    spinlock_t oq_lock;
    + unsigned long lock_flags;
    };
    struct pm8001_hba_memspace {
    void __iomem *memvirtaddr;
    @@ -738,9 +739,7 @@ pm8001_ccb_task_free_done(struct pm8001_hba_info *pm8001_ha,
    {
    pm8001_ccb_task_free(pm8001_ha, task, ccb, ccb_idx);
    smp_mb(); /*in order to force CPU ordering*/
    - spin_unlock(&pm8001_ha->lock);
    task->task_done(task);
    - spin_lock(&pm8001_ha->lock);
    }

    #endif
    diff --git a/drivers/scsi/pm8001/pm80xx_hwi.c b/drivers/scsi/pm8001/pm80xx_hwi.c
    index 6ffe17b849ae8..ed02e1aaf868c 100644
    --- a/drivers/scsi/pm8001/pm80xx_hwi.c
    +++ b/drivers/scsi/pm8001/pm80xx_hwi.c
    @@ -2379,7 +2379,8 @@ static void mpi_ssp_event(struct pm8001_hba_info *pm8001_ha, void *piomb)

    /*See the comments for mpi_ssp_completion */
    static void
    -mpi_sata_completion(struct pm8001_hba_info *pm8001_ha, void *piomb)
    +mpi_sata_completion(struct pm8001_hba_info *pm8001_ha,
    + struct outbound_queue_table *circularQ, void *piomb)
    {
    struct sas_task *t;
    struct pm8001_ccb_info *ccb;
    @@ -2616,7 +2617,11 @@ mpi_sata_completion(struct pm8001_hba_info *pm8001_ha, void *piomb)
    IO_OPEN_CNX_ERROR_IT_NEXUS_LOSS);
    ts->resp = SAS_TASK_UNDELIVERED;
    ts->stat = SAS_QUEUE_FULL;
    + spin_unlock_irqrestore(&circularQ->oq_lock,
    + circularQ->lock_flags);
    pm8001_ccb_task_free_done(pm8001_ha, t, ccb, tag);
    + spin_lock_irqsave(&circularQ->oq_lock,
    + circularQ->lock_flags);
    return;
    }
    break;
    @@ -2632,7 +2637,11 @@ mpi_sata_completion(struct pm8001_hba_info *pm8001_ha, void *piomb)
    IO_OPEN_CNX_ERROR_IT_NEXUS_LOSS);
    ts->resp = SAS_TASK_UNDELIVERED;
    ts->stat = SAS_QUEUE_FULL;
    + spin_unlock_irqrestore(&circularQ->oq_lock,
    + circularQ->lock_flags);
    pm8001_ccb_task_free_done(pm8001_ha, t, ccb, tag);
    + spin_lock_irqsave(&circularQ->oq_lock,
    + circularQ->lock_flags);
    return;
    }
    break;
    @@ -2656,7 +2665,11 @@ mpi_sata_completion(struct pm8001_hba_info *pm8001_ha, void *piomb)
    IO_OPEN_CNX_ERROR_STP_RESOURCES_BUSY);
    ts->resp = SAS_TASK_UNDELIVERED;
    ts->stat = SAS_QUEUE_FULL;
    + spin_unlock_irqrestore(&circularQ->oq_lock,
    + circularQ->lock_flags);
    pm8001_ccb_task_free_done(pm8001_ha, t, ccb, tag);
    + spin_lock_irqsave(&circularQ->oq_lock,
    + circularQ->lock_flags);
    return;
    }
    break;
    @@ -2727,7 +2740,11 @@ mpi_sata_completion(struct pm8001_hba_info *pm8001_ha, void *piomb)
    IO_DS_NON_OPERATIONAL);
    ts->resp = SAS_TASK_UNDELIVERED;
    ts->stat = SAS_QUEUE_FULL;
    + spin_unlock_irqrestore(&circularQ->oq_lock,
    + circularQ->lock_flags);
    pm8001_ccb_task_free_done(pm8001_ha, t, ccb, tag);
    + spin_lock_irqsave(&circularQ->oq_lock,
    + circularQ->lock_flags);
    return;
    }
    break;
    @@ -2747,7 +2764,11 @@ mpi_sata_completion(struct pm8001_hba_info *pm8001_ha, void *piomb)
    IO_DS_IN_ERROR);
    ts->resp = SAS_TASK_UNDELIVERED;
    ts->stat = SAS_QUEUE_FULL;
    + spin_unlock_irqrestore(&circularQ->oq_lock,
    + circularQ->lock_flags);
    pm8001_ccb_task_free_done(pm8001_ha, t, ccb, tag);
    + spin_lock_irqsave(&circularQ->oq_lock,
    + circularQ->lock_flags);
    return;
    }
    break;
    @@ -2785,12 +2806,17 @@ mpi_sata_completion(struct pm8001_hba_info *pm8001_ha, void *piomb)
    pm8001_ccb_task_free(pm8001_ha, t, ccb, tag);
    } else {
    spin_unlock_irqrestore(&t->task_state_lock, flags);
    + spin_unlock_irqrestore(&circularQ->oq_lock,
    + circularQ->lock_flags);
    pm8001_ccb_task_free_done(pm8001_ha, t, ccb, tag);
    + spin_lock_irqsave(&circularQ->oq_lock,
    + circularQ->lock_flags);
    }
    }

    /*See the comments for mpi_ssp_completion */
    -static void mpi_sata_event(struct pm8001_hba_info *pm8001_ha, void *piomb)
    +static void mpi_sata_event(struct pm8001_hba_info *pm8001_ha,
    + struct outbound_queue_table *circularQ, void *piomb)
    {
    struct sas_task *t;
    struct task_status_struct *ts;
    @@ -2890,7 +2916,11 @@ static void mpi_sata_event(struct pm8001_hba_info *pm8001_ha, void *piomb)
    IO_OPEN_CNX_ERROR_IT_NEXUS_LOSS);
    ts->resp = SAS_TASK_COMPLETE;
    ts->stat = SAS_QUEUE_FULL;
    + spin_unlock_irqrestore(&circularQ->oq_lock,
    + circularQ->lock_flags);
    pm8001_ccb_task_free_done(pm8001_ha, t, ccb, tag);
    + spin_lock_irqsave(&circularQ->oq_lock,
    + circularQ->lock_flags);
    return;
    }
    break;
    @@ -3002,7 +3032,11 @@ static void mpi_sata_event(struct pm8001_hba_info *pm8001_ha, void *piomb)
    pm8001_ccb_task_free(pm8001_ha, t, ccb, tag);
    } else {
    spin_unlock_irqrestore(&t->task_state_lock, flags);
    + spin_unlock_irqrestore(&circularQ->oq_lock,
    + circularQ->lock_flags);
    pm8001_ccb_task_free_done(pm8001_ha, t, ccb, tag);
    + spin_lock_irqsave(&circularQ->oq_lock,
    + circularQ->lock_flags);
    }
    }

    @@ -3902,7 +3936,8 @@ static int ssp_coalesced_comp_resp(struct pm8001_hba_info *pm8001_ha,
    * @pm8001_ha: our hba card information
    * @piomb: IO message buffer
    */
    -static void process_one_iomb(struct pm8001_hba_info *pm8001_ha, void *piomb)
    +static void process_one_iomb(struct pm8001_hba_info *pm8001_ha,
    + struct outbound_queue_table *circularQ, void *piomb)
    {
    __le32 pHeader = *(__le32 *)piomb;
    u32 opc = (u32)((le32_to_cpu(pHeader)) & 0xFFF);
    @@ -3944,11 +3979,11 @@ static void process_one_iomb(struct pm8001_hba_info *pm8001_ha, void *piomb)
    break;
    case OPC_OUB_SATA_COMP:
    pm8001_dbg(pm8001_ha, MSG, "OPC_OUB_SATA_COMP\n");
    - mpi_sata_completion(pm8001_ha, piomb);
    + mpi_sata_completion(pm8001_ha, circularQ, piomb);
    break;
    case OPC_OUB_SATA_EVENT:
    pm8001_dbg(pm8001_ha, MSG, "OPC_OUB_SATA_EVENT\n");
    - mpi_sata_event(pm8001_ha, piomb);
    + mpi_sata_event(pm8001_ha, circularQ, piomb);
    break;
    case OPC_OUB_SSP_EVENT:
    pm8001_dbg(pm8001_ha, MSG, "OPC_OUB_SSP_EVENT\n");
    @@ -4117,7 +4152,6 @@ static int process_oq(struct pm8001_hba_info *pm8001_ha, u8 vec)
    void *pMsg1 = NULL;
    u8 bc;
    u32 ret = MPI_IO_STATUS_FAIL;
    - unsigned long flags;
    u32 regval;

    if (vec == (pm8001_ha->max_q_num - 1)) {
    @@ -4134,7 +4168,7 @@ static int process_oq(struct pm8001_hba_info *pm8001_ha, u8 vec)
    }
    }
    circularQ = &pm8001_ha->outbnd_q_tbl[vec];
    - spin_lock_irqsave(&circularQ->oq_lock, flags);
    + spin_lock_irqsave(&circularQ->oq_lock, circularQ->lock_flags);
    do {
    /* spurious interrupt during setup if kexec-ing and
    * driver doing a doorbell access w/ the pre-kexec oq
    @@ -4145,7 +4179,8 @@ static int process_oq(struct pm8001_hba_info *pm8001_ha, u8 vec)
    ret = pm8001_mpi_msg_consume(pm8001_ha, circularQ, &pMsg1, &bc);
    if (MPI_IO_STATUS_SUCCESS == ret) {
    /* process the outbound message */
    - process_one_iomb(pm8001_ha, (void *)(pMsg1 - 4));
    + process_one_iomb(pm8001_ha, circularQ,
    + (void *)(pMsg1 - 4));
    /* free the message from the outbound circular buffer */
    pm8001_mpi_msg_free_set(pm8001_ha, pMsg1,
    circularQ, bc);
    @@ -4160,7 +4195,7 @@ static int process_oq(struct pm8001_hba_info *pm8001_ha, u8 vec)
    break;
    }
    } while (1);
    - spin_unlock_irqrestore(&circularQ->oq_lock, flags);
    + spin_unlock_irqrestore(&circularQ->oq_lock, circularQ->lock_flags);
    return ret;
    }

    --
    2.33.0


    \
     
     \ /
      Last update: 2021-11-16 01:40    [W:2.344 / U:3.688 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site