drivers/scsi/libsas/sas_scsi_host.c | 41 +++-------------------------- 1 file changed, 3 insertions(+), 38 deletions(-)
sas_queue_reset is always called with param "wait" set to 0, so
remove it from this function's param list. And remove unused
function sas_wait_eh.
Signed-off-by: Wenchao Hao <haowenchao2@huawei.com>
---
drivers/scsi/libsas/sas_scsi_host.c | 41 +++--------------------------
1 file changed, 3 insertions(+), 38 deletions(-)
diff --git a/drivers/scsi/libsas/sas_scsi_host.c b/drivers/scsi/libsas/sas_scsi_host.c
index 94c5f14f3c16..3f01e77eaee3 100644
--- a/drivers/scsi/libsas/sas_scsi_host.c
+++ b/drivers/scsi/libsas/sas_scsi_host.c
@@ -387,37 +387,7 @@ struct sas_phy *sas_get_local_phy(struct domain_device *dev)
}
EXPORT_SYMBOL_GPL(sas_get_local_phy);
-static void sas_wait_eh(struct domain_device *dev)
-{
- struct sas_ha_struct *ha = dev->port->ha;
- DEFINE_WAIT(wait);
-
- if (dev_is_sata(dev)) {
- ata_port_wait_eh(dev->sata_dev.ap);
- return;
- }
- retry:
- spin_lock_irq(&ha->lock);
-
- while (test_bit(SAS_DEV_EH_PENDING, &dev->state)) {
- prepare_to_wait(&ha->eh_wait_q, &wait, TASK_UNINTERRUPTIBLE);
- spin_unlock_irq(&ha->lock);
- schedule();
- spin_lock_irq(&ha->lock);
- }
- finish_wait(&ha->eh_wait_q, &wait);
-
- spin_unlock_irq(&ha->lock);
-
- /* make sure SCSI EH is complete */
- if (scsi_host_in_recovery(ha->core.shost)) {
- msleep(10);
- goto retry;
- }
-}
-
-static int sas_queue_reset(struct domain_device *dev, int reset_type,
- u64 lun, int wait)
+static int sas_queue_reset(struct domain_device *dev, int reset_type, u64 lun)
{
struct sas_ha_struct *ha = dev->port->ha;
int scheduled = 0, tries = 100;
@@ -425,8 +395,6 @@ static int sas_queue_reset(struct domain_device *dev, int reset_type,
/* ata: promote lun reset to bus reset */
if (dev_is_sata(dev)) {
sas_ata_schedule_reset(dev);
- if (wait)
- sas_ata_wait_eh(dev);
return SUCCESS;
}
@@ -444,9 +412,6 @@ static int sas_queue_reset(struct domain_device *dev, int reset_type,
}
spin_unlock_irq(&ha->lock);
- if (wait)
- sas_wait_eh(dev);
-
if (scheduled)
return SUCCESS;
}
@@ -499,7 +464,7 @@ int sas_eh_device_reset_handler(struct scsi_cmnd *cmd)
struct sas_internal *i = to_sas_internal(host->transportt);
if (current != host->ehandler)
- return sas_queue_reset(dev, SAS_DEV_LU_RESET, cmd->device->lun, 0);
+ return sas_queue_reset(dev, SAS_DEV_LU_RESET, cmd->device->lun);
int_to_scsilun(cmd->device->lun, &lun);
@@ -522,7 +487,7 @@ int sas_eh_target_reset_handler(struct scsi_cmnd *cmd)
struct sas_internal *i = to_sas_internal(host->transportt);
if (current != host->ehandler)
- return sas_queue_reset(dev, SAS_DEV_RESET, 0, 0);
+ return sas_queue_reset(dev, SAS_DEV_RESET, 0);
if (!i->dft->lldd_I_T_nexus_reset)
return FAILED;
--
2.32.0On Sat, 29 Jul 2023 18:24:51 +0800, Wenchao Hao wrote:
> sas_queue_reset is always called with param "wait" set to 0, so
> remove it from this function's param list. And remove unused
> function sas_wait_eh.
>
>
Applied to 6.6/scsi-queue, thanks!
[1/1] scsi:libsas: Simplify sas_queue_reset and remove unused code
https://git.kernel.org/mkp/scsi/c/be946e31bcf2
--
Martin K. Petersen Oracle Linux Engineering
On 2023/7/29 18:24, Wenchao Hao wrote:
> sas_queue_reset is always called with param "wait" set to 0, so
> remove it from this function's param list. And remove unused
> function sas_wait_eh.
>
Ping...
> Signed-off-by: Wenchao Hao <haowenchao2@huawei.com>
> ---
> drivers/scsi/libsas/sas_scsi_host.c | 41 +++--------------------------
> 1 file changed, 3 insertions(+), 38 deletions(-)
>
> diff --git a/drivers/scsi/libsas/sas_scsi_host.c b/drivers/scsi/libsas/sas_scsi_host.c
> index 94c5f14f3c16..3f01e77eaee3 100644
> --- a/drivers/scsi/libsas/sas_scsi_host.c
> +++ b/drivers/scsi/libsas/sas_scsi_host.c
> @@ -387,37 +387,7 @@ struct sas_phy *sas_get_local_phy(struct domain_device *dev)
> }
> EXPORT_SYMBOL_GPL(sas_get_local_phy);
>
> -static void sas_wait_eh(struct domain_device *dev)
> -{
> - struct sas_ha_struct *ha = dev->port->ha;
> - DEFINE_WAIT(wait);
> -
> - if (dev_is_sata(dev)) {
> - ata_port_wait_eh(dev->sata_dev.ap);
> - return;
> - }
> - retry:
> - spin_lock_irq(&ha->lock);
> -
> - while (test_bit(SAS_DEV_EH_PENDING, &dev->state)) {
> - prepare_to_wait(&ha->eh_wait_q, &wait, TASK_UNINTERRUPTIBLE);
> - spin_unlock_irq(&ha->lock);
> - schedule();
> - spin_lock_irq(&ha->lock);
> - }
> - finish_wait(&ha->eh_wait_q, &wait);
> -
> - spin_unlock_irq(&ha->lock);
> -
> - /* make sure SCSI EH is complete */
> - if (scsi_host_in_recovery(ha->core.shost)) {
> - msleep(10);
> - goto retry;
> - }
> -}
> -
> -static int sas_queue_reset(struct domain_device *dev, int reset_type,
> - u64 lun, int wait)
> +static int sas_queue_reset(struct domain_device *dev, int reset_type, u64 lun)
> {
> struct sas_ha_struct *ha = dev->port->ha;
> int scheduled = 0, tries = 100;
> @@ -425,8 +395,6 @@ static int sas_queue_reset(struct domain_device *dev, int reset_type,
> /* ata: promote lun reset to bus reset */
> if (dev_is_sata(dev)) {
> sas_ata_schedule_reset(dev);
> - if (wait)
> - sas_ata_wait_eh(dev);
> return SUCCESS;
> }
>
> @@ -444,9 +412,6 @@ static int sas_queue_reset(struct domain_device *dev, int reset_type,
> }
> spin_unlock_irq(&ha->lock);
>
> - if (wait)
> - sas_wait_eh(dev);
> -
> if (scheduled)
> return SUCCESS;
> }
> @@ -499,7 +464,7 @@ int sas_eh_device_reset_handler(struct scsi_cmnd *cmd)
> struct sas_internal *i = to_sas_internal(host->transportt);
>
> if (current != host->ehandler)
> - return sas_queue_reset(dev, SAS_DEV_LU_RESET, cmd->device->lun, 0);
> + return sas_queue_reset(dev, SAS_DEV_LU_RESET, cmd->device->lun);
>
> int_to_scsilun(cmd->device->lun, &lun);
>
> @@ -522,7 +487,7 @@ int sas_eh_target_reset_handler(struct scsi_cmnd *cmd)
> struct sas_internal *i = to_sas_internal(host->transportt);
>
> if (current != host->ehandler)
> - return sas_queue_reset(dev, SAS_DEV_RESET, 0, 0);
> + return sas_queue_reset(dev, SAS_DEV_RESET, 0);
>
> if (!i->dft->lldd_I_T_nexus_reset)
> return FAILED;
>> sas_queue_reset is always called with param "wait" set to 0, so >> remove it from this function's param list. And remove unused function >> sas_wait_eh. > > Ping... Applied to 6.6/scsi-staging, thanks! -- Martin K. Petersen Oracle Linux Engineering
On 7/29/23 19:24, Wenchao Hao wrote: > sas_queue_reset is always called with param "wait" set to 0, so > remove it from this function's param list. And remove unused > function sas_wait_eh. > > Signed-off-by: Wenchao Hao <haowenchao2@huawei.com> Looks OK. Reviewed-by: Damien Le Moal <dlemoal@kernel.org> -- Damien Le Moal Western Digital Research
Hi Wenchao, On 2023/7/29 18:24, Wenchao Hao wrote: > sas_queue_reset is always called with param "wait" set to 0, so > remove it from this function's param list. And remove unused > function sas_wait_eh. I did not find any users passing '1' to sas_queue_reset() in the git history. It seems it is never used. So It's ok to remove it, I guess. Just one nit, there should be a blank between "scsi:" and "libsas" in the subject: scsi: libsas: Simplify sas_queue_reset and remove unused code Otherwise looks good to me: Reviewed-by: Jason Yan <yanaijie@huawei.com>
On 7/31/23 10:44, Jason Yan wrote: > Hi Wenchao, > > On 2023/7/29 18:24, Wenchao Hao wrote: >> sas_queue_reset is always called with param "wait" set to 0, so >> remove it from this function's param list. And remove unused >> function sas_wait_eh. > > I did not find any users passing '1' to sas_queue_reset() in the git > history. It seems it is never used. So It's ok to remove it, I guess. > > Just one nit, there should be a blank between "scsi:" and "libsas" in > the subject: > > scsi: libsas: Simplify sas_queue_reset and remove unused code > > Otherwise looks good to me: > Reviewed-by: Jason Yan <yanaijie@huawei.com> Yeah, code wise, it looks good. However, I am seeing issues with completions for commands that use command duration limits. There are some unusual completions that needs special handling with CDL (e.g. some aborted commands can be notified with a success and sense-data-available-bit set. For these, we kick libata-EH but it seems that this is not well working with libsas. So I wonder if this code may need to be used for CDL... So let's please hold on before applying this. Let me check the CDL issues I am seeing first. -- Damien Le Moal Western Digital Research
On 2023/7/31 10:05, Damien Le Moal wrote: > On 7/31/23 10:44, Jason Yan wrote: >> Hi Wenchao, >> >> On 2023/7/29 18:24, Wenchao Hao wrote: >>> sas_queue_reset is always called with param "wait" set to 0, so >>> remove it from this function's param list. And remove unused >>> function sas_wait_eh. >> >> I did not find any users passing '1' to sas_queue_reset() in the git >> history. It seems it is never used. So It's ok to remove it, I guess. >> >> Just one nit, there should be a blank between "scsi:" and "libsas" in >> the subject: >> >> scsi: libsas: Simplify sas_queue_reset and remove unused code >> >> Otherwise looks good to me: >> Reviewed-by: Jason Yan <yanaijie@huawei.com> > > Yeah, code wise, it looks good. > However, I am seeing issues with completions for commands that use command > duration limits. There are some unusual completions that needs special handling > with CDL (e.g. some aborted commands can be notified with a success and > sense-data-available-bit set. For these, we kick libata-EH but it seems that > this is not well working with libsas. So I wonder if this code may need to be > used for CDL... So let's please hold on before applying this. Let me check the > CDL issues I am seeing first. > Sure. It's just a cleanup. Let's hold on until it is confirmed. Thanks, Jason
© 2016 - 2026 Red Hat, Inc.