* [PATCH 0/2] Some small fixes for hisi_sas @ 2024-04-01 5:49 chenxiang 2024-04-01 5:49 ` [PATCH 1/2] scsi: hisi_sas: Handle the NCQ error returned by D2H frame chenxiang 2024-04-01 5:49 ` [PATCH 2/2] scsi: hisi_sas: Modify the deadline for ata_wait_after_reset() chenxiang 0 siblings, 2 replies; 7+ messages in thread From: chenxiang @ 2024-04-01 5:49 UTC (permalink / raw) To: jejb, martin.petersen; +Cc: linuxarm, linux-scsi From: Xiang Chen <chenxiang66@hisilicon.com> This series contain two small fixes including: - Handle the NCQ error returned by D2H frame; - Correct the deadline value for ata_wait_after_reset() in function hisi_sas_debug_I_T_nexus_reset; Xingui Yang (1): scsi: hisi_sas: Handle the NCQ error returned by D2H frame Yihang Li (1): scsi: hisi_sas: Modify the deadline for ata_wait_after_reset() drivers/scsi/hisi_sas/hisi_sas_main.c | 4 +++- drivers/scsi/hisi_sas/hisi_sas_v3_hw.c | 9 ++++++++- 2 files changed, 11 insertions(+), 2 deletions(-) -- 2.30.0 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/2] scsi: hisi_sas: Handle the NCQ error returned by D2H frame 2024-04-01 5:49 [PATCH 0/2] Some small fixes for hisi_sas chenxiang @ 2024-04-01 5:49 ` chenxiang 2024-04-01 7:10 ` Damien Le Moal 2024-04-01 5:49 ` [PATCH 2/2] scsi: hisi_sas: Modify the deadline for ata_wait_after_reset() chenxiang 1 sibling, 1 reply; 7+ messages in thread From: chenxiang @ 2024-04-01 5:49 UTC (permalink / raw) To: jejb, martin.petersen; +Cc: linuxarm, linux-scsi From: Xingui Yang <yangxingui@huawei.com> We find that some disks use D2H frame instead of SDB frame to return NCQ error. Currently, only the I/O corresponding to the D2H frame is processed in this scenario, which does not meet the processing requirements of the NCQ error scenario. So we set dev_status to HISI_SAS_DEV_NCQ_ERR and abort all I/Os of the disk in this scenario. Signed-off-by: Xingui Yang <yangxingui@huawei.com> Signed-off-by: Xiang Chen <chenxiang66@hisilicon.com> --- drivers/scsi/hisi_sas/hisi_sas_v3_hw.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c index 7d2a33514538..3935fa6bc72b 100644 --- a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c +++ b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c @@ -2244,7 +2244,14 @@ slot_err_v3_hw(struct hisi_hba *hisi_hba, struct sas_task *task, case SAS_PROTOCOL_SATA | SAS_PROTOCOL_STP: if ((dw0 & CMPLT_HDR_RSPNS_XFRD_MSK) && (sipc_rx_err_type & RX_FIS_STATUS_ERR_MSK)) { - ts->stat = SAS_PROTO_RESPONSE; + if (task->ata_task.use_ncq) { + struct domain_device *device = task->dev; + struct hisi_sas_device *sas_dev = + device->lldd_dev; + sas_dev->dev_status = HISI_SAS_DEV_NCQ_ERR; + slot->abort = 1; + } else + ts->stat = SAS_PROTO_RESPONSE; } else if (dma_rx_err_type & RX_DATA_LEN_UNDERFLOW_MSK) { ts->residual = trans_tx_fail_type; ts->stat = SAS_DATA_UNDERRUN; -- 2.30.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] scsi: hisi_sas: Handle the NCQ error returned by D2H frame 2024-04-01 5:49 ` [PATCH 1/2] scsi: hisi_sas: Handle the NCQ error returned by D2H frame chenxiang @ 2024-04-01 7:10 ` Damien Le Moal 2024-04-02 1:10 ` chenxiang (M) 0 siblings, 1 reply; 7+ messages in thread From: Damien Le Moal @ 2024-04-01 7:10 UTC (permalink / raw) To: chenxiang, jejb, martin.petersen; +Cc: linuxarm, linux-scsi On 4/1/24 14:49, chenxiang wrote: > From: Xingui Yang <yangxingui@huawei.com> > > We find that some disks use D2H frame instead of SDB frame to return NCQ > error. Currently, only the I/O corresponding to the D2H frame is processed > in this scenario, which does not meet the processing requirements of the > NCQ error scenario. > So we set dev_status to HISI_SAS_DEV_NCQ_ERR and abort all I/Os of the disk > in this scenario. > > Signed-off-by: Xingui Yang <yangxingui@huawei.com> > Signed-off-by: Xiang Chen <chenxiang66@hisilicon.com> > --- > drivers/scsi/hisi_sas/hisi_sas_v3_hw.c | 9 ++++++++- > 1 file changed, 8 insertions(+), 1 deletion(-) > > diff --git a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c > index 7d2a33514538..3935fa6bc72b 100644 > --- a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c > +++ b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c > @@ -2244,7 +2244,14 @@ slot_err_v3_hw(struct hisi_hba *hisi_hba, struct sas_task *task, > case SAS_PROTOCOL_SATA | SAS_PROTOCOL_STP: > if ((dw0 & CMPLT_HDR_RSPNS_XFRD_MSK) && > (sipc_rx_err_type & RX_FIS_STATUS_ERR_MSK)) { > - ts->stat = SAS_PROTO_RESPONSE; > + if (task->ata_task.use_ncq) { > + struct domain_device *device = task->dev;> + struct hisi_sas_device *sas_dev = > + device->lldd_dev; Missing blank line after the declaration. And why the line split for the above declaration ? That fits in 80 chars line... > + sas_dev->dev_status = HISI_SAS_DEV_NCQ_ERR; > + slot->abort = 1; > + } else > + ts->stat = SAS_PROTO_RESPONSE; Missing the curly brackets here. > } else if (dma_rx_err_type & RX_DATA_LEN_UNDERFLOW_MSK) { > ts->residual = trans_tx_fail_type; > ts->stat = SAS_DATA_UNDERRUN; -- Damien Le Moal Western Digital Research ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] scsi: hisi_sas: Handle the NCQ error returned by D2H frame 2024-04-01 7:10 ` Damien Le Moal @ 2024-04-02 1:10 ` chenxiang (M) 0 siblings, 0 replies; 7+ messages in thread From: chenxiang (M) @ 2024-04-02 1:10 UTC (permalink / raw) To: Damien Le Moal, jejb, martin.petersen; +Cc: linuxarm, linux-scsi Hi Damien, 在 2024/4/1 星期一 15:10, Damien Le Moal 写道: > On 4/1/24 14:49, chenxiang wrote: >> From: Xingui Yang <yangxingui@huawei.com> >> >> We find that some disks use D2H frame instead of SDB frame to return NCQ >> error. Currently, only the I/O corresponding to the D2H frame is processed >> in this scenario, which does not meet the processing requirements of the >> NCQ error scenario. >> So we set dev_status to HISI_SAS_DEV_NCQ_ERR and abort all I/Os of the disk >> in this scenario. >> >> Signed-off-by: Xingui Yang <yangxingui@huawei.com> >> Signed-off-by: Xiang Chen <chenxiang66@hisilicon.com> >> --- >> drivers/scsi/hisi_sas/hisi_sas_v3_hw.c | 9 ++++++++- >> 1 file changed, 8 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c >> index 7d2a33514538..3935fa6bc72b 100644 >> --- a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c >> +++ b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c >> @@ -2244,7 +2244,14 @@ slot_err_v3_hw(struct hisi_hba *hisi_hba, struct sas_task *task, >> case SAS_PROTOCOL_SATA | SAS_PROTOCOL_STP: >> if ((dw0 & CMPLT_HDR_RSPNS_XFRD_MSK) && >> (sipc_rx_err_type & RX_FIS_STATUS_ERR_MSK)) { >> - ts->stat = SAS_PROTO_RESPONSE; >> + if (task->ata_task.use_ncq) { >> + struct domain_device *device = task->dev;> + struct hisi_sas_device *sas_dev = >> + device->lldd_dev; > Missing blank line after the declaration. And why the line split for the above > declaration ? That fits in 80 chars line... Will change it in next version. > >> + sas_dev->dev_status = HISI_SAS_DEV_NCQ_ERR; >> + slot->abort = 1; >> + } else >> + ts->stat = SAS_PROTO_RESPONSE; > Missing the curly brackets here. Will change it in next version. > >> } else if (dma_rx_err_type & RX_DATA_LEN_UNDERFLOW_MSK) { >> ts->residual = trans_tx_fail_type; >> ts->stat = SAS_DATA_UNDERRUN; ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 2/2] scsi: hisi_sas: Modify the deadline for ata_wait_after_reset() 2024-04-01 5:49 [PATCH 0/2] Some small fixes for hisi_sas chenxiang 2024-04-01 5:49 ` [PATCH 1/2] scsi: hisi_sas: Handle the NCQ error returned by D2H frame chenxiang @ 2024-04-01 5:49 ` chenxiang 2024-04-01 7:08 ` Damien Le Moal 1 sibling, 1 reply; 7+ messages in thread From: chenxiang @ 2024-04-01 5:49 UTC (permalink / raw) To: jejb, martin.petersen; +Cc: linuxarm, linux-scsi From: Yihang Li <liyihang9@huawei.com> We found that the second parameter of function ata_wait_after_reset() is incorrectly used. We call smp_ata_check_ready_type() to poll the device type until the 30s timeout, so the correct deadline should be (jiffies + 30000). Fixes: 3c2673a09cf1 ("scsi: hisi_sas: Fix SATA devices missing issue during I_T nexus reset") Signed-off-by: xiabing <xiabing12@h-partners.com> Signed-off-by: Yihang Li <liyihang9@huawei.com> Signed-off-by: Xiang Chen <chenxiang66@hisilicon.com> --- drivers/scsi/hisi_sas/hisi_sas_main.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/scsi/hisi_sas/hisi_sas_main.c b/drivers/scsi/hisi_sas/hisi_sas_main.c index 097dfe4b620d..7245600aedb2 100644 --- a/drivers/scsi/hisi_sas/hisi_sas_main.c +++ b/drivers/scsi/hisi_sas/hisi_sas_main.c @@ -1796,8 +1796,10 @@ static int hisi_sas_debug_I_T_nexus_reset(struct domain_device *device) if (dev_is_sata(device)) { struct ata_link *link = &device->sata_dev.ap->link; + unsigned long deadline = ata_deadline(jiffies, + HISI_SAS_WAIT_PHYUP_TIMEOUT / HZ * 1000); - rc = ata_wait_after_reset(link, HISI_SAS_WAIT_PHYUP_TIMEOUT, + rc = ata_wait_after_reset(link, deadline, smp_ata_check_ready_type); } else { msleep(2000); -- 2.30.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] scsi: hisi_sas: Modify the deadline for ata_wait_after_reset() 2024-04-01 5:49 ` [PATCH 2/2] scsi: hisi_sas: Modify the deadline for ata_wait_after_reset() chenxiang @ 2024-04-01 7:08 ` Damien Le Moal 2024-04-02 1:51 ` chenxiang (M) 0 siblings, 1 reply; 7+ messages in thread From: Damien Le Moal @ 2024-04-01 7:08 UTC (permalink / raw) To: chenxiang, jejb, martin.petersen; +Cc: linuxarm, linux-scsi On 4/1/24 14:49, chenxiang wrote: > From: Yihang Li <liyihang9@huawei.com> > > We found that the second parameter of function ata_wait_after_reset() is > incorrectly used. We call smp_ata_check_ready_type() to poll the device > type until the 30s timeout, so the correct deadline should be (jiffies + > 30000). > > Fixes: 3c2673a09cf1 ("scsi: hisi_sas: Fix SATA devices missing issue during I_T nexus reset") > Signed-off-by: xiabing <xiabing12@h-partners.com> > Signed-off-by: Yihang Li <liyihang9@huawei.com> > Signed-off-by: Xiang Chen <chenxiang66@hisilicon.com> > --- > drivers/scsi/hisi_sas/hisi_sas_main.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/scsi/hisi_sas/hisi_sas_main.c b/drivers/scsi/hisi_sas/hisi_sas_main.c > index 097dfe4b620d..7245600aedb2 100644 > --- a/drivers/scsi/hisi_sas/hisi_sas_main.c > +++ b/drivers/scsi/hisi_sas/hisi_sas_main.c > @@ -1796,8 +1796,10 @@ static int hisi_sas_debug_I_T_nexus_reset(struct domain_device *device) > > if (dev_is_sata(device)) { > struct ata_link *link = &device->sata_dev.ap->link; > + unsigned long deadline = ata_deadline(jiffies, > + HISI_SAS_WAIT_PHYUP_TIMEOUT / HZ * 1000); At the very least, you should use jiffies_to_msecs() here. But I do not see the point though given that ata_deadline will do "jiffies + msecs_to_jiffies()". So may be calling: rc = ata_wait_after_reset(link, jiffies + HISI_SAS_WAIT_PHYUP_TIMEOUT, smp_ata_check_ready_type); May be simpler and more readable. > > - rc = ata_wait_after_reset(link, HISI_SAS_WAIT_PHYUP_TIMEOUT, > + rc = ata_wait_after_reset(link, deadline, > smp_ata_check_ready_type); > } else { > msleep(2000); -- Damien Le Moal Western Digital Research ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] scsi: hisi_sas: Modify the deadline for ata_wait_after_reset() 2024-04-01 7:08 ` Damien Le Moal @ 2024-04-02 1:51 ` chenxiang (M) 0 siblings, 0 replies; 7+ messages in thread From: chenxiang (M) @ 2024-04-02 1:51 UTC (permalink / raw) To: Damien Le Moal, jejb, martin.petersen; +Cc: linuxarm, linux-scsi Hi Damien, 在 2024/4/1 星期一 15:08, Damien Le Moal 写道: > On 4/1/24 14:49, chenxiang wrote: >> From: Yihang Li <liyihang9@huawei.com> >> >> We found that the second parameter of function ata_wait_after_reset() is >> incorrectly used. We call smp_ata_check_ready_type() to poll the device >> type until the 30s timeout, so the correct deadline should be (jiffies + >> 30000). >> >> Fixes: 3c2673a09cf1 ("scsi: hisi_sas: Fix SATA devices missing issue during I_T nexus reset") >> Signed-off-by: xiabing <xiabing12@h-partners.com> >> Signed-off-by: Yihang Li <liyihang9@huawei.com> >> Signed-off-by: Xiang Chen <chenxiang66@hisilicon.com> >> --- >> drivers/scsi/hisi_sas/hisi_sas_main.c | 4 +++- >> 1 file changed, 3 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/scsi/hisi_sas/hisi_sas_main.c b/drivers/scsi/hisi_sas/hisi_sas_main.c >> index 097dfe4b620d..7245600aedb2 100644 >> --- a/drivers/scsi/hisi_sas/hisi_sas_main.c >> +++ b/drivers/scsi/hisi_sas/hisi_sas_main.c >> @@ -1796,8 +1796,10 @@ static int hisi_sas_debug_I_T_nexus_reset(struct domain_device *device) >> >> if (dev_is_sata(device)) { >> struct ata_link *link = &device->sata_dev.ap->link; >> + unsigned long deadline = ata_deadline(jiffies, >> + HISI_SAS_WAIT_PHYUP_TIMEOUT / HZ * 1000); > At the very least, you should use jiffies_to_msecs() here. But I do not see the > point though given that ata_deadline will do "jiffies + msecs_to_jiffies()". We found the issue when reading the code of libata-eh which uses ata_deadline() and ata_wait_after_reset(). So we modify it as above. > > So may be calling: > > rc = ata_wait_after_reset(link, jiffies + HISI_SAS_WAIT_PHYUP_TIMEOUT, > smp_ata_check_ready_type); > > May be simpler and more readable. OK, it is more simpler and readable. Will change it in next version. > >> >> - rc = ata_wait_after_reset(link, HISI_SAS_WAIT_PHYUP_TIMEOUT, >> + rc = ata_wait_after_reset(link, deadline, >> smp_ata_check_ready_type); >> } else { >> msleep(2000); ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2024-04-02 1:51 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-04-01 5:49 [PATCH 0/2] Some small fixes for hisi_sas chenxiang 2024-04-01 5:49 ` [PATCH 1/2] scsi: hisi_sas: Handle the NCQ error returned by D2H frame chenxiang 2024-04-01 7:10 ` Damien Le Moal 2024-04-02 1:10 ` chenxiang (M) 2024-04-01 5:49 ` [PATCH 2/2] scsi: hisi_sas: Modify the deadline for ata_wait_after_reset() chenxiang 2024-04-01 7:08 ` Damien Le Moal 2024-04-02 1:51 ` chenxiang (M)
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox