2022-02-18 00:13:22

by John Garry

[permalink] [raw]
Subject: [PATCH v2 00/18] scsi: libsas and users: Factor out LLDD TMF code

The LLDD TMF code is almost identical between hisi_sas, pm8001, and mvsas
drivers.

This series factors out that code into libsas, thus reducing much
duplication and giving a net reduction of ~350 LoC.

There are some subtle differences between the core TMF handler and each
of the LLDDs old implementation, so any review and testing is appreciated.

Some other minor patches are thrown in:
- Delete unused macro in hisi_sas driver
- Delete unused libsas callback
- Delete unused SAS_SG_ERR
- Add enum for response frame datapres field
- Handle unrecognised errors in sas_scsi_find_task()

I have another follow-up series to factor out the internal abort code,
which is common to hisi_sas and pm8001 drivers.

Based on mkp-scsi 5.18 staging queue at commit ac2beb4e3bd7

Differences to v1:
- Add Reviewed-by and Tested-by tags (Thanks!)
- Add SAS_SG_ERR patch
- Add sas_scsi_find_task() patch
- Use switch statement in sas_ssp_task_response()
- Add DATAPRES enum in sas.h
- Reword "Add struct sas_tmf_task" patch
- Don't print TMF code in sas_execute_tmf()
- Rebase

John Garry (18):
scsi: libsas: Handle non-TMF codes in sas_scsi_find_task()
scsi: libsas: Use enum for response frame DATAPRES field
scsi: libsas: Delete lldd_clear_aca callback
scsi: libsas: Delete SAS_SG_ERR
scsi: hisi_sas: Delete unused I_T_NEXUS_RESET_PHYUP_TIMEOUT
scsi: libsas: Move SMP task handlers to core
scsi: libsas: Add struct sas_tmf_task
scsi: libsas: Add sas_task.tmf
scsi: libsas: Add sas_execute_tmf()
scsi: libsas: Add sas_execute_ssp_tmf()
scsi: libsas: Add TMF handler exec complete callback
scsi: libsas: Add TMF handler aborted callback
scsi: libsas: Add sas_abort_task_set()
scsi: libsas: Add sas_clear_task_set()
scsi: libsas: Add sas_lu_reset()
scsi: libsas: Add sas_query_task()
scsi: libsas: Add sas_abort_task()
scsi: libsas: Add sas_execute_ata_cmd()

Documentation/scsi/libsas.rst | 2 -
drivers/scsi/aic94xx/aic94xx.h | 1 -
drivers/scsi/aic94xx/aic94xx_init.c | 1 -
drivers/scsi/aic94xx/aic94xx_tmf.c | 11 +-
drivers/scsi/hisi_sas/hisi_sas.h | 9 +-
drivers/scsi/hisi_sas/hisi_sas_main.c | 227 ++++--------------------
drivers/scsi/hisi_sas/hisi_sas_v1_hw.c | 2 +-
drivers/scsi/hisi_sas/hisi_sas_v2_hw.c | 9 +-
drivers/scsi/hisi_sas/hisi_sas_v3_hw.c | 2 +-
drivers/scsi/isci/init.c | 1 -
drivers/scsi/isci/request.c | 7 +-
drivers/scsi/isci/task.c | 18 --
drivers/scsi/isci/task.h | 4 -
drivers/scsi/libsas/sas_ata.c | 10 +-
drivers/scsi/libsas/sas_expander.c | 24 +--
drivers/scsi/libsas/sas_internal.h | 6 +
drivers/scsi/libsas/sas_scsi_host.c | 229 ++++++++++++++++++++++++-
drivers/scsi/libsas/sas_task.c | 14 +-
drivers/scsi/mvsas/mv_defs.h | 5 -
drivers/scsi/mvsas/mv_init.c | 5 +-
drivers/scsi/mvsas/mv_sas.c | 179 +------------------
drivers/scsi/mvsas/mv_sas.h | 3 -
drivers/scsi/pm8001/pm8001_hwi.c | 4 +-
drivers/scsi/pm8001/pm8001_init.c | 5 +-
drivers/scsi/pm8001/pm8001_sas.c | 194 ++++-----------------
drivers/scsi/pm8001/pm8001_sas.h | 14 +-
include/scsi/libsas.h | 24 ++-
include/scsi/sas.h | 7 +
28 files changed, 377 insertions(+), 640 deletions(-)

--
2.26.2


2022-02-18 00:20:33

by John Garry

[permalink] [raw]
Subject: [PATCH v2 15/18] scsi: libsas: Add sas_lu_reset()

Add a generic implementation of LU reset TMF handler, and use in LLDDs.

Signed-off-by: John Garry <[email protected]>
Reviewed-by: Christoph Hellwig <[email protected]>
Tested-by: Yihang Li <[email protected]>
---
drivers/scsi/hisi_sas/hisi_sas_main.c | 4 +---
drivers/scsi/libsas/sas_scsi_host.c | 10 ++++++++++
drivers/scsi/mvsas/mv_sas.c | 4 +---
drivers/scsi/pm8001/pm8001_sas.c | 4 +---
include/scsi/libsas.h | 1 +
5 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/drivers/scsi/hisi_sas/hisi_sas_main.c b/drivers/scsi/hisi_sas/hisi_sas_main.c
index 6826ddfeaca5..3773874b0c2e 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_main.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_main.c
@@ -1933,9 +1933,7 @@ static int hisi_sas_lu_reset(struct domain_device *device, u8 *lun)
hisi_sas_release_task(hisi_hba, device);
sas_put_local_phy(phy);
} else {
- struct sas_tmf_task tmf_task = { .tmf = TMF_LU_RESET };
-
- rc = hisi_sas_debug_issue_ssp_tmf(device, lun, &tmf_task);
+ rc = sas_lu_reset(device, lun);
if (rc == TMF_RESP_FUNC_COMPLETE)
hisi_sas_release_task(hisi_hba, device);
}
diff --git a/drivers/scsi/libsas/sas_scsi_host.c b/drivers/scsi/libsas/sas_scsi_host.c
index ac669215c3bc..d6f29e13204e 100644
--- a/drivers/scsi/libsas/sas_scsi_host.c
+++ b/drivers/scsi/libsas/sas_scsi_host.c
@@ -1063,6 +1063,16 @@ int sas_clear_task_set(struct domain_device *dev, u8 *lun)
}
EXPORT_SYMBOL_GPL(sas_clear_task_set);

+int sas_lu_reset(struct domain_device *dev, u8 *lun)
+{
+ struct sas_tmf_task tmf_task = {
+ .tmf = TMF_LU_RESET,
+ };
+
+ return sas_execute_ssp_tmf(dev, lun, &tmf_task);
+}
+EXPORT_SYMBOL_GPL(sas_lu_reset);
+
/*
* Tell an upper layer that it needs to initiate an abort for a given task.
* This should only ever be called by an LLDD.
diff --git a/drivers/scsi/mvsas/mv_sas.c b/drivers/scsi/mvsas/mv_sas.c
index 37604b1ebd46..fdaaa4380e74 100644
--- a/drivers/scsi/mvsas/mv_sas.c
+++ b/drivers/scsi/mvsas/mv_sas.c
@@ -1381,13 +1381,11 @@ int mvs_lu_reset(struct domain_device *dev, u8 *lun)
{
unsigned long flags;
int rc = TMF_RESP_FUNC_FAILED;
- struct sas_tmf_task tmf_task;
struct mvs_device * mvi_dev = dev->lldd_dev;
struct mvs_info *mvi = mvi_dev->mvi_info;

- tmf_task.tmf = TMF_LU_RESET;
mvi_dev->dev_status = MVS_DEV_EH;
- rc = mvs_debug_issue_ssp_tmf(dev, lun, &tmf_task);
+ rc = sas_lu_reset(dev, lun);
if (rc == TMF_RESP_FUNC_COMPLETE) {
spin_lock_irqsave(&mvi->lock, flags);
mvs_release_task(mvi, dev);
diff --git a/drivers/scsi/pm8001/pm8001_sas.c b/drivers/scsi/pm8001/pm8001_sas.c
index fd86490616e8..18e8420055b5 100644
--- a/drivers/scsi/pm8001/pm8001_sas.c
+++ b/drivers/scsi/pm8001/pm8001_sas.c
@@ -1112,7 +1112,6 @@ int pm8001_I_T_nexus_event_handler(struct domain_device *dev)
int pm8001_lu_reset(struct domain_device *dev, u8 *lun)
{
int rc = TMF_RESP_FUNC_FAILED;
- struct sas_tmf_task tmf_task;
struct pm8001_device *pm8001_dev = dev->lldd_dev;
struct pm8001_hba_info *pm8001_ha = pm8001_find_ha_by_dev(dev);
DECLARE_COMPLETION_ONSTACK(completion_setstate);
@@ -1127,8 +1126,7 @@ int pm8001_lu_reset(struct domain_device *dev, u8 *lun)
pm8001_dev, DS_OPERATIONAL);
wait_for_completion(&completion_setstate);
} else {
- tmf_task.tmf = TMF_LU_RESET;
- rc = pm8001_issue_ssp_tmf(dev, lun, &tmf_task);
+ rc = sas_lu_reset(dev, lun);
}
/* If failed, fall-through I_T_Nexus reset */
pm8001_dbg(pm8001_ha, EH, "for device[%x]:rc=%d\n",
diff --git a/include/scsi/libsas.h b/include/scsi/libsas.h
index f71a47740ff8..7b1e2e7f5a6c 100644
--- a/include/scsi/libsas.h
+++ b/include/scsi/libsas.h
@@ -724,6 +724,7 @@ int sas_request_addr(struct Scsi_Host *shost, u8 *addr);

int sas_abort_task_set(struct domain_device *dev, u8 *lun);
int sas_clear_task_set(struct domain_device *dev, u8 *lun);
+int sas_lu_reset(struct domain_device *dev, u8 *lun);

int sas_notify_port_event(struct asd_sas_phy *phy, enum port_event event,
gfp_t gfp_flags);
--
2.26.2

2022-02-18 04:21:28

by Damien Le Moal

[permalink] [raw]
Subject: Re: [PATCH v2 00/18] scsi: libsas and users: Factor out LLDD TMF code

On 2/18/22 00:42, John Garry wrote:
> The LLDD TMF code is almost identical between hisi_sas, pm8001, and mvsas
> drivers.
>
> This series factors out that code into libsas, thus reducing much
> duplication and giving a net reduction of ~350 LoC.
>
> There are some subtle differences between the core TMF handler and each
> of the LLDDs old implementation, so any review and testing is appreciated.
>
> Some other minor patches are thrown in:
> - Delete unused macro in hisi_sas driver
> - Delete unused libsas callback
> - Delete unused SAS_SG_ERR
> - Add enum for response frame datapres field
> - Handle unrecognised errors in sas_scsi_find_task()
>
> I have another follow-up series to factor out the internal abort code,
> which is common to hisi_sas and pm8001 drivers.
>
> Based on mkp-scsi 5.18 staging queue at commit ac2beb4e3bd7

I tested this series with my pm8001 v5 series on top. Everything looks
good, no problems detected with both SAS and SATA drives tests. So feel
free to add:

Tested-by: Damien Le Moal <[email protected]>

Martin,

This series and my pm8001 series have a conflict. When applying the
pm8001 patches on top of these libsas changes, patch 28 has a fairly
easy to resolve conflict. Let me know if you want me to send a v6
rebased on top of this.

Thanks !


--
Damien Le Moal
Western Digital Research

2022-02-18 04:53:11

by Martin K. Petersen

[permalink] [raw]
Subject: Re: [PATCH v2 00/18] scsi: libsas and users: Factor out LLDD TMF code


Damien,

> This series and my pm8001 series have a conflict. When applying the
> pm8001 patches on top of these libsas changes, patch 28 has a fairly
> easy to resolve conflict. Let me know if you want me to send a v6
> rebased on top of this.

I'll fix it up.

--
Martin K. Petersen Oracle Linux Engineering

2022-02-20 14:06:25

by Martin K. Petersen

[permalink] [raw]
Subject: Re: [PATCH v2 00/18] scsi: libsas and users: Factor out LLDD TMF code


John,

> The LLDD TMF code is almost identical between hisi_sas, pm8001, and
> mvsas drivers.
>
> This series factors out that code into libsas, thus reducing much
> duplication and giving a net reduction of ~350 LoC.

Applied to 5.18/scsi-staging, thanks!

--
Martin K. Petersen Oracle Linux Engineering

2022-02-20 20:41:22

by Damien Le Moal

[permalink] [raw]
Subject: Re: [PATCH v2 00/18] scsi: libsas and users: Factor out LLDD TMF code

On 2/20/22 06:55, Martin K. Petersen wrote:
>
> John,
>
>> The LLDD TMF code is almost identical between hisi_sas, pm8001, and
>> mvsas drivers.
>>
>> This series factors out that code into libsas, thus reducing much
>> duplication and giving a net reduction of ~350 LoC.
>
> Applied to 5.18/scsi-staging, thanks!

Did you push this ? I do not see John series in the branch...


--
Damien Le Moal
Western Digital Research

2022-02-20 21:23:53

by Martin K. Petersen

[permalink] [raw]
Subject: Re: [PATCH v2 00/18] scsi: libsas and users: Factor out LLDD TMF code


Damien,

> This series and my pm8001 series have a conflict. When applying the
> pm8001 patches on top of these libsas changes, patch 28 has a fairly
> easy to resolve conflict. Let me know if you want me to send a v6
> rebased on top of this.

"fairly easy to resolve", huh? Sure, if you manually rework the entire
patch.

Please send me an updated version of #28. The rest of the series is
fine...

Thanks!

--
Martin K. Petersen Oracle Linux Engineering

2022-02-21 07:18:11

by Martin K. Petersen

[permalink] [raw]
Subject: Re: [PATCH v2 00/18] scsi: libsas and users: Factor out LLDD TMF code


Damien,

>> Applied to 5.18/scsi-staging, thanks!
>
> Did you push this ? I do not see John series in the branch...

It's there now.

--
Martin K. Petersen Oracle Linux Engineering

2022-02-21 09:13:26

by John Garry

[permalink] [raw]
Subject: Re: [PATCH v2 00/18] scsi: libsas and users: Factor out LLDD TMF code

On 19/02/2022 21:53, Martin K. Petersen wrote:
> Damien,
>
>> This series and my pm8001 series have a conflict. When applying the
>> pm8001 patches on top of these libsas changes, patch 28 has a fairly
>> easy to resolve conflict. Let me know if you want me to send a v6
>> rebased on top of this.
> "fairly easy to resolve", huh? Sure, if you manually rework the entire
> patch.
>
> Please send me an updated version of #28. The rest of the series is
> fine...

Thanks Martin and Damien. I'll mention potential conflicts in my cover
letters in future to help co-ordinate things better.

John

2022-02-21 09:22:41

by Damien Le Moal

[permalink] [raw]
Subject: Re: [PATCH v2 00/18] scsi: libsas and users: Factor out LLDD TMF code

On 2/20/22 10:31, Martin K. Petersen wrote:
>
> Damien,
>
>>> Applied to 5.18/scsi-staging, thanks!
>>
>> Did you push this ? I do not see John series in the branch...
>
> It's there now.
>

Got it. Thanks !

--
Damien Le Moal
Western Digital Research

2022-02-21 09:25:15

by Damien Le Moal

[permalink] [raw]
Subject: Re: [PATCH v2 00/18] scsi: libsas and users: Factor out LLDD TMF code

On 2/20/22 06:53, Martin K. Petersen wrote:
>
> Damien,
>
>> This series and my pm8001 series have a conflict. When applying the
>> pm8001 patches on top of these libsas changes, patch 28 has a fairly
>> easy to resolve conflict. Let me know if you want me to send a v6
>> rebased on top of this.
>
> "fairly easy to resolve", huh? Sure, if you manually rework the entire
> patch.

Sorry about that. It is easy to resolve once you have been staring at
the code for days :) Sending v6 for patch 28.

>
> Please send me an updated version of #28. The rest of the series is
> fine...
>
> Thanks!
>


--
Damien Le Moal
Western Digital Research