scsi: pm8001: Fix tag values handling

The function pm8001_tag_alloc() determines free tags using the function
find_first_zero_bit() which can return 0 when the first bit of the bitmap
being inspected is 0. As such, tag 0 is a valid tag value that should not
be dismissed as invalid. Fix the functions pm8001_work_fn(),
mpi_sata_completion(), pm8001_mpi_task_abort_resp() and
pm8001_open_reject_retry() to not dismiss 0 tags as invalid.

The value 0xffffffff is used for invalid tags for unused ccb information
structures. Add the macro definition PM8001_INVALID_TAG to define this
value.

Link: https://lore.kernel.org/r/20220220031810.738362-20-damien.lemoal@opensource.wdc.com
Reviewed-by: Jack Wang <jinpu.wang@ionos.com>
Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>

diff --git a/drivers/scsi/pm8001/pm8001_hwi.c b/drivers/scsi/pm8001/pm8001_hwi.c
index be57b9ae5486fd2deeb7c8dff144d7687a6fc2c6..6f9ee77cc5766e6d36cc2512a8a2ffb08ccdd541 100644 (file)
--- a/drivers/scsi/pm8001/pm8001_hwi.c
+++ b/drivers/scsi/pm8001/pm8001_hwi.c
@@ -1522,7 +1522,6 @@ void pm8001_work_fn(struct work_struct *work)
        case IO_XFER_ERROR_BREAK:
        {       /* This one stashes the sas_task instead */
                struct sas_task *t = (struct sas_task *)pm8001_dev;
-               u32 tag;
                struct pm8001_ccb_info *ccb;
                struct pm8001_hba_info *pm8001_ha = pw->pm8001_ha;
                unsigned long flags, flags1;
@@ -1544,8 +1543,8 @@ void pm8001_work_fn(struct work_struct *work)
                /* Search for a possible ccb that matches the task */
                for (i = 0; ccb = NULL, i < PM8001_MAX_CCB; i++) {
                        ccb = &pm8001_ha->ccb_info[i];
-                       tag = ccb->ccb_tag;
-                       if ((tag != 0xFFFFFFFF) && (ccb->task == t))
+                       if ((ccb->ccb_tag != PM8001_INVALID_TAG) &&
+                           (ccb->task == t))
                                break;
                }
                if (!ccb) {
@@ -1566,11 +1565,11 @@ void pm8001_work_fn(struct work_struct *work)
                        spin_unlock_irqrestore(&t->task_state_lock, flags1);
                        pm8001_dbg(pm8001_ha, FAIL, "task 0x%p done with event 0x%x resp 0x%x stat 0x%x but aborted by upper layer!\n",
                                   t, pw->handler, ts->resp, ts->stat);
-                       pm8001_ccb_task_free(pm8001_ha, t, ccb, tag);
+                       pm8001_ccb_task_free(pm8001_ha, t, ccb, ccb->ccb_tag);
                        spin_unlock_irqrestore(&pm8001_ha->lock, flags);
                } else {
                        spin_unlock_irqrestore(&t->task_state_lock, flags1);
-                       pm8001_ccb_task_free(pm8001_ha, t, ccb, tag);
+                       pm8001_ccb_task_free(pm8001_ha, t, ccb, ccb->ccb_tag);
                        mb();/* in order to force CPU ordering */
                        spin_unlock_irqrestore(&pm8001_ha->lock, flags);
                        t->task_done(t);
@@ -1579,7 +1578,6 @@ void pm8001_work_fn(struct work_struct *work)
        case IO_XFER_OPEN_RETRY_TIMEOUT:
        {       /* This one stashes the sas_task instead */
                struct sas_task *t = (struct sas_task *)pm8001_dev;
-               u32 tag;
                struct pm8001_ccb_info *ccb;
                struct pm8001_hba_info *pm8001_ha = pw->pm8001_ha;
                unsigned long flags, flags1;
@@ -1613,8 +1611,8 @@ void pm8001_work_fn(struct work_struct *work)
                /* Search for a possible ccb that matches the task */
                for (i = 0; ccb = NULL, i < PM8001_MAX_CCB; i++) {
                        ccb = &pm8001_ha->ccb_info[i];
-                       tag = ccb->ccb_tag;
-                       if ((tag != 0xFFFFFFFF) && (ccb->task == t))
+                       if ((ccb->ccb_tag != PM8001_INVALID_TAG) &&
+                           (ccb->task == t))
                                break;
                }
                if (!ccb) {
@@ -1685,19 +1683,13 @@ void pm8001_work_fn(struct work_struct *work)
                struct task_status_struct *ts;
                struct sas_task *task;
                int i;
-               u32 tag, device_id;
+               u32 device_id;
 
                for (i = 0; ccb = NULL, i < PM8001_MAX_CCB; i++) {
                        ccb = &pm8001_ha->ccb_info[i];
                        task = ccb->task;
                        ts = &task->task_status;
-                       tag = ccb->ccb_tag;
-                       /* check if tag is NULL */
-                       if (!tag) {
-                               pm8001_dbg(pm8001_ha, FAIL,
-                                       "tag Null\n");
-                               continue;
-                       }
+
                        if (task != NULL) {
                                dev = task->dev;
                                if (!dev) {
@@ -1706,10 +1698,11 @@ void pm8001_work_fn(struct work_struct *work)
                                        continue;
                                }
                                /*complete sas task and update to top layer */
-                               pm8001_ccb_task_free(pm8001_ha, task, ccb, tag);
+                               pm8001_ccb_task_free(pm8001_ha, task, ccb,
+                                                    ccb->ccb_tag);
                                ts->resp = SAS_TASK_COMPLETE;
                                task->task_done(task);
-                       } else if (tag != 0xFFFFFFFF) {
+                       } else if (ccb->ccb_tag != PM8001_INVALID_TAG) {
                                /* complete the internal commands/non-sas task */
                                pm8001_dev = ccb->device;
                                if (pm8001_dev->dcompletion) {
@@ -1717,7 +1710,7 @@ void pm8001_work_fn(struct work_struct *work)
                                        pm8001_dev->dcompletion = NULL;
                                }
                                complete(pm8001_ha->nvmd_completion);
-                               pm8001_tag_free(pm8001_ha, tag);
+                               pm8001_tag_free(pm8001_ha, ccb->ccb_tag);
                        }
                }
                /* Deregister all the device ids  */
@@ -2313,11 +2306,6 @@ mpi_sata_completion(struct pm8001_hba_info *pm8001_ha, void *piomb)
        param = le32_to_cpu(psataPayload->param);
        tag = le32_to_cpu(psataPayload->tag);
 
-       if (!tag) {
-               pm8001_dbg(pm8001_ha, FAIL, "tag null\n");
-               return;
-       }
-
        ccb = &pm8001_ha->ccb_info[tag];
        t = ccb->task;
        pm8001_dev = ccb->device;
@@ -3051,7 +3039,7 @@ void pm8001_mpi_set_dev_state_resp(struct pm8001_hba_info *pm8001_ha,
                   device_id, pds, nds, status);
        complete(pm8001_dev->setds_completion);
        ccb->task = NULL;
-       ccb->ccb_tag = 0xFFFFFFFF;
+       ccb->ccb_tag = PM8001_INVALID_TAG;
        pm8001_tag_free(pm8001_ha, tag);
 }
 
@@ -3069,7 +3057,7 @@ void pm8001_mpi_set_nvmd_resp(struct pm8001_hba_info *pm8001_ha, void *piomb)
                                dlen_status);
        }
        ccb->task = NULL;
-       ccb->ccb_tag = 0xFFFFFFFF;
+       ccb->ccb_tag = PM8001_INVALID_TAG;
        pm8001_tag_free(pm8001_ha, tag);
 }
 
@@ -3096,7 +3084,7 @@ pm8001_mpi_get_nvmd_resp(struct pm8001_hba_info *pm8001_ha, void *piomb)
                 * freed by requesting path anywhere.
                 */
                ccb->task = NULL;
-               ccb->ccb_tag = 0xFFFFFFFF;
+               ccb->ccb_tag = PM8001_INVALID_TAG;
                pm8001_tag_free(pm8001_ha, tag);
                return;
        }
@@ -3142,7 +3130,7 @@ pm8001_mpi_get_nvmd_resp(struct pm8001_hba_info *pm8001_ha, void *piomb)
        complete(pm8001_ha->nvmd_completion);
        pm8001_dbg(pm8001_ha, MSG, "Get nvmd data complete!\n");
        ccb->task = NULL;
-       ccb->ccb_tag = 0xFFFFFFFF;
+       ccb->ccb_tag = PM8001_INVALID_TAG;
        pm8001_tag_free(pm8001_ha, tag);
 }
 
@@ -3555,7 +3543,7 @@ int pm8001_mpi_reg_resp(struct pm8001_hba_info *pm8001_ha, void *piomb)
        }
        complete(pm8001_dev->dcompletion);
        ccb->task = NULL;
-       ccb->ccb_tag = 0xFFFFFFFF;
+       ccb->ccb_tag = PM8001_INVALID_TAG;
        pm8001_tag_free(pm8001_ha, htag);
        return 0;
 }
@@ -3627,7 +3615,7 @@ int pm8001_mpi_fw_flash_update_resp(struct pm8001_hba_info *pm8001_ha,
        }
        kfree(ccb->fw_control_context);
        ccb->task = NULL;
-       ccb->ccb_tag = 0xFFFFFFFF;
+       ccb->ccb_tag = PM8001_INVALID_TAG;
        pm8001_tag_free(pm8001_ha, tag);
        complete(pm8001_ha->nvmd_completion);
        return 0;
@@ -3663,10 +3651,6 @@ int pm8001_mpi_task_abort_resp(struct pm8001_hba_info *pm8001_ha, void *piomb)
 
        status = le32_to_cpu(pPayload->status);
        tag = le32_to_cpu(pPayload->tag);
-       if (!tag) {
-               pm8001_dbg(pm8001_ha, FAIL, " TAG NULL. RETURNING !!!\n");
-               return -1;
-       }
 
        scp = le32_to_cpu(pPayload->scp);
        ccb = &pm8001_ha->ccb_info[tag];

diff --git a/drivers/scsi/pm8001/pm8001_init.c b/drivers/scsi/pm8001/pm8001_init.c
index 3811052869538ce9674134c455db09c8c6568072..9b04f1a6a67d7c807b886c2da899db459b1afcaa 100644 (file)
--- a/drivers/scsi/pm8001/pm8001_init.c
+++ b/drivers/scsi/pm8001/pm8001_init.c
@@ -1217,10 +1217,11 @@ pm8001_init_ccb_tag(struct pm8001_hba_info *pm8001_ha, struct Scsi_Host *shost,
                        goto err_out;
                }
                pm8001_ha->ccb_info[i].task = NULL;
-               pm8001_ha->ccb_info[i].ccb_tag = 0xffffffff;
+               pm8001_ha->ccb_info[i].ccb_tag = PM8001_INVALID_TAG;
                pm8001_ha->ccb_info[i].device = NULL;
                ++pm8001_ha->tags_num;
        }
+
        return 0;
 
 err_out_noccb:

diff --git a/drivers/scsi/pm8001/pm8001_sas.c b/drivers/scsi/pm8001/pm8001_sas.c
index f5678be4c17bbdea4349542c8c5af8d2500becba..f409dfa049c07017082229e3453b00deb1becdf9 100644 (file)
--- a/drivers/scsi/pm8001/pm8001_sas.c
+++ b/drivers/scsi/pm8001/pm8001_sas.c
@@ -553,7 +553,7 @@ void pm8001_ccb_task_free(struct pm8001_hba_info *pm8001_ha,
 
        task->lldd_task = NULL;
        ccb->task = NULL;
-       ccb->ccb_tag = 0xFFFFFFFF;
+       ccb->ccb_tag = PM8001_INVALID_TAG;
        ccb->open_retry = 0;
        pm8001_tag_free(pm8001_ha, ccb_idx);
 }
@@ -831,9 +831,11 @@ void pm8001_open_reject_retry(
                struct task_status_struct *ts;
                struct pm8001_device *pm8001_dev;
                unsigned long flags1;
-               u32 tag;
                struct pm8001_ccb_info *ccb = &pm8001_ha->ccb_info[i];
 
+               if (ccb->ccb_tag == PM8001_INVALID_TAG)
+                       continue;
+
                pm8001_dev = ccb->device;
                if (!pm8001_dev || (pm8001_dev->dev_type == SAS_PHY_UNUSED))
                        continue;
@@ -845,9 +847,6 @@ void pm8001_open_reject_retry(
                                continue;
                } else if (pm8001_dev != device_to_close)
                        continue;
-               tag = ccb->ccb_tag;
-               if (!tag || (tag == 0xFFFFFFFF))
-                       continue;
                task = ccb->task;
                if (!task || !task->task_done)
                        continue;
@@ -867,11 +866,11 @@ void pm8001_open_reject_retry(
                                & SAS_TASK_STATE_ABORTED))) {
                        spin_unlock_irqrestore(&task->task_state_lock,
                                flags1);
-                       pm8001_ccb_task_free(pm8001_ha, task, ccb, tag);
+                       pm8001_ccb_task_free(pm8001_ha, task, ccb, ccb->ccb_tag);
                } else {
                        spin_unlock_irqrestore(&task->task_state_lock,
                                flags1);
-                       pm8001_ccb_task_free(pm8001_ha, task, ccb, tag);
+                       pm8001_ccb_task_free(pm8001_ha, task, ccb, ccb->ccb_tag);
                        mb();/* in order to force CPU ordering */
                        spin_unlock_irqrestore(&pm8001_ha->lock, flags);
                        task->task_done(task);

diff --git a/drivers/scsi/pm8001/pm8001_sas.h b/drivers/scsi/pm8001/pm8001_sas.h
index d26f251867798d0f5913e8478bbae7e7774ae0b1..5082c7dc07eecb67e8517594b4ab2458642bfd6d 100644 (file)
--- a/drivers/scsi/pm8001/pm8001_sas.h
+++ b/drivers/scsi/pm8001/pm8001_sas.h
@@ -732,6 +732,8 @@ void pm8001_free_dev(struct pm8001_device *pm8001_dev);
 /* ctl shared API */
 extern const struct attribute_group *pm8001_host_groups[];
 
+#define PM8001_INVALID_TAG     ((u32)-1)
+
 static inline void
 pm8001_ccb_task_free_done(struct pm8001_hba_info *pm8001_ha,
                        struct sas_task *task, struct pm8001_ccb_info *ccb,

diff --git a/drivers/scsi/pm8001/pm80xx_hwi.c b/drivers/scsi/pm8001/pm80xx_hwi.c
index 76260d06b6bea9f5f67f7c132dd77c252e06a4fc..cc46e2013eebdf97887c35b73670fcbb9698a7c7 100644 (file)
--- a/drivers/scsi/pm8001/pm80xx_hwi.c
+++ b/drivers/scsi/pm8001/pm80xx_hwi.c
@@ -2402,11 +2402,6 @@ mpi_sata_completion(struct pm8001_hba_info *pm8001_ha,
        param = le32_to_cpu(psataPayload->param);
        tag = le32_to_cpu(psataPayload->tag);
 
-       if (!tag) {
-               pm8001_dbg(pm8001_ha, FAIL, "tag null\n");
-               return;
-       }
-
        ccb = &pm8001_ha->ccb_info[tag];
        t = ccb->task;
        pm8001_dev = ccb->device;