git: d353c342a12c - stable/13 - isp: Improve task aborts handling
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Mon, 23 Dec 2024 14:34:59 UTC
The branch stable/13 has been updated by mav:
URL: https://cgit.FreeBSD.org/src/commit/?id=d353c342a12cd346c79c074660709657f7a741ef
commit d353c342a12cd346c79c074660709657f7a741ef
Author: Alexander Motin <mav@FreeBSD.org>
AuthorDate: 2024-12-09 16:52:26 +0000
Commit: Alexander Motin <mav@FreeBSD.org>
CommitDate: 2024-12-23 14:34:50 +0000
isp: Improve task aborts handling
- When handling notify acknowledge from target code for task abort
request, not only send abort to the firmware, but also delete the
ATIO private data associated with the command. It is required for
proper tag reuse, allowing new "conflicting" commands to be passed
to the target. CTL was already fixed to handle that right, instead
of delaying them in restart queue of the driver.
- When target finally aborts the command (which it should have
done before the notify ack) we should not send another abort to
the firmware. Since we already sent the abort and deleted ATIO
private data above, just return successful completion here, doing
nothing. Since the tag can be reused by that time, we can not
rely on its uniqueness, so when searching to the ATIO private data
compare also the aborted CCB pointer in addition to the tag.
- Fix BA_RJT sending in isp_acknak_abts(). While it should be
rare, teach the code to send error responses for ABTS requests.
MFC after: 2 weeks
(cherry picked from commit ec3175fc3b2c1b07144a85e6a3e2fe534acc0c87)
---
sys/dev/isp/isp_freebsd.c | 142 +++++++++++++++++++++-------------------------
sys/dev/isp/isp_freebsd.h | 9 ++-
sys/dev/isp/isp_target.c | 11 ++--
3 files changed, 77 insertions(+), 85 deletions(-)
diff --git a/sys/dev/isp/isp_freebsd.c b/sys/dev/isp/isp_freebsd.c
index 51c1e3abb6de..d5aa7a54142e 100644
--- a/sys/dev/isp/isp_freebsd.c
+++ b/sys/dev/isp/isp_freebsd.c
@@ -626,8 +626,9 @@ isp_free_pcmd(ispsoftc_t *isp, union ccb *ccb)
*/
#ifdef ISP_TARGET_MODE
static ISP_INLINE tstate_t *get_lun_statep(ispsoftc_t *, int, lun_id_t);
-static atio_private_data_t *isp_get_atpd(ispsoftc_t *, int, uint32_t);
+static atio_private_data_t *isp_get_atpd(ispsoftc_t *, int, uint32_t, void *);
static atio_private_data_t *isp_find_atpd(ispsoftc_t *, int, uint32_t);
+static atio_private_data_t *isp_find_atpd_ccb(ispsoftc_t *, int, uint32_t, void *);
static void isp_put_atpd(ispsoftc_t *, int, atio_private_data_t *);
static inot_private_data_t *isp_get_ntpd(ispsoftc_t *, int);
static inot_private_data_t *isp_find_ntpd(ispsoftc_t *, int, uint32_t, uint32_t);
@@ -715,7 +716,7 @@ isp_tmcmd_restart(ispsoftc_t *isp)
}
static atio_private_data_t *
-isp_get_atpd(ispsoftc_t *isp, int chan, uint32_t tag)
+isp_get_atpd(ispsoftc_t *isp, int chan, uint32_t tag, void *ccb)
{
struct isp_fc *fc = ISP_FC_PC(isp, chan);
atio_private_data_t *atp;
@@ -723,6 +724,7 @@ isp_get_atpd(ispsoftc_t *isp, int chan, uint32_t tag)
atp = LIST_FIRST(&fc->atfree);
if (atp) {
LIST_REMOVE(atp, next);
+ atp->ccb = ccb;
atp->tag = tag;
LIST_INSERT_HEAD(&fc->atused[ATPDPHASH(tag)], atp, next);
}
@@ -742,6 +744,23 @@ isp_find_atpd(ispsoftc_t *isp, int chan, uint32_t tag)
return (NULL);
}
+/*
+ * Similar to above, but in addition to tag searches for opaque CCB pointer,
+ * It can be used in situations when the tag alone may already be reused.
+ */
+static atio_private_data_t *
+isp_find_atpd_ccb(ispsoftc_t *isp, int chan, uint32_t tag, void *ccb)
+{
+ struct isp_fc *fc = ISP_FC_PC(isp, chan);
+ atio_private_data_t *atp;
+
+ LIST_FOREACH(atp, &fc->atused[ATPDPHASH(tag)], next) {
+ if (atp->tag == tag && atp->ccb == ccb)
+ return (atp);
+ }
+ return (NULL);
+}
+
static void
isp_put_atpd(ispsoftc_t *isp, int chan, atio_private_data_t *atp)
{
@@ -967,16 +986,6 @@ isp_target_start_ctio(ispsoftc_t *isp, union ccb *ccb, enum Start_Ctio_How how)
continue;
}
- /*
- * Is this command a dead duck?
- */
- if (atp->dead) {
- isp_prt(isp, ISP_LOGERR, "%s: [0x%x] not sending a CTIO for a dead command", __func__, cso->tag_id);
- ccb->ccb_h.status = CAM_REQ_ABORTED;
- xpt_done(ccb);
- continue;
- }
-
/*
* Check to make sure we're still in target mode.
*/
@@ -1378,7 +1387,7 @@ isp_handle_platform_atio7(ispsoftc_t *isp, at7_entry_t *aep)
*/
goto noresrc;
}
- atp = isp_get_atpd(isp, chan, aep->at_rxid);
+ atp = isp_get_atpd(isp, chan, aep->at_rxid, atiop);
if (atp == NULL) {
isp_prt(isp, ISP_LOGTDEBUG0, "[0x%x] out of atps", aep->at_rxid);
isp_endcmd(isp, aep, nphdl, chan, SCSI_BUSY, 0);
@@ -1733,32 +1742,8 @@ isp_handle_platform_target_notify_ack(ispsoftc_t *isp, isp_notify_t *mp, uint32_
/*
* This case is for a responding to an ABTS frame
*/
- if (mp->nt_lreserved && ((isphdr_t *)mp->nt_lreserved)->rqs_entry_type == RQSTYPE_ABTS_RCVD) {
-
- /*
- * Overload nt_need_ack here to mark whether we've terminated the associated command.
- */
- if (mp->nt_need_ack) {
- abts_t *abts = (abts_t *)mp->nt_lreserved;
-
- ISP_MEMZERO(cto, sizeof (ct7_entry_t));
- isp_prt(isp, ISP_LOGTDEBUG0, "%s: [%x] terminating after ABTS received", __func__, abts->abts_rxid_task);
- cto->ct_header.rqs_entry_type = RQSTYPE_CTIO7;
- cto->ct_header.rqs_entry_count = 1;
- cto->ct_nphdl = mp->nt_nphdl;
- cto->ct_rxid = abts->abts_rxid_task;
- cto->ct_iid_lo = mp->nt_sid;
- cto->ct_iid_hi = mp->nt_sid >> 16;
- cto->ct_oxid = abts->abts_ox_id;
- cto->ct_vpidx = mp->nt_channel;
- cto->ct_flags = CT7_NOACK|CT7_TERMINATE;
- if (isp_send_entry(isp, cto)) {
- return (ENOMEM);
- }
- mp->nt_need_ack = 0;
- }
- return (isp_acknak_abts(isp, mp->nt_lreserved, 0));
- }
+ if (mp->nt_lreserved && ((isphdr_t *)mp->nt_lreserved)->rqs_entry_type == RQSTYPE_ABTS_RCVD)
+ return (isp_acknak_abts(isp, mp->nt_lreserved, (rsp == 0) ? 0 : EINVAL));
/*
* General purpose acknowledgement
@@ -1890,17 +1875,15 @@ bad:
}
}
+/*
+ * Clean aborted commands pending restart
+ */
static void
isp_target_mark_aborted_early(ispsoftc_t *isp, int chan, tstate_t *tptr, uint32_t tag_id)
{
- struct isp_fc *fc = ISP_FC_PC(isp, chan);
- atio_private_data_t *atp;
inot_private_data_t *ntp, *tmp;
uint32_t this_tag_id;
- /*
- * First, clean any commands pending restart
- */
STAILQ_FOREACH_SAFE(ntp, &tptr->restart_queue, next, tmp) {
this_tag_id = ((at7_entry_t *)ntp->data)->at_rxid;
if ((uint64_t)tag_id == TAG_ANY || tag_id == this_tag_id) {
@@ -1911,16 +1894,6 @@ isp_target_mark_aborted_early(ispsoftc_t *isp, int chan, tstate_t *tptr, uint32_
isp_put_ntpd(isp, chan, ntp);
}
}
-
- /*
- * Now mark other ones dead as well.
- */
- for (atp = fc->atpool; atp < &fc->atpool[ATPDPSIZE]; atp++) {
- if (atp->lun != tptr->ts_lun)
- continue;
- if ((uint64_t)tag_id == TAG_ANY || atp->tag == tag_id)
- atp->dead = 1;
- }
}
#endif
@@ -2283,6 +2256,25 @@ isp_kthread(void *arg)
}
#ifdef ISP_TARGET_MODE
+static int
+isp_abort_atpd(ispsoftc_t *isp, int chan, atio_private_data_t *atp)
+{
+ uint8_t storage[QENTRY_LEN];
+ ct7_entry_t *cto = (ct7_entry_t *) storage;
+
+ ISP_MEMZERO(cto, sizeof (ct7_entry_t));
+ cto->ct_header.rqs_entry_type = RQSTYPE_CTIO7;
+ cto->ct_header.rqs_entry_count = 1;
+ cto->ct_nphdl = atp->nphdl;
+ cto->ct_vpidx = chan;
+ cto->ct_iid_lo = atp->sid;
+ cto->ct_iid_hi = atp->sid >> 16;
+ cto->ct_rxid = atp->tag;
+ cto->ct_flags = CT7_NOACK|CT7_TERMINATE;
+ cto->ct_oxid = atp->oxid;
+ return (isp_send_entry(isp, cto));
+}
+
static void
isp_abort_atio(ispsoftc_t *isp, union ccb *ccb)
{
@@ -2308,30 +2300,16 @@ isp_abort_atio(ispsoftc_t *isp, union ccb *ccb)
}
/* Search for the ATIO among running. */
- atp = isp_find_atpd(isp, XS_CHANNEL(accb), accb->atio.tag_id);
+ atp = isp_find_atpd_ccb(isp, XS_CHANNEL(accb), accb->atio.tag_id, accb);
if (atp != NULL) {
- /* Send TERMINATE to firmware. */
- if (!atp->dead) {
- uint8_t storage[QENTRY_LEN];
- ct7_entry_t *cto = (ct7_entry_t *) storage;
-
- ISP_MEMZERO(cto, sizeof (ct7_entry_t));
- cto->ct_header.rqs_entry_type = RQSTYPE_CTIO7;
- cto->ct_header.rqs_entry_count = 1;
- cto->ct_nphdl = atp->nphdl;
- cto->ct_rxid = atp->tag;
- cto->ct_iid_lo = atp->sid;
- cto->ct_iid_hi = atp->sid >> 16;
- cto->ct_oxid = atp->oxid;
- cto->ct_vpidx = XS_CHANNEL(accb);
- cto->ct_flags = CT7_NOACK|CT7_TERMINATE;
- isp_send_entry(isp, cto);
+ if (isp_abort_atpd(isp, XS_CHANNEL(accb), atp)) {
+ ccb->ccb_h.status = CAM_UA_ABORT;
+ return;
}
isp_put_atpd(isp, XS_CHANNEL(accb), atp);
- ccb->ccb_h.status = CAM_REQ_CMP;
- } else {
- ccb->ccb_h.status = CAM_UA_ABORT;
}
+
+ ccb->ccb_h.status = CAM_REQ_CMP;
}
static void
@@ -2504,6 +2482,7 @@ isp_action(struct cam_sim *sim, union ccb *ccb)
}
case XPT_NOTIFY_ACKNOWLEDGE: /* notify ack */
{
+ atio_private_data_t *atp;
inot_private_data_t *ntp;
/*
@@ -2522,8 +2501,19 @@ isp_action(struct cam_sim *sim, union ccb *ccb)
xpt_done(ccb);
break;
}
- if (isp_handle_platform_target_notify_ack(isp, &ntp->nt,
- (ccb->ccb_h.flags & CAM_SEND_STATUS) ? ccb->cna2.arg : 0)) {
+
+ /*
+ * Target should abort all affected CCBs before ACK-ing INOT,
+ * but if/since it doesn't, add this hack to allow tag reuse.
+ */
+ uint32_t rsp = (ccb->ccb_h.flags & CAM_SEND_STATUS) ? ccb->cna2.arg : 0;
+ if (ntp->nt.nt_ncode == NT_ABORT_TASK && (rsp & 0xff) == 0 &&
+ (atp = isp_find_atpd(isp, XS_CHANNEL(ccb), ccb->cna2.seq_id)) != NULL) {
+ if (isp_abort_atpd(isp, XS_CHANNEL(ccb), atp) == 0)
+ isp_put_atpd(isp, XS_CHANNEL(ccb), atp);
+ }
+
+ if (isp_handle_platform_target_notify_ack(isp, &ntp->nt, rsp)) {
cam_freeze_devq(ccb->ccb_h.path);
cam_release_devq(ccb->ccb_h.path, RELSIM_RELEASE_AFTER_TIMEOUT, 0, 10, 0);
ccb->ccb_h.status &= ~CAM_STATUS_MASK;
diff --git a/sys/dev/isp/isp_freebsd.h b/sys/dev/isp/isp_freebsd.h
index bd5bba92c0a9..5bb3dd43b6de 100644
--- a/sys/dev/isp/isp_freebsd.h
+++ b/sys/dev/isp/isp_freebsd.h
@@ -89,10 +89,11 @@ void isp_put_ecmd(struct ispsoftc *, isp_ecmd_t *);
#include <dev/isp/isp_target.h>
typedef struct atio_private_data {
LIST_ENTRY(atio_private_data) next;
+ void * ccb;
+ uint32_t tag; /* typically f/w RX_ID */
uint32_t orig_datalen;
uint32_t bytes_xfered;
uint32_t bytes_in_transit;
- uint32_t tag; /* typically f/w RX_ID */
lun_id_t lun;
uint32_t nphdl;
uint32_t sid;
@@ -102,11 +103,9 @@ typedef struct atio_private_data {
uint16_t word3; /* PRLI word3 params */
uint16_t ctcnt; /* number of CTIOs currently active */
uint8_t seqno; /* CTIO sequence number */
- uint32_t
- srr_notify_rcvd : 1,
- cdb0 : 8,
+ uint8_t cdb0;
+ uint8_t srr_notify_rcvd : 1,
sendst : 1,
- dead : 1,
tattr : 3,
state : 3;
void * ests;
diff --git a/sys/dev/isp/isp_target.c b/sys/dev/isp/isp_target.c
index 081524aff53c..9d8f8e2a3766 100644
--- a/sys/dev/isp/isp_target.c
+++ b/sys/dev/isp/isp_target.c
@@ -481,10 +481,14 @@ isp_acknak_abts(ispsoftc_t *isp, void *arg, int errno)
ISP_MEMCPY(rsp, abts, QENTRY_LEN);
rsp->abts_rsp_header.rqs_entry_type = RQSTYPE_ABTS_RSP;
+ isp_prt(isp, ISP_LOGTINFO, "[0x%x] ABTS of 0x%x being %s'd",
+ rsp->abts_rsp_rxid_abts, rsp->abts_rsp_rxid_task,
+ (errno == 0) ? "BA_ACC" : "BA_RJT");
+ rsp->abts_rsp_r_ctl = (errno == 0) ? BA_ACC : BA_RJT;
+
/*
* Swap destination and source for response.
*/
- rsp->abts_rsp_r_ctl = BA_ACC;
tmpw = rsp->abts_rsp_did_lo;
tmpb = rsp->abts_rsp_did_hi;
rsp->abts_rsp_did_lo = rsp->abts_rsp_sid_lo;
@@ -505,15 +509,14 @@ isp_acknak_abts(ispsoftc_t *isp, void *arg, int errno)
rx_id = rsp->abts_rsp_rx_id;
ox_id = rsp->abts_rsp_ox_id;
ISP_MEMZERO(&rsp->abts_rsp_payload.ba_acc, sizeof (rsp->abts_rsp_payload.ba_acc));
- isp_prt(isp, ISP_LOGTINFO, "[0x%x] ABTS of 0x%x being BA_ACC'd", rsp->abts_rsp_rxid_abts, rsp->abts_rsp_rxid_task);
rsp->abts_rsp_payload.ba_acc.aborted_rx_id = rx_id;
rsp->abts_rsp_payload.ba_acc.aborted_ox_id = ox_id;
rsp->abts_rsp_payload.ba_acc.high_seq_cnt = 0xffff;
} else {
- ISP_MEMZERO(&rsp->abts_rsp_payload.ba_rjt, sizeof (rsp->abts_rsp_payload.ba_acc));
+ ISP_MEMZERO(&rsp->abts_rsp_payload.ba_rjt, sizeof (rsp->abts_rsp_payload.ba_rjt));
switch (errno) {
case ENOMEM:
- rsp->abts_rsp_payload.ba_rjt.reason = 5; /* Logical Unit Busy */
+ rsp->abts_rsp_payload.ba_rjt.reason = 5; /* Logical busy */
break;
default:
rsp->abts_rsp_payload.ba_rjt.reason = 9; /* Unable to perform command request */