* [PATCHv8 00/18] scsi: enabled reserved commands for LLDDs
@ 2021-05-03 15:03 Hannes Reinecke
2021-05-03 15:03 ` [PATCH 01/18] fnic: kill 'exclude_id' argument to fnic_cleanup_io() Hannes Reinecke
` (17 more replies)
0 siblings, 18 replies; 55+ messages in thread
From: Hannes Reinecke @ 2021-05-03 15:03 UTC (permalink / raw)
To: Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, John Garry, linux-scsi,
Hannes Reinecke
Hi all,
quite some drivers use internal commands for various purposes, most
commonly sending TMFs or querying the HBA status.
While these commands use the same submission mechanism than normal
I/O commands, they will not be counted as outstanding commands,
requiring those drivers to implement their own mechanism to figure
out outstanding commands.
The block layer already has the concept of 'reserved' tags for
precisely this purpose, namely non-I/O tags which live off a separate
tag pool. That guarantees that these commands can always be sent,
and won't be influenced by tag starvation from the I/O tag pool.
This patchset enables the use of reserved tags for the SCSI midlayer
by allocating a virtual LUN for the HBA itself which just serves
as a resource to allocate valid tags from.
This removes quite some hacks which were required for some
drivers (eg. fnic or snic), and allows the use of tagset
iterators within the drivers.
Command allocation currently ignores the hardware queues, as none
of the modified drivers is mq-capable.
The entire patchset can be found at
git://git.kernel.org/pub/scm/linux/kernel/git/hare/scsi-devel.git
reserved-tags.v8
This patchset also includes the busy_iter patches for fnic, which
were also sent as a separate patchset. So if they are applied
separately they can be dropped from this patchset.
As usual, comments and reviews are welcome.
Changes to v7:
- Drop changes to hisi_sas, pm8001, and mv_sas
- Drop patch to introduce REQ_INTERNAL flag
- Include reviews from John Garry
Changes to v6:
- Remove patch to drop gdth
- Rework libsas to use a tag per slow task
- Update hisi_sas, pm8001, and mv_sas
Changes to v5:
- Remove patch for csiostor
- Warn on normal commands in scsi_put_reserved_cmd()
- Fixup aacraid to not only scsi_put_internal_cmd() for
reserved commands
- Add 'nr_reserved_cmds' field to host template
- Reshuffle patches
Changes to v4:
- Fixup kbuild warning
- Include reviews from Bart
Changes to v3:
- Kill gdth
- Only convert fnic, snic, hpsa, and aacraid
- Drop command emulation for pseudo host device
- make 'can_queue' exclude the number or reserved tags
- Drop persistent commands proposal
- Sanitize host device handling
Changes to v2:
- Update patches from John Garry
- Use virtual LUN as suggested by Christoph
- Improve SCSI Host device to present a real SCSI device
- Implement 'persistent' commands for AENs
- Convert Megaraid SAS
Changes to v1:
- Make scsi_{get, put}_reserved_cmd() for Scsi host
- Previously we separate scsi_{get, put}_reserved_cmd() for sdev
and scsi_host_get_reserved_cmd() for the host
- Fix how Scsi_Host.can_queue is set in the virtio-scsi change
- Drop Scsi_Host.use_reserved_cmd_q
- Drop scsi_is_reserved_cmd()
- Add support in libsas and associated HBA drivers
- Allocate reserved command in slow task
- Switch hisi_sas to use reserved Scsi command
- Reorder the series a little
- Some tidying
Hannes Reinecke (18):
fnic: kill 'exclude_id' argument to fnic_cleanup_io()
fnic: use scsi_host_busy_iter() to traverse commands
scsi: add scsi_{get,put}_internal_cmd() helper
fnic: use internal commands
scsi: use real inquiry data when initialising devices
scsi: Use dummy inquiry data for the host device
scsi: revamp host device handling
snic: use reserved commands
snic: use tagset iter for traversing commands
scsi: implement reserved command handling
hpsa: move hpsa_hba_inquiry after scsi_add_host()
hpsa: use reserved commands
hpsa: use scsi_host_busy_iter() to traverse outstanding commands
hpsa: drop refcount field from CommandList
aacraid: move scsi_add_host()
aacraid: store target id in host_scribble
aacraid: use scsi_get_internal_cmd()
aacraid: use scsi_host_busy_iter() to traverse outstanding commands
drivers/scsi/aacraid/aachba.c | 137 ++---
drivers/scsi/aacraid/aacraid.h | 10 +-
drivers/scsi/aacraid/commctrl.c | 25 +-
drivers/scsi/aacraid/comminit.c | 2 +-
drivers/scsi/aacraid/commsup.c | 118 ++--
drivers/scsi/aacraid/dpcsup.c | 2 +-
drivers/scsi/aacraid/linit.c | 175 +++---
drivers/scsi/fnic/fnic_scsi.c | 927 +++++++++++++++-----------------
drivers/scsi/hosts.c | 3 +
drivers/scsi/hpsa.c | 365 ++++++-------
drivers/scsi/hpsa.h | 3 +-
drivers/scsi/hpsa_cmd.h | 10 -
drivers/scsi/scsi_devinfo.c | 1 +
drivers/scsi/scsi_lib.c | 48 +-
drivers/scsi/scsi_scan.c | 96 ++--
drivers/scsi/scsi_sysfs.c | 5 +-
drivers/scsi/snic/snic.h | 4 +-
drivers/scsi/snic/snic_main.c | 7 +
drivers/scsi/snic/snic_scsi.c | 524 +++++++++---------
include/scsi/scsi_device.h | 3 +
include/scsi/scsi_host.h | 36 +-
21 files changed, 1234 insertions(+), 1267 deletions(-)
--
2.29.2
^ permalink raw reply [flat|nested] 55+ messages in thread
* [PATCH 01/18] fnic: kill 'exclude_id' argument to fnic_cleanup_io()
2021-05-03 15:03 [PATCHv8 00/18] scsi: enabled reserved commands for LLDDs Hannes Reinecke
@ 2021-05-03 15:03 ` Hannes Reinecke
2021-05-04 2:12 ` Bart Van Assche
2021-05-04 9:49 ` Christoph Hellwig
2021-05-03 15:03 ` [PATCH 02/18] fnic: use scsi_host_busy_iter() to traverse commands Hannes Reinecke
` (16 subsequent siblings)
17 siblings, 2 replies; 55+ messages in thread
From: Hannes Reinecke @ 2021-05-03 15:03 UTC (permalink / raw)
To: Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, John Garry, linux-scsi,
Hannes Reinecke
'exclude_id' is always SCSI_NO_TAG, which will never be reached
when traversing the list of tags.
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
drivers/scsi/fnic/fnic_scsi.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/drivers/scsi/fnic/fnic_scsi.c b/drivers/scsi/fnic/fnic_scsi.c
index e619a82f921b..f41d1b1c2e39 100644
--- a/drivers/scsi/fnic/fnic_scsi.c
+++ b/drivers/scsi/fnic/fnic_scsi.c
@@ -102,7 +102,7 @@ static const char *fnic_fcpio_status_to_str(unsigned int status)
return fcpio_status_str[status];
}
-static void fnic_cleanup_io(struct fnic *fnic, int exclude_id);
+static void fnic_cleanup_io(struct fnic *fnic);
static inline spinlock_t *fnic_io_lock_hash(struct fnic *fnic,
struct scsi_cmnd *sc)
@@ -638,7 +638,7 @@ static int fnic_fcpio_fw_reset_cmpl_handler(struct fnic *fnic,
atomic64_inc(&reset_stats->fw_reset_completions);
/* Clean up all outstanding io requests */
- fnic_cleanup_io(fnic, SCSI_NO_TAG);
+ fnic_cleanup_io(fnic);
atomic64_set(&fnic->fnic_stats.fw_stats.active_fw_reqs, 0);
atomic64_set(&fnic->fnic_stats.io_stats.active_ios, 0);
@@ -1361,7 +1361,7 @@ int fnic_wq_copy_cmpl_handler(struct fnic *fnic, int copy_work_to_do)
return wq_work_done;
}
-static void fnic_cleanup_io(struct fnic *fnic, int exclude_id)
+static void fnic_cleanup_io(struct fnic *fnic)
{
int i;
struct fnic_io_req *io_req;
@@ -1372,9 +1372,6 @@ static void fnic_cleanup_io(struct fnic *fnic, int exclude_id)
struct fnic_stats *fnic_stats = &fnic->fnic_stats;
for (i = 0; i < fnic->fnic_max_tag_id; i++) {
- if (i == exclude_id)
- continue;
-
io_lock = fnic_io_lock_tag(fnic, i);
spin_lock_irqsave(io_lock, flags);
sc = scsi_host_find_tag(fnic->lport->host, i);
--
2.29.2
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [PATCH 02/18] fnic: use scsi_host_busy_iter() to traverse commands
2021-05-03 15:03 [PATCHv8 00/18] scsi: enabled reserved commands for LLDDs Hannes Reinecke
2021-05-03 15:03 ` [PATCH 01/18] fnic: kill 'exclude_id' argument to fnic_cleanup_io() Hannes Reinecke
@ 2021-05-03 15:03 ` Hannes Reinecke
2021-05-04 9:50 ` Christoph Hellwig
2021-05-03 15:03 ` [PATCH 03/18] scsi: add scsi_{get,put}_internal_cmd() helper Hannes Reinecke
` (15 subsequent siblings)
17 siblings, 1 reply; 55+ messages in thread
From: Hannes Reinecke @ 2021-05-03 15:03 UTC (permalink / raw)
To: Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, John Garry, linux-scsi,
Hannes Reinecke
Use scsi_host_busy_iter() to traverse commands instead of
hand-crafted routines walking the command list.
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
drivers/scsi/fnic/fnic_scsi.c | 821 ++++++++++++++++------------------
1 file changed, 375 insertions(+), 446 deletions(-)
diff --git a/drivers/scsi/fnic/fnic_scsi.c b/drivers/scsi/fnic/fnic_scsi.c
index f41d1b1c2e39..762cc8bd2653 100644
--- a/drivers/scsi/fnic/fnic_scsi.c
+++ b/drivers/scsi/fnic/fnic_scsi.c
@@ -1361,90 +1361,90 @@ int fnic_wq_copy_cmpl_handler(struct fnic *fnic, int copy_work_to_do)
return wq_work_done;
}
-static void fnic_cleanup_io(struct fnic *fnic)
+static bool fnic_cleanup_io_iter(struct scsi_cmnd *sc, void *data,
+ bool reserved)
{
- int i;
+ struct fnic *fnic = data;
struct fnic_io_req *io_req;
unsigned long flags = 0;
- struct scsi_cmnd *sc;
spinlock_t *io_lock;
unsigned long start_time = 0;
struct fnic_stats *fnic_stats = &fnic->fnic_stats;
- for (i = 0; i < fnic->fnic_max_tag_id; i++) {
- io_lock = fnic_io_lock_tag(fnic, i);
- spin_lock_irqsave(io_lock, flags);
- sc = scsi_host_find_tag(fnic->lport->host, i);
- if (!sc) {
- spin_unlock_irqrestore(io_lock, flags);
- continue;
- }
-
- io_req = (struct fnic_io_req *)CMD_SP(sc);
- if ((CMD_FLAGS(sc) & FNIC_DEVICE_RESET) &&
- !(CMD_FLAGS(sc) & FNIC_DEV_RST_DONE)) {
- /*
- * We will be here only when FW completes reset
- * without sending completions for outstanding ios.
- */
- CMD_FLAGS(sc) |= FNIC_DEV_RST_DONE;
- if (io_req && io_req->dr_done)
- complete(io_req->dr_done);
- else if (io_req && io_req->abts_done)
- complete(io_req->abts_done);
- spin_unlock_irqrestore(io_lock, flags);
- continue;
- } else if (CMD_FLAGS(sc) & FNIC_DEVICE_RESET) {
- spin_unlock_irqrestore(io_lock, flags);
- continue;
- }
- if (!io_req) {
- spin_unlock_irqrestore(io_lock, flags);
- continue;
- }
-
- CMD_SP(sc) = NULL;
-
- spin_unlock_irqrestore(io_lock, flags);
+ io_lock = fnic_io_lock_tag(fnic, sc->request->tag);
+ spin_lock_irqsave(io_lock, flags);
+ io_req = (struct fnic_io_req *)CMD_SP(sc);
+ if ((CMD_FLAGS(sc) & FNIC_DEVICE_RESET) &&
+ !(CMD_FLAGS(sc) & FNIC_DEV_RST_DONE)) {
/*
- * If there is a scsi_cmnd associated with this io_req, then
- * free the corresponding state
+ * We will be here only when FW completes reset
+ * without sending completions for outstanding ios.
*/
- start_time = io_req->start_time;
- fnic_release_ioreq_buf(fnic, io_req, sc);
- mempool_free(io_req, fnic->io_req_pool);
+ CMD_FLAGS(sc) |= FNIC_DEV_RST_DONE;
+ if (io_req && io_req->dr_done)
+ complete(io_req->dr_done);
+ else if (io_req && io_req->abts_done)
+ complete(io_req->abts_done);
+ spin_unlock_irqrestore(io_lock, flags);
+ return true;
+ } else if (CMD_FLAGS(sc) & FNIC_DEVICE_RESET) {
+ spin_unlock_irqrestore(io_lock, flags);
+ return true;
+ }
+ if (!io_req) {
+ spin_unlock_irqrestore(io_lock, flags);
+ goto cleanup_scsi_cmd;
+ }
- sc->result = DID_TRANSPORT_DISRUPTED << 16;
- FNIC_SCSI_DBG(KERN_DEBUG, fnic->lport->host,
- "%s: tag:0x%x : sc:0x%p duration = %lu DID_TRANSPORT_DISRUPTED\n",
- __func__, sc->request->tag, sc,
- (jiffies - start_time));
+ CMD_SP(sc) = NULL;
- if (atomic64_read(&fnic->io_cmpl_skip))
- atomic64_dec(&fnic->io_cmpl_skip);
- else
- atomic64_inc(&fnic_stats->io_stats.io_completions);
+ spin_unlock_irqrestore(io_lock, flags);
- /* Complete the command to SCSI */
- if (sc->scsi_done) {
- if (!(CMD_FLAGS(sc) & FNIC_IO_ISSUED))
- shost_printk(KERN_ERR, fnic->lport->host,
- "Calling done for IO not issued to fw: tag:0x%x sc:0x%p\n",
- sc->request->tag, sc);
+ /*
+ * If there is a scsi_cmnd associated with this io_req, then
+ * free the corresponding state
+ */
+ start_time = io_req->start_time;
+ fnic_release_ioreq_buf(fnic, io_req, sc);
+ mempool_free(io_req, fnic->io_req_pool);
- FNIC_TRACE(fnic_cleanup_io,
- sc->device->host->host_no, i, sc,
- jiffies_to_msecs(jiffies - start_time),
- 0, ((u64)sc->cmnd[0] << 32 |
- (u64)sc->cmnd[2] << 24 |
- (u64)sc->cmnd[3] << 16 |
- (u64)sc->cmnd[4] << 8 | sc->cmnd[5]),
- (((u64)CMD_FLAGS(sc) << 32) | CMD_STATE(sc)));
+cleanup_scsi_cmd:
+ sc->result = DID_TRANSPORT_DISRUPTED << 16;
+ FNIC_SCSI_DBG(KERN_DEBUG, fnic->lport->host,
+ "fnic_cleanup_io: tag:0x%x : sc:0x%p duration = %lu DID_TRANSPORT_DISRUPTED\n",
+ sc->request->tag, sc, (jiffies - start_time));
- sc->scsi_done(sc);
- }
+ if (atomic64_read(&fnic->io_cmpl_skip))
+ atomic64_dec(&fnic->io_cmpl_skip);
+ else
+ atomic64_inc(&fnic_stats->io_stats.io_completions);
+
+ /* Complete the command to SCSI */
+ if (sc->scsi_done) {
+ if (!(CMD_FLAGS(sc) & FNIC_IO_ISSUED))
+ shost_printk(KERN_ERR, fnic->lport->host,
+ "Calling done for IO not issued to fw: tag:0x%x sc:0x%p\n",
+ sc->request->tag, sc);
+
+ FNIC_TRACE(fnic_cleanup_io,
+ sc->device->host->host_no, sc->request->tag, sc,
+ jiffies_to_msecs(jiffies - start_time),
+ 0, ((u64)sc->cmnd[0] << 32 |
+ (u64)sc->cmnd[2] << 24 |
+ (u64)sc->cmnd[3] << 16 |
+ (u64)sc->cmnd[4] << 8 | sc->cmnd[5]),
+ (((u64)CMD_FLAGS(sc) << 32) | CMD_STATE(sc)));
+
+ sc->scsi_done(sc);
}
+ return true;
+}
+
+static void fnic_cleanup_io(struct fnic *fnic)
+{
+ scsi_host_busy_iter(fnic->lport->host,
+ fnic_cleanup_io_iter, fnic);
}
void fnic_wq_copy_cleanup_handler(struct vnic_wq_copy *wq,
@@ -1555,143 +1555,141 @@ static inline int fnic_queue_abort_io_req(struct fnic *fnic, int tag,
return 0;
}
-static void fnic_rport_exch_reset(struct fnic *fnic, u32 port_id)
+struct fnic_rport_abort_io_iter_data {
+ struct fnic *fnic;
+ u32 port_id;
+ int term_cnt;
+};
+
+static bool fnic_rport_abort_io_iter(struct scsi_cmnd *sc, void *data,
+ bool reserved)
{
- int tag;
- int abt_tag;
- int term_cnt = 0;
+ struct fnic_rport_abort_io_iter_data *iter_data = data;
+ struct fnic *fnic = iter_data->fnic;
+ int abt_tag = sc->request->tag;
struct fnic_io_req *io_req;
spinlock_t *io_lock;
unsigned long flags;
- struct scsi_cmnd *sc;
struct reset_stats *reset_stats = &fnic->fnic_stats.reset_stats;
struct terminate_stats *term_stats = &fnic->fnic_stats.term_stats;
struct scsi_lun fc_lun;
enum fnic_ioreq_state old_ioreq_state;
- FNIC_SCSI_DBG(KERN_DEBUG,
- fnic->lport->host,
- "fnic_rport_exch_reset called portid 0x%06x\n",
- port_id);
-
- if (fnic->in_remove)
- return;
-
- for (tag = 0; tag < fnic->fnic_max_tag_id; tag++) {
- abt_tag = tag;
- io_lock = fnic_io_lock_tag(fnic, tag);
- spin_lock_irqsave(io_lock, flags);
- sc = scsi_host_find_tag(fnic->lport->host, tag);
- if (!sc) {
- spin_unlock_irqrestore(io_lock, flags);
- continue;
- }
+ io_lock = fnic_io_lock_tag(fnic, abt_tag);
+ spin_lock_irqsave(io_lock, flags);
- io_req = (struct fnic_io_req *)CMD_SP(sc);
+ io_req = (struct fnic_io_req *)CMD_SP(sc);
- if (!io_req || io_req->port_id != port_id) {
- spin_unlock_irqrestore(io_lock, flags);
- continue;
- }
+ if (!io_req || io_req->port_id != iter_data->port_id) {
+ spin_unlock_irqrestore(io_lock, flags);
+ return true;
+ }
- if ((CMD_FLAGS(sc) & FNIC_DEVICE_RESET) &&
- (!(CMD_FLAGS(sc) & FNIC_DEV_RST_ISSUED))) {
- FNIC_SCSI_DBG(KERN_DEBUG, fnic->lport->host,
+ if ((CMD_FLAGS(sc) & FNIC_DEVICE_RESET) &&
+ (!(CMD_FLAGS(sc) & FNIC_DEV_RST_ISSUED))) {
+ FNIC_SCSI_DBG(KERN_DEBUG, fnic->lport->host,
"fnic_rport_exch_reset dev rst not pending sc 0x%p\n",
sc);
- spin_unlock_irqrestore(io_lock, flags);
- continue;
- }
+ spin_unlock_irqrestore(io_lock, flags);
+ return true;
+ }
- /*
- * Found IO that is still pending with firmware and
- * belongs to rport that went away
- */
- if (CMD_STATE(sc) == FNIC_IOREQ_ABTS_PENDING) {
- spin_unlock_irqrestore(io_lock, flags);
- continue;
- }
- if (io_req->abts_done) {
- shost_printk(KERN_ERR, fnic->lport->host,
+ /*
+ * Found IO that is still pending with firmware and
+ * belongs to rport that went away
+ */
+ if (CMD_STATE(sc) == FNIC_IOREQ_ABTS_PENDING) {
+ spin_unlock_irqrestore(io_lock, flags);
+ return true;
+ }
+ if (io_req->abts_done) {
+ shost_printk(KERN_ERR, fnic->lport->host,
"fnic_rport_exch_reset: io_req->abts_done is set "
"state is %s\n",
fnic_ioreq_state_to_str(CMD_STATE(sc)));
- }
+ }
- if (!(CMD_FLAGS(sc) & FNIC_IO_ISSUED)) {
- shost_printk(KERN_ERR, fnic->lport->host,
- "rport_exch_reset "
- "IO not yet issued %p tag 0x%x flags "
- "%x state %d\n",
- sc, tag, CMD_FLAGS(sc), CMD_STATE(sc));
- }
- old_ioreq_state = CMD_STATE(sc);
- CMD_STATE(sc) = FNIC_IOREQ_ABTS_PENDING;
- CMD_ABTS_STATUS(sc) = FCPIO_INVALID_CODE;
- if (CMD_FLAGS(sc) & FNIC_DEVICE_RESET) {
- atomic64_inc(&reset_stats->device_reset_terminates);
- abt_tag = (tag | FNIC_TAG_DEV_RST);
- FNIC_SCSI_DBG(KERN_DEBUG, fnic->lport->host,
- "fnic_rport_exch_reset dev rst sc 0x%p\n",
- sc);
- }
+ if (!(CMD_FLAGS(sc) & FNIC_IO_ISSUED)) {
+ shost_printk(KERN_ERR, fnic->lport->host,
+ "rport_exch_reset "
+ "IO not yet issued %p tag 0x%x flags "
+ "%x state %d\n",
+ sc, abt_tag, CMD_FLAGS(sc), CMD_STATE(sc));
+ }
+ old_ioreq_state = CMD_STATE(sc);
+ CMD_STATE(sc) = FNIC_IOREQ_ABTS_PENDING;
+ CMD_ABTS_STATUS(sc) = FCPIO_INVALID_CODE;
+ if (CMD_FLAGS(sc) & FNIC_DEVICE_RESET) {
+ atomic64_inc(&reset_stats->device_reset_terminates);
+ abt_tag |= FNIC_TAG_DEV_RST;
+ }
+ FNIC_SCSI_DBG(KERN_DEBUG, fnic->lport->host,
+ "fnic_rport_exch_reset dev rst sc 0x%p\n", sc);
+ BUG_ON(io_req->abts_done);
- BUG_ON(io_req->abts_done);
+ FNIC_SCSI_DBG(KERN_DEBUG, fnic->lport->host,
+ "fnic_rport_reset_exch: Issuing abts\n");
- FNIC_SCSI_DBG(KERN_DEBUG, fnic->lport->host,
- "fnic_rport_reset_exch: Issuing abts\n");
+ spin_unlock_irqrestore(io_lock, flags);
+ /* Now queue the abort command to firmware */
+ int_to_scsilun(sc->device->lun, &fc_lun);
+
+ if (fnic_queue_abort_io_req(fnic, abt_tag,
+ FCPIO_ITMF_ABT_TASK_TERM,
+ fc_lun.scsi_lun, io_req)) {
+ /*
+ * Revert the cmd state back to old state, if
+ * it hasn't changed in between. This cmd will get
+ * aborted later by scsi_eh, or cleaned up during
+ * lun reset
+ */
+ spin_lock_irqsave(io_lock, flags);
+ if (CMD_STATE(sc) == FNIC_IOREQ_ABTS_PENDING)
+ CMD_STATE(sc) = old_ioreq_state;
+ spin_unlock_irqrestore(io_lock, flags);
+ } else {
+ spin_lock_irqsave(io_lock, flags);
+ if (CMD_FLAGS(sc) & FNIC_DEVICE_RESET)
+ CMD_FLAGS(sc) |= FNIC_DEV_RST_TERM_ISSUED;
+ else
+ CMD_FLAGS(sc) |= FNIC_IO_INTERNAL_TERM_ISSUED;
spin_unlock_irqrestore(io_lock, flags);
+ atomic64_inc(&term_stats->terminates);
+ iter_data->term_cnt++;
+ }
+ return true;
+}
- /* Now queue the abort command to firmware */
- int_to_scsilun(sc->device->lun, &fc_lun);
+static void fnic_rport_exch_reset(struct fnic *fnic, u32 port_id)
+{
+ struct terminate_stats *term_stats = &fnic->fnic_stats.term_stats;
+ struct fnic_rport_abort_io_iter_data iter_data = {
+ .fnic = fnic,
+ .port_id = port_id,
+ .term_cnt = 0,
+ };
- if (fnic_queue_abort_io_req(fnic, abt_tag,
- FCPIO_ITMF_ABT_TASK_TERM,
- fc_lun.scsi_lun, io_req)) {
- /*
- * Revert the cmd state back to old state, if
- * it hasn't changed in between. This cmd will get
- * aborted later by scsi_eh, or cleaned up during
- * lun reset
- */
- spin_lock_irqsave(io_lock, flags);
- if (CMD_STATE(sc) == FNIC_IOREQ_ABTS_PENDING)
- CMD_STATE(sc) = old_ioreq_state;
- spin_unlock_irqrestore(io_lock, flags);
- } else {
- spin_lock_irqsave(io_lock, flags);
- if (CMD_FLAGS(sc) & FNIC_DEVICE_RESET)
- CMD_FLAGS(sc) |= FNIC_DEV_RST_TERM_ISSUED;
- else
- CMD_FLAGS(sc) |= FNIC_IO_INTERNAL_TERM_ISSUED;
- spin_unlock_irqrestore(io_lock, flags);
- atomic64_inc(&term_stats->terminates);
- term_cnt++;
- }
- }
- if (term_cnt > atomic64_read(&term_stats->max_terminates))
- atomic64_set(&term_stats->max_terminates, term_cnt);
+ FNIC_SCSI_DBG(KERN_DEBUG,
+ fnic->lport->host,
+ "fnic_rport_exch_reset called portid 0x%06x\n",
+ port_id);
+
+ if (fnic->in_remove)
+ return;
+
+ scsi_host_busy_iter(fnic->lport->host, fnic_rport_abort_io_iter,
+ &iter_data);
+ if (iter_data.term_cnt > atomic64_read(&term_stats->max_terminates))
+ atomic64_set(&term_stats->max_terminates, iter_data.term_cnt);
}
void fnic_terminate_rport_io(struct fc_rport *rport)
{
- int tag;
- int abt_tag;
- int term_cnt = 0;
- struct fnic_io_req *io_req;
- spinlock_t *io_lock;
- unsigned long flags;
- struct scsi_cmnd *sc;
- struct scsi_lun fc_lun;
struct fc_rport_libfc_priv *rdata;
struct fc_lport *lport;
struct fnic *fnic;
- struct fc_rport *cmd_rport;
- struct reset_stats *reset_stats;
- struct terminate_stats *term_stats;
- enum fnic_ioreq_state old_ioreq_state;
if (!rport) {
printk(KERN_ERR "fnic_terminate_rport_io: rport is NULL\n");
@@ -1719,108 +1717,7 @@ void fnic_terminate_rport_io(struct fc_rport *rport)
if (fnic->in_remove)
return;
- reset_stats = &fnic->fnic_stats.reset_stats;
- term_stats = &fnic->fnic_stats.term_stats;
-
- for (tag = 0; tag < fnic->fnic_max_tag_id; tag++) {
- abt_tag = tag;
- io_lock = fnic_io_lock_tag(fnic, tag);
- spin_lock_irqsave(io_lock, flags);
- sc = scsi_host_find_tag(fnic->lport->host, tag);
- if (!sc) {
- spin_unlock_irqrestore(io_lock, flags);
- continue;
- }
-
- io_req = (struct fnic_io_req *)CMD_SP(sc);
- if (!io_req) {
- spin_unlock_irqrestore(io_lock, flags);
- continue;
- }
-
- cmd_rport = starget_to_rport(scsi_target(sc->device));
- if (rport != cmd_rport) {
- spin_unlock_irqrestore(io_lock, flags);
- continue;
- }
-
- if ((CMD_FLAGS(sc) & FNIC_DEVICE_RESET) &&
- (!(CMD_FLAGS(sc) & FNIC_DEV_RST_ISSUED))) {
- FNIC_SCSI_DBG(KERN_DEBUG, fnic->lport->host,
- "fnic_terminate_rport_io dev rst not pending sc 0x%p\n",
- sc);
- spin_unlock_irqrestore(io_lock, flags);
- continue;
- }
- /*
- * Found IO that is still pending with firmware and
- * belongs to rport that went away
- */
- if (CMD_STATE(sc) == FNIC_IOREQ_ABTS_PENDING) {
- spin_unlock_irqrestore(io_lock, flags);
- continue;
- }
- if (io_req->abts_done) {
- shost_printk(KERN_ERR, fnic->lport->host,
- "fnic_terminate_rport_io: io_req->abts_done is set "
- "state is %s\n",
- fnic_ioreq_state_to_str(CMD_STATE(sc)));
- }
- if (!(CMD_FLAGS(sc) & FNIC_IO_ISSUED)) {
- FNIC_SCSI_DBG(KERN_INFO, fnic->lport->host,
- "fnic_terminate_rport_io "
- "IO not yet issued %p tag 0x%x flags "
- "%x state %d\n",
- sc, tag, CMD_FLAGS(sc), CMD_STATE(sc));
- }
- old_ioreq_state = CMD_STATE(sc);
- CMD_STATE(sc) = FNIC_IOREQ_ABTS_PENDING;
- CMD_ABTS_STATUS(sc) = FCPIO_INVALID_CODE;
- if (CMD_FLAGS(sc) & FNIC_DEVICE_RESET) {
- atomic64_inc(&reset_stats->device_reset_terminates);
- abt_tag = (tag | FNIC_TAG_DEV_RST);
- FNIC_SCSI_DBG(KERN_DEBUG, fnic->lport->host,
- "fnic_terminate_rport_io dev rst sc 0x%p\n", sc);
- }
-
- BUG_ON(io_req->abts_done);
-
- FNIC_SCSI_DBG(KERN_DEBUG,
- fnic->lport->host,
- "fnic_terminate_rport_io: Issuing abts\n");
-
- spin_unlock_irqrestore(io_lock, flags);
-
- /* Now queue the abort command to firmware */
- int_to_scsilun(sc->device->lun, &fc_lun);
-
- if (fnic_queue_abort_io_req(fnic, abt_tag,
- FCPIO_ITMF_ABT_TASK_TERM,
- fc_lun.scsi_lun, io_req)) {
- /*
- * Revert the cmd state back to old state, if
- * it hasn't changed in between. This cmd will get
- * aborted later by scsi_eh, or cleaned up during
- * lun reset
- */
- spin_lock_irqsave(io_lock, flags);
- if (CMD_STATE(sc) == FNIC_IOREQ_ABTS_PENDING)
- CMD_STATE(sc) = old_ioreq_state;
- spin_unlock_irqrestore(io_lock, flags);
- } else {
- spin_lock_irqsave(io_lock, flags);
- if (CMD_FLAGS(sc) & FNIC_DEVICE_RESET)
- CMD_FLAGS(sc) |= FNIC_DEV_RST_TERM_ISSUED;
- else
- CMD_FLAGS(sc) |= FNIC_IO_INTERNAL_TERM_ISSUED;
- spin_unlock_irqrestore(io_lock, flags);
- atomic64_inc(&term_stats->terminates);
- term_cnt++;
- }
- }
- if (term_cnt > atomic64_read(&term_stats->max_terminates))
- atomic64_set(&term_stats->max_terminates, term_cnt);
-
+ fnic_rport_exch_reset(fnic, rport->port_id);
}
/*
@@ -2115,165 +2012,183 @@ static inline int fnic_queue_dr_io_req(struct fnic *fnic,
return ret;
}
-/*
- * Clean up any pending aborts on the lun
- * For each outstanding IO on this lun, whose abort is not completed by fw,
- * issue a local abort. Wait for abort to complete. Return 0 if all commands
- * successfully aborted, 1 otherwise
- */
-static int fnic_clean_pending_aborts(struct fnic *fnic,
- struct scsi_cmnd *lr_sc,
- bool new_sc)
+struct fnic_pending_aborts_iter_data {
+ struct fnic *fnic;
+ struct scsi_cmnd *lr_sc;
+ struct scsi_device *lun_dev;
+ int ret;
+};
+static bool fnic_pending_aborts_iter(struct scsi_cmnd *sc,
+ void *data, bool reserved)
{
- int tag, abt_tag;
+ struct fnic_pending_aborts_iter_data *iter_data = data;
+ struct fnic *fnic = iter_data->fnic;
+ struct scsi_device *lun_dev = iter_data->lun_dev;
+ int abt_tag = sc->request->tag;
struct fnic_io_req *io_req;
spinlock_t *io_lock;
unsigned long flags;
- int ret = 0;
- struct scsi_cmnd *sc;
struct scsi_lun fc_lun;
- struct scsi_device *lun_dev = lr_sc->device;
DECLARE_COMPLETION_ONSTACK(tm_done);
enum fnic_ioreq_state old_ioreq_state;
- for (tag = 0; tag < fnic->fnic_max_tag_id; tag++) {
- io_lock = fnic_io_lock_tag(fnic, tag);
- spin_lock_irqsave(io_lock, flags);
- sc = scsi_host_find_tag(fnic->lport->host, tag);
- /*
- * ignore this lun reset cmd if issued using new SC
- * or cmds that do not belong to this lun
- */
- if (!sc || ((sc == lr_sc) && new_sc) || sc->device != lun_dev) {
- spin_unlock_irqrestore(io_lock, flags);
- continue;
- }
+ if (sc == iter_data->lr_sc || sc->device != lun_dev)
+ return true;
+ if (reserved)
+ return true;
- io_req = (struct fnic_io_req *)CMD_SP(sc);
-
- if (!io_req || sc->device != lun_dev) {
- spin_unlock_irqrestore(io_lock, flags);
- continue;
- }
-
- /*
- * Found IO that is still pending with firmware and
- * belongs to the LUN that we are resetting
- */
- FNIC_SCSI_DBG(KERN_DEBUG, fnic->lport->host,
- "Found IO in %s on lun\n",
- fnic_ioreq_state_to_str(CMD_STATE(sc)));
-
- if (CMD_STATE(sc) == FNIC_IOREQ_ABTS_PENDING) {
- spin_unlock_irqrestore(io_lock, flags);
- continue;
- }
- if ((CMD_FLAGS(sc) & FNIC_DEVICE_RESET) &&
- (!(CMD_FLAGS(sc) & FNIC_DEV_RST_ISSUED))) {
- FNIC_SCSI_DBG(KERN_INFO, fnic->lport->host,
- "%s dev rst not pending sc 0x%p\n", __func__,
- sc);
- spin_unlock_irqrestore(io_lock, flags);
- continue;
- }
+ io_lock = fnic_io_lock_tag(fnic, abt_tag);
+ spin_lock_irqsave(io_lock, flags);
+ io_req = (struct fnic_io_req *)CMD_SP(sc);
+ if (!io_req) {
+ spin_unlock_irqrestore(io_lock, flags);
+ return true;
+ }
- if (io_req->abts_done)
- shost_printk(KERN_ERR, fnic->lport->host,
- "%s: io_req->abts_done is set state is %s\n",
- __func__, fnic_ioreq_state_to_str(CMD_STATE(sc)));
- old_ioreq_state = CMD_STATE(sc);
- /*
- * Any pending IO issued prior to reset is expected to be
- * in abts pending state, if not we need to set
- * FNIC_IOREQ_ABTS_PENDING to indicate the IO is abort pending.
- * When IO is completed, the IO will be handed over and
- * handled in this function.
- */
- CMD_STATE(sc) = FNIC_IOREQ_ABTS_PENDING;
+ /*
+ * Found IO that is still pending with firmware and
+ * belongs to the LUN that we are resetting
+ */
+ FNIC_SCSI_DBG(KERN_DEBUG, fnic->lport->host,
+ "Found IO in %s on lun\n",
+ fnic_ioreq_state_to_str(CMD_STATE(sc)));
- BUG_ON(io_req->abts_done);
+ if (CMD_STATE(sc) == FNIC_IOREQ_ABTS_PENDING) {
+ spin_unlock_irqrestore(io_lock, flags);
+ return true;
+ }
+ if ((CMD_FLAGS(sc) & FNIC_DEVICE_RESET) &&
+ (!(CMD_FLAGS(sc) & FNIC_DEV_RST_ISSUED))) {
+ FNIC_SCSI_DBG(KERN_INFO, fnic->lport->host,
+ "%s dev rst not pending sc 0x%p\n", __func__,
+ sc);
+ spin_unlock_irqrestore(io_lock, flags);
+ return true;
+ }
- abt_tag = tag;
- if (CMD_FLAGS(sc) & FNIC_DEVICE_RESET) {
- abt_tag |= FNIC_TAG_DEV_RST;
- FNIC_SCSI_DBG(KERN_INFO, fnic->lport->host,
- "%s: dev rst sc 0x%p\n", __func__, sc);
- }
+ if (io_req->abts_done)
+ shost_printk(KERN_ERR, fnic->lport->host,
+ "%s: io_req->abts_done is set state is %s\n",
+ __func__, fnic_ioreq_state_to_str(CMD_STATE(sc)));
+ old_ioreq_state = CMD_STATE(sc);
+ /*
+ * Any pending IO issued prior to reset is expected to be
+ * in abts pending state, if not we need to set
+ * FNIC_IOREQ_ABTS_PENDING to indicate the IO is abort pending.
+ * When IO is completed, the IO will be handed over and
+ * handled in this function.
+ */
+ CMD_STATE(sc) = FNIC_IOREQ_ABTS_PENDING;
- CMD_ABTS_STATUS(sc) = FCPIO_INVALID_CODE;
- io_req->abts_done = &tm_done;
- spin_unlock_irqrestore(io_lock, flags);
+ BUG_ON(io_req->abts_done);
- /* Now queue the abort command to firmware */
- int_to_scsilun(sc->device->lun, &fc_lun);
+ if (CMD_FLAGS(sc) & FNIC_DEVICE_RESET) {
+ abt_tag |= FNIC_TAG_DEV_RST;
+ FNIC_SCSI_DBG(KERN_INFO, fnic->lport->host,
+ "%s: dev rst sc 0x%p\n", __func__, sc);
+ }
- if (fnic_queue_abort_io_req(fnic, abt_tag,
- FCPIO_ITMF_ABT_TASK_TERM,
- fc_lun.scsi_lun, io_req)) {
- spin_lock_irqsave(io_lock, flags);
- io_req = (struct fnic_io_req *)CMD_SP(sc);
- if (io_req)
- io_req->abts_done = NULL;
- if (CMD_STATE(sc) == FNIC_IOREQ_ABTS_PENDING)
- CMD_STATE(sc) = old_ioreq_state;
- spin_unlock_irqrestore(io_lock, flags);
- ret = 1;
- goto clean_pending_aborts_end;
- } else {
- spin_lock_irqsave(io_lock, flags);
- if (CMD_FLAGS(sc) & FNIC_DEVICE_RESET)
- CMD_FLAGS(sc) |= FNIC_DEV_RST_TERM_ISSUED;
- spin_unlock_irqrestore(io_lock, flags);
- }
- CMD_FLAGS(sc) |= FNIC_IO_INTERNAL_TERM_ISSUED;
+ CMD_ABTS_STATUS(sc) = FCPIO_INVALID_CODE;
+ io_req->abts_done = &tm_done;
+ spin_unlock_irqrestore(io_lock, flags);
- wait_for_completion_timeout(&tm_done,
- msecs_to_jiffies
- (fnic->config.ed_tov));
+ /* Now queue the abort command to firmware */
+ int_to_scsilun(sc->device->lun, &fc_lun);
- /* Recheck cmd state to check if it is now aborted */
+ if (fnic_queue_abort_io_req(fnic, abt_tag,
+ FCPIO_ITMF_ABT_TASK_TERM,
+ fc_lun.scsi_lun, io_req)) {
spin_lock_irqsave(io_lock, flags);
io_req = (struct fnic_io_req *)CMD_SP(sc);
- if (!io_req) {
- spin_unlock_irqrestore(io_lock, flags);
- CMD_FLAGS(sc) |= FNIC_IO_ABT_TERM_REQ_NULL;
- continue;
- }
+ if (io_req)
+ io_req->abts_done = NULL;
+ if (CMD_STATE(sc) == FNIC_IOREQ_ABTS_PENDING)
+ CMD_STATE(sc) = old_ioreq_state;
+ spin_unlock_irqrestore(io_lock, flags);
+ iter_data->ret = FAILED;
+ return false;
+ } else {
+ spin_lock_irqsave(io_lock, flags);
+ if (CMD_FLAGS(sc) & FNIC_DEVICE_RESET)
+ CMD_FLAGS(sc) |= FNIC_DEV_RST_TERM_ISSUED;
+ spin_unlock_irqrestore(io_lock, flags);
+ }
+ CMD_FLAGS(sc) |= FNIC_IO_INTERNAL_TERM_ISSUED;
- io_req->abts_done = NULL;
+ wait_for_completion_timeout(&tm_done, msecs_to_jiffies
+ (fnic->config.ed_tov));
- /* if abort is still pending with fw, fail */
- if (CMD_ABTS_STATUS(sc) == FCPIO_INVALID_CODE) {
- spin_unlock_irqrestore(io_lock, flags);
- CMD_FLAGS(sc) |= FNIC_IO_ABT_TERM_DONE;
- ret = 1;
- goto clean_pending_aborts_end;
- }
- CMD_STATE(sc) = FNIC_IOREQ_ABTS_COMPLETE;
+ /* Recheck cmd state to check if it is now aborted */
+ spin_lock_irqsave(io_lock, flags);
+ io_req = (struct fnic_io_req *)CMD_SP(sc);
+ if (!io_req) {
+ spin_unlock_irqrestore(io_lock, flags);
+ CMD_FLAGS(sc) |= FNIC_IO_ABT_TERM_REQ_NULL;
+ return true;
+ }
- /* original sc used for lr is handled by dev reset code */
- if (sc != lr_sc)
- CMD_SP(sc) = NULL;
+ io_req->abts_done = NULL;
+
+ /* if abort is still pending with fw, fail */
+ if (CMD_ABTS_STATUS(sc) == FCPIO_INVALID_CODE) {
spin_unlock_irqrestore(io_lock, flags);
+ CMD_FLAGS(sc) |= FNIC_IO_ABT_TERM_DONE;
+ iter_data->ret = FAILED;
+ return false;
+ }
+ CMD_STATE(sc) = FNIC_IOREQ_ABTS_COMPLETE;
- /* original sc used for lr is handled by dev reset code */
- if (sc != lr_sc) {
- fnic_release_ioreq_buf(fnic, io_req, sc);
- mempool_free(io_req, fnic->io_req_pool);
- }
+ /* original sc used for lr is handled by dev reset code */
+ if (sc != iter_data->lr_sc)
+ CMD_SP(sc) = NULL;
+ spin_unlock_irqrestore(io_lock, flags);
- /*
- * Any IO is returned during reset, it needs to call scsi_done
- * to return the scsi_cmnd to upper layer.
- */
- if (sc->scsi_done) {
- /* Set result to let upper SCSI layer retry */
- sc->result = DID_RESET << 16;
- sc->scsi_done(sc);
- }
+ /* original sc used for lr is handled by dev reset code */
+ if (sc != iter_data->lr_sc) {
+ fnic_release_ioreq_buf(fnic, io_req, sc);
+ mempool_free(io_req, fnic->io_req_pool);
}
+ /*
+ * Any IO is returned during reset, it needs to call scsi_done
+ * to return the scsi_cmnd to upper layer.
+ */
+ if (sc->scsi_done) {
+ /* Set result to let upper SCSI layer retry */
+ sc->result = DID_RESET << 16;
+ sc->scsi_done(sc);
+ }
+ return true;
+}
+
+/*
+ * Clean up any pending aborts on the lun
+ * For each outstanding IO on this lun, whose abort is not completed by fw,
+ * issue a local abort. Wait for abort to complete. Return 0 if all commands
+ * successfully aborted, 1 otherwise
+ */
+static int fnic_clean_pending_aborts(struct fnic *fnic,
+ struct scsi_cmnd *lr_sc,
+ bool new_sc)
+
+{
+ int ret = SUCCESS;
+ struct fnic_pending_aborts_iter_data iter_data = {
+ .fnic = fnic,
+ .lun_dev = lr_sc->device,
+ .ret = SUCCESS,
+ };
+
+ if (new_sc)
+ iter_data.lr_sc = lr_sc;
+
+ scsi_host_busy_iter(fnic->lport->host,
+ fnic_pending_aborts_iter, &iter_data);
+ if (iter_data.ret == FAILED) {
+ ret = iter_data.ret;
+ goto clean_pending_aborts_end;
+ }
schedule_timeout(msecs_to_jiffies(2 * fnic->config.ed_tov));
/* walk again to check, if IOs are still pending in fw */
@@ -2772,58 +2687,72 @@ void fnic_exch_mgr_reset(struct fc_lport *lp, u32 sid, u32 did)
}
-/*
- * fnic_is_abts_pending() is a helper function that
- * walks through tag map to check if there is any IOs pending,if there is one,
- * then it returns 1 (true), otherwise 0 (false)
- * if @lr_sc is non NULL, then it checks IOs specific to particular LUN,
- * otherwise, it checks for all IOs.
- */
-int fnic_is_abts_pending(struct fnic *fnic, struct scsi_cmnd *lr_sc)
+static bool fnic_abts_pending_iter(struct scsi_cmnd *sc, void *data,
+ bool reserved)
{
- int tag;
+ struct fnic_pending_aborts_iter_data *iter_data = data;
+ struct fnic *fnic = iter_data->fnic;
+ int cmd_state;
struct fnic_io_req *io_req;
spinlock_t *io_lock;
unsigned long flags;
- int ret = 0;
- struct scsi_cmnd *sc;
- struct scsi_device *lun_dev = NULL;
- if (lr_sc)
- lun_dev = lr_sc->device;
+ /*
+ * ignore this lun reset cmd or cmds that do not belong to
+ * this lun
+ */
+ if (iter_data->lr_sc && sc == iter_data->lr_sc)
+ return true;
+ if (iter_data->lun_dev && sc->device != iter_data->lun_dev)
+ return true;
- /* walk again to check, if IOs are still pending in fw */
- for (tag = 0; tag < fnic->fnic_max_tag_id; tag++) {
- sc = scsi_host_find_tag(fnic->lport->host, tag);
- /*
- * ignore this lun reset cmd or cmds that do not belong to
- * this lun
- */
- if (!sc || (lr_sc && (sc->device != lun_dev || sc == lr_sc)))
- continue;
+ io_lock = fnic_io_lock_hash(fnic, sc);
+ spin_lock_irqsave(io_lock, flags);
- io_lock = fnic_io_lock_hash(fnic, sc);
- spin_lock_irqsave(io_lock, flags);
+ io_req = (struct fnic_io_req *)CMD_SP(sc);
+ if (!io_req) {
+ spin_unlock_irqrestore(io_lock, flags);
+ return true;
+ }
- io_req = (struct fnic_io_req *)CMD_SP(sc);
+ /*
+ * Found IO that is still pending with firmware and
+ * belongs to the LUN that we are resetting
+ */
+ FNIC_SCSI_DBG(KERN_INFO, fnic->lport->host,
+ "Found IO in %s on lun\n",
+ fnic_ioreq_state_to_str(CMD_STATE(sc)));
+ cmd_state = CMD_STATE(sc);
+ spin_unlock_irqrestore(io_lock, flags);
+ if (cmd_state == FNIC_IOREQ_ABTS_PENDING)
+ iter_data->ret = 1;
- if (!io_req || sc->device != lun_dev) {
- spin_unlock_irqrestore(io_lock, flags);
- continue;
- }
+ return iter_data->ret ? false : true;
+}
- /*
- * Found IO that is still pending with firmware and
- * belongs to the LUN that we are resetting
- */
- FNIC_SCSI_DBG(KERN_INFO, fnic->lport->host,
- "Found IO in %s on lun\n",
- fnic_ioreq_state_to_str(CMD_STATE(sc)));
+/*
+ * fnic_is_abts_pending() is a helper function that
+ * walks through tag map to check if there is any IOs pending,if there is one,
+ * then it returns 1 (true), otherwise 0 (false)
+ * if @lr_sc is non NULL, then it checks IOs specific to particular LUN,
+ * otherwise, it checks for all IOs.
+ */
+int fnic_is_abts_pending(struct fnic *fnic, struct scsi_cmnd *lr_sc)
+{
+ struct fnic_pending_aborts_iter_data iter_data = {
+ .fnic = fnic,
+ .lun_dev = NULL,
+ .ret = 0,
+ };
- if (CMD_STATE(sc) == FNIC_IOREQ_ABTS_PENDING)
- ret = 1;
- spin_unlock_irqrestore(io_lock, flags);
+ if (lr_sc) {
+ iter_data.lun_dev = lr_sc->device;
+ iter_data.lr_sc = lr_sc;
}
- return ret;
+ /* walk again to check, if IOs are still pending in fw */
+ scsi_host_busy_iter(fnic->lport->host,
+ fnic_abts_pending_iter, &iter_data);
+
+ return iter_data.ret;
}
--
2.29.2
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [PATCH 03/18] scsi: add scsi_{get,put}_internal_cmd() helper
2021-05-03 15:03 [PATCHv8 00/18] scsi: enabled reserved commands for LLDDs Hannes Reinecke
2021-05-03 15:03 ` [PATCH 01/18] fnic: kill 'exclude_id' argument to fnic_cleanup_io() Hannes Reinecke
2021-05-03 15:03 ` [PATCH 02/18] fnic: use scsi_host_busy_iter() to traverse commands Hannes Reinecke
@ 2021-05-03 15:03 ` Hannes Reinecke
2021-05-04 2:21 ` Bart Van Assche
` (2 more replies)
2021-05-03 15:03 ` [PATCH 04/18] fnic: use internal commands Hannes Reinecke
` (14 subsequent siblings)
17 siblings, 3 replies; 55+ messages in thread
From: Hannes Reinecke @ 2021-05-03 15:03 UTC (permalink / raw)
To: Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, John Garry, linux-scsi,
Hannes Reinecke
Add helper functions to allow LLDDs to allocate and free
internal commands.
Signed-off-by: Hannes Reinecke <hare@suse.de>
Reviewed-by: John Garry <john.garry@huawei.com>
---
drivers/scsi/scsi_lib.c | 38 ++++++++++++++++++++++++++++++++++++++
include/scsi/scsi_device.h | 3 +++
2 files changed, 41 insertions(+)
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index d7c0d5a5f263..f83b04e49bae 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -1989,6 +1989,44 @@ void scsi_mq_destroy_tags(struct Scsi_Host *shost)
blk_mq_free_tag_set(&shost->tag_set);
}
+/**
+ * scsi_get_internal_cmd - allocate an internal SCSI command
+ * @sdev: SCSI device from which to allocate the command
+ * @op: operation (REQ_OP_SCSI_IN or REQ_OP_SCSI_OUT)
+ * @flags: BLK_MQ_REQ_* flags, e.g. BLK_MQ_REQ_NOWAIT.
+ *
+ * Allocates a SCSI command for internal LLDD use.
+ */
+struct scsi_cmnd *scsi_get_internal_cmd(struct scsi_device *sdev,
+ unsigned int op, blk_mq_req_flags_t flags)
+{
+ struct request *rq;
+ struct scsi_cmnd *scmd;
+
+ WARN_ON_ONCE(((op & REQ_OP_MASK) != REQ_OP_SCSI_IN) &&
+ ((op & REQ_OP_MASK) != REQ_OP_SCSI_OUT));
+ rq = blk_mq_alloc_request(sdev->request_queue, op, flags);
+ if (IS_ERR(rq))
+ return NULL;
+ scmd = blk_mq_rq_to_pdu(rq);
+ scmd->request = rq;
+ scmd->device = sdev;
+ return scmd;
+}
+EXPORT_SYMBOL_GPL(scsi_get_internal_cmd);
+
+/**
+ * scsi_put_internal_cmd - free an internal SCSI command
+ * @scmd: SCSI command to be freed
+ */
+void scsi_put_internal_cmd(struct scsi_cmnd *scmd)
+{
+ struct request *rq = blk_mq_rq_from_pdu(scmd);
+
+ blk_mq_free_request(rq);
+}
+EXPORT_SYMBOL_GPL(scsi_put_internal_cmd);
+
/**
* scsi_device_from_queue - return sdev associated with a request_queue
* @q: The request queue to return the sdev from
diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
index ac6ab16abee7..eaa5414ff220 100644
--- a/include/scsi/scsi_device.h
+++ b/include/scsi/scsi_device.h
@@ -462,6 +462,9 @@ static inline int scsi_execute_req(struct scsi_device *sdev,
return scsi_execute(sdev, cmd, data_direction, buffer,
bufflen, NULL, sshdr, timeout, retries, 0, 0, resid);
}
+struct scsi_cmnd *scsi_get_internal_cmd(struct scsi_device *sdev,
+ unsigned int op, blk_mq_req_flags_t flags);
+void scsi_put_internal_cmd(struct scsi_cmnd *scmd);
extern void sdev_disable_disk_events(struct scsi_device *sdev);
extern void sdev_enable_disk_events(struct scsi_device *sdev);
extern int scsi_vpd_lun_id(struct scsi_device *, char *, size_t);
--
2.29.2
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [PATCH 04/18] fnic: use internal commands
2021-05-03 15:03 [PATCHv8 00/18] scsi: enabled reserved commands for LLDDs Hannes Reinecke
` (2 preceding siblings ...)
2021-05-03 15:03 ` [PATCH 03/18] scsi: add scsi_{get,put}_internal_cmd() helper Hannes Reinecke
@ 2021-05-03 15:03 ` Hannes Reinecke
2021-05-04 2:25 ` Bart Van Assche
2021-05-03 15:03 ` [PATCH 05/18] scsi: use real inquiry data when initialising devices Hannes Reinecke
` (13 subsequent siblings)
17 siblings, 1 reply; 55+ messages in thread
From: Hannes Reinecke @ 2021-05-03 15:03 UTC (permalink / raw)
To: Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, John Garry, linux-scsi,
Hannes Reinecke
Remove hack to get tag for the reset command by using internal
commands.
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
drivers/scsi/fnic/fnic_scsi.c | 111 +++++++++++++++-------------------
1 file changed, 48 insertions(+), 63 deletions(-)
diff --git a/drivers/scsi/fnic/fnic_scsi.c b/drivers/scsi/fnic/fnic_scsi.c
index 762cc8bd2653..77a85fff8fa1 100644
--- a/drivers/scsi/fnic/fnic_scsi.c
+++ b/drivers/scsi/fnic/fnic_scsi.c
@@ -2169,20 +2169,16 @@ static bool fnic_pending_aborts_iter(struct scsi_cmnd *sc,
* successfully aborted, 1 otherwise
*/
static int fnic_clean_pending_aborts(struct fnic *fnic,
- struct scsi_cmnd *lr_sc,
- bool new_sc)
-
+ struct scsi_cmnd *lr_sc)
{
int ret = SUCCESS;
struct fnic_pending_aborts_iter_data iter_data = {
.fnic = fnic,
.lun_dev = lr_sc->device,
+ .lr_sc = lr_sc,
.ret = SUCCESS,
};
- if (new_sc)
- iter_data.lr_sc = lr_sc;
-
scsi_host_busy_iter(fnic->lport->host,
fnic_pending_aborts_iter, &iter_data);
if (iter_data.ret == FAILED) {
@@ -2247,19 +2243,19 @@ int fnic_device_reset(struct scsi_cmnd *sc)
spinlock_t *io_lock;
unsigned long flags;
unsigned long start_time = 0;
+ struct scsi_device *sdev = sc->device;
struct scsi_lun fc_lun;
struct fnic_stats *fnic_stats;
struct reset_stats *reset_stats;
int tag = 0;
DECLARE_COMPLETION_ONSTACK(tm_done);
- int tag_gen_flag = 0; /*to track tags allocated by fnic driver*/
- bool new_sc = 0;
+ struct scsi_cmnd *reset_sc = NULL;
/* Wait for rport to unblock */
fc_block_scsi_eh(sc);
/* Get local-port, check ready and link up */
- lp = shost_priv(sc->device->host);
+ lp = shost_priv(sdev->host);
fnic = lport_priv(lp);
fnic_stats = &fnic->fnic_stats;
@@ -2267,10 +2263,10 @@ int fnic_device_reset(struct scsi_cmnd *sc)
atomic64_inc(&reset_stats->device_resets);
- rport = starget_to_rport(scsi_target(sc->device));
+ rport = starget_to_rport(scsi_target(sdev));
FNIC_SCSI_DBG(KERN_DEBUG, fnic->lport->host,
"Device reset called FCID 0x%x, LUN 0x%llx sc 0x%p\n",
- rport->port_id, sc->device->lun, sc);
+ rport->port_id, sdev->lun, sc);
if (lp->state != LPORT_ST_READY || !(lp->link_up))
goto fnic_device_reset_end;
@@ -2281,42 +2277,31 @@ int fnic_device_reset(struct scsi_cmnd *sc)
goto fnic_device_reset_end;
}
- CMD_FLAGS(sc) = FNIC_DEVICE_RESET;
- /* Allocate tag if not present */
+ reset_sc = scsi_get_internal_cmd(sdev, REQ_OP_SCSI_IN,
+ BLK_MQ_REQ_NOWAIT);
+ if (unlikely(!reset_sc))
+ goto fnic_device_reset_end;
- tag = sc->request->tag;
- if (unlikely(tag < 0)) {
- /*
- * Really should fix the midlayer to pass in a proper
- * request for ioctls...
- */
- tag = fnic_scsi_host_start_tag(fnic, sc);
- if (unlikely(tag == SCSI_NO_TAG))
- goto fnic_device_reset_end;
- tag_gen_flag = 1;
- new_sc = 1;
- }
- io_lock = fnic_io_lock_hash(fnic, sc);
+ CMD_FLAGS(reset_sc) = FNIC_DEVICE_RESET;
+ tag = reset_sc->request->tag;
+ io_lock = fnic_io_lock_hash(fnic, reset_sc);
spin_lock_irqsave(io_lock, flags);
- io_req = (struct fnic_io_req *)CMD_SP(sc);
/*
- * If there is a io_req attached to this command, then use it,
- * else allocate a new one.
+ * Allocate a new io_req.
*/
+ io_req = mempool_alloc(fnic->io_req_pool, GFP_ATOMIC);
if (!io_req) {
- io_req = mempool_alloc(fnic->io_req_pool, GFP_ATOMIC);
- if (!io_req) {
- spin_unlock_irqrestore(io_lock, flags);
- goto fnic_device_reset_end;
- }
- memset(io_req, 0, sizeof(*io_req));
- io_req->port_id = rport->port_id;
- CMD_SP(sc) = (char *)io_req;
+ spin_unlock_irqrestore(io_lock, flags);
+ goto fnic_device_reset_end;
}
+ memset(io_req, 0, sizeof(*io_req));
+ io_req->port_id = rport->port_id;
+ CMD_SP(reset_sc) = (char *)io_req;
+
io_req->dr_done = &tm_done;
- CMD_STATE(sc) = FNIC_IOREQ_CMD_PENDING;
- CMD_LR_STATUS(sc) = FCPIO_INVALID_CODE;
+ CMD_STATE(reset_sc) = FNIC_IOREQ_CMD_PENDING;
+ CMD_LR_STATUS(reset_sc) = FCPIO_INVALID_CODE;
spin_unlock_irqrestore(io_lock, flags);
FNIC_SCSI_DBG(KERN_DEBUG, fnic->lport->host, "TAG %x\n", tag);
@@ -2325,15 +2310,15 @@ int fnic_device_reset(struct scsi_cmnd *sc)
* issue the device reset, if enqueue failed, clean up the ioreq
* and break assoc with scsi cmd
*/
- if (fnic_queue_dr_io_req(fnic, sc, io_req)) {
+ if (fnic_queue_dr_io_req(fnic, reset_sc, io_req)) {
spin_lock_irqsave(io_lock, flags);
- io_req = (struct fnic_io_req *)CMD_SP(sc);
+ io_req = (struct fnic_io_req *)CMD_SP(reset_sc);
if (io_req)
io_req->dr_done = NULL;
goto fnic_device_reset_clean;
}
spin_lock_irqsave(io_lock, flags);
- CMD_FLAGS(sc) |= FNIC_DEV_RST_ISSUED;
+ CMD_FLAGS(reset_sc) |= FNIC_DEV_RST_ISSUED;
spin_unlock_irqrestore(io_lock, flags);
/*
@@ -2344,16 +2329,16 @@ int fnic_device_reset(struct scsi_cmnd *sc)
msecs_to_jiffies(FNIC_LUN_RESET_TIMEOUT));
spin_lock_irqsave(io_lock, flags);
- io_req = (struct fnic_io_req *)CMD_SP(sc);
+ io_req = (struct fnic_io_req *)CMD_SP(reset_sc);
if (!io_req) {
spin_unlock_irqrestore(io_lock, flags);
FNIC_SCSI_DBG(KERN_DEBUG, fnic->lport->host,
- "io_req is null tag 0x%x sc 0x%p\n", tag, sc);
+ "io_req is null tag 0x%x sc 0x%p\n", tag, reset_sc);
goto fnic_device_reset_end;
}
io_req->dr_done = NULL;
- status = CMD_LR_STATUS(sc);
+ status = CMD_LR_STATUS(reset_sc);
/*
* If lun reset not completed, bail out with failed. io_req
@@ -2363,16 +2348,16 @@ int fnic_device_reset(struct scsi_cmnd *sc)
atomic64_inc(&reset_stats->device_reset_timeouts);
FNIC_SCSI_DBG(KERN_DEBUG, fnic->lport->host,
"Device reset timed out\n");
- CMD_FLAGS(sc) |= FNIC_DEV_RST_TIMED_OUT;
+ CMD_FLAGS(reset_sc) |= FNIC_DEV_RST_TIMED_OUT;
spin_unlock_irqrestore(io_lock, flags);
- int_to_scsilun(sc->device->lun, &fc_lun);
+ int_to_scsilun(sdev->lun, &fc_lun);
/*
* Issue abort and terminate on device reset request.
* If q'ing of terminate fails, retry it after a delay.
*/
while (1) {
spin_lock_irqsave(io_lock, flags);
- if (CMD_FLAGS(sc) & FNIC_DEV_RST_TERM_ISSUED) {
+ if (CMD_FLAGS(reset_sc) & FNIC_DEV_RST_TERM_ISSUED) {
spin_unlock_irqrestore(io_lock, flags);
break;
}
@@ -2385,13 +2370,13 @@ int fnic_device_reset(struct scsi_cmnd *sc)
msecs_to_jiffies(FNIC_ABT_TERM_DELAY_TIMEOUT));
} else {
spin_lock_irqsave(io_lock, flags);
- CMD_FLAGS(sc) |= FNIC_DEV_RST_TERM_ISSUED;
- CMD_STATE(sc) = FNIC_IOREQ_ABTS_PENDING;
+ CMD_FLAGS(reset_sc) |= FNIC_DEV_RST_TERM_ISSUED;
+ CMD_STATE(reset_sc) = FNIC_IOREQ_ABTS_PENDING;
io_req->abts_done = &tm_done;
spin_unlock_irqrestore(io_lock, flags);
FNIC_SCSI_DBG(KERN_DEBUG, fnic->lport->host,
"Abort and terminate issued on Device reset "
- "tag 0x%x sc 0x%p\n", tag, sc);
+ "tag 0x%x sc 0x%p\n", tag, reset_sc);
break;
}
}
@@ -2403,7 +2388,7 @@ int fnic_device_reset(struct scsi_cmnd *sc)
msecs_to_jiffies(FNIC_LUN_RESET_TIMEOUT));
break;
} else {
- io_req = (struct fnic_io_req *)CMD_SP(sc);
+ io_req = (struct fnic_io_req *)CMD_SP(reset_sc);
io_req->abts_done = NULL;
goto fnic_device_reset_clean;
}
@@ -2418,7 +2403,7 @@ int fnic_device_reset(struct scsi_cmnd *sc)
FNIC_SCSI_DBG(KERN_DEBUG,
fnic->lport->host,
"Device reset completed - failed\n");
- io_req = (struct fnic_io_req *)CMD_SP(sc);
+ io_req = (struct fnic_io_req *)CMD_SP(reset_sc);
goto fnic_device_reset_clean;
}
@@ -2429,7 +2414,7 @@ int fnic_device_reset(struct scsi_cmnd *sc)
* the lun reset cmd. If all cmds get cleaned, the lun reset
* succeeds
*/
- if (fnic_clean_pending_aborts(fnic, sc, new_sc)) {
+ if (fnic_clean_pending_aborts(fnic, reset_sc)) {
spin_lock_irqsave(io_lock, flags);
io_req = (struct fnic_io_req *)CMD_SP(sc);
FNIC_SCSI_DBG(KERN_DEBUG, fnic->lport->host,
@@ -2440,35 +2425,35 @@ int fnic_device_reset(struct scsi_cmnd *sc)
/* Clean lun reset command */
spin_lock_irqsave(io_lock, flags);
- io_req = (struct fnic_io_req *)CMD_SP(sc);
+ io_req = (struct fnic_io_req *)CMD_SP(reset_sc);
if (io_req)
/* Completed, and successful */
ret = SUCCESS;
fnic_device_reset_clean:
if (io_req)
- CMD_SP(sc) = NULL;
+ CMD_SP(reset_sc) = NULL;
spin_unlock_irqrestore(io_lock, flags);
if (io_req) {
start_time = io_req->start_time;
- fnic_release_ioreq_buf(fnic, io_req, sc);
+ fnic_release_ioreq_buf(fnic, io_req, reset_sc);
mempool_free(io_req, fnic->io_req_pool);
}
fnic_device_reset_end:
- FNIC_TRACE(fnic_device_reset, sc->device->host->host_no,
- sc->request->tag, sc,
+ FNIC_TRACE(fnic_device_reset, sdev->host->host_no,
+ reset_sc->request->tag, reset_sc,
jiffies_to_msecs(jiffies - start_time),
- 0, ((u64)sc->cmnd[0] << 32 |
+ 0, ((u64)reset_sc->cmnd[0] << 32 |
(u64)sc->cmnd[2] << 24 | (u64)sc->cmnd[3] << 16 |
(u64)sc->cmnd[4] << 8 | sc->cmnd[5]),
(((u64)CMD_FLAGS(sc) << 32) | CMD_STATE(sc)));
- /* free tag if it is allocated */
- if (unlikely(tag_gen_flag))
- fnic_scsi_host_end_tag(fnic, sc);
+ /* free internal command if it is allocated */
+ if (reset_sc)
+ scsi_put_internal_cmd(reset_sc);
FNIC_SCSI_DBG(KERN_DEBUG, fnic->lport->host,
"Returning from device reset %s\n",
--
2.29.2
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [PATCH 05/18] scsi: use real inquiry data when initialising devices
2021-05-03 15:03 [PATCHv8 00/18] scsi: enabled reserved commands for LLDDs Hannes Reinecke
` (3 preceding siblings ...)
2021-05-03 15:03 ` [PATCH 04/18] fnic: use internal commands Hannes Reinecke
@ 2021-05-03 15:03 ` Hannes Reinecke
2021-05-04 2:28 ` Bart Van Assche
2021-05-04 9:55 ` Christoph Hellwig
2021-05-03 15:03 ` [PATCH 06/18] scsi: Use dummy inquiry data for the host device Hannes Reinecke
` (12 subsequent siblings)
17 siblings, 2 replies; 55+ messages in thread
From: Hannes Reinecke @ 2021-05-03 15:03 UTC (permalink / raw)
To: Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, John Garry, linux-scsi,
Hannes Reinecke
Use dummy inquiry data when initialising devices and not just
some 'nullnullnull' string.
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
drivers/scsi/scsi_scan.c | 34 +++++++++++++++++++++++++---------
1 file changed, 25 insertions(+), 9 deletions(-)
diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
index 9f1b7f3c650a..a50abba41ac1 100644
--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -81,7 +81,27 @@
#define SCSI_SCAN_TARGET_PRESENT 1
#define SCSI_SCAN_LUN_PRESENT 2
-static const char *scsi_null_device_strs = "nullnullnullnull";
+/*
+ * Dummy inquiry for virtual LUNs:
+ *
+ * standard INQUIRY: [qualifier indicates no connected LU]
+ * PQual=1 Device_type=31 RMB=0 LU_CONG=0 version=0x05 [SPC-3]
+ * [AERC=0] [TrmTsk=0] NormACA=0 HiSUP=0 Resp_data_format=2
+ * SCCS=0 ACC=0 TPGS=0 3PC=0 Protect=0 [BQue=0]
+ * EncServ=0 MultiP=0 [MChngr=0] [ACKREQQ=0] Addr16=0
+ * [RelAdr=0] WBus16=0 Sync=0 [Linked=0] [TranDis=0] CmdQue=0
+ * length=36 (0x24) Peripheral device type: no physical device on this lu
+ * Vendor identification: LINUX
+ * Product identification: VIRTUALLUN
+ * Product revision level: 1.0
+ */
+static const unsigned char scsi_null_inquiry[36] = {
+ 0x3f, 0x00, 0x05, 0x02, 0x1f, 0x00, 0x00, 0x00,
+ 0x4c, 0x49, 0x4e, 0x55, 0x58, 0x20, 0x20, 0x20,
+ 0x56, 0x49, 0x52, 0x54, 0x55, 0x41, 0x4c, 0x4c,
+ 0x55, 0x4e, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20,
+ 0x31, 0x2e, 0x30, 0x20
+};
#define MAX_SCSI_LUNS 512
@@ -225,9 +245,10 @@ static struct scsi_device *scsi_alloc_sdev(struct scsi_target *starget,
if (!sdev)
goto out;
- sdev->vendor = scsi_null_device_strs;
- sdev->model = scsi_null_device_strs;
- sdev->rev = scsi_null_device_strs;
+ sdev->type = scsi_null_inquiry[0] & 0x1f;
+ sdev->vendor = scsi_null_inquiry + 8;
+ sdev->model = scsi_null_inquiry + 16;
+ sdev->rev = scsi_null_inquiry + 32;
sdev->host = shost;
sdev->queue_ramp_up_period = SCSI_DEFAULT_RAMP_UP_PERIOD;
sdev->id = starget->id;
@@ -254,11 +275,6 @@ static struct scsi_device *scsi_alloc_sdev(struct scsi_target *starget,
* slave_configure function */
sdev->max_device_blocked = SCSI_DEFAULT_DEVICE_BLOCKED;
- /*
- * Some low level driver could use device->type
- */
- sdev->type = -1;
-
/*
* Assume that the device will have handshaking problems,
* and then fix this field later if it turns out it
--
2.29.2
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [PATCH 06/18] scsi: Use dummy inquiry data for the host device
2021-05-03 15:03 [PATCHv8 00/18] scsi: enabled reserved commands for LLDDs Hannes Reinecke
` (4 preceding siblings ...)
2021-05-03 15:03 ` [PATCH 05/18] scsi: use real inquiry data when initialising devices Hannes Reinecke
@ 2021-05-03 15:03 ` Hannes Reinecke
2021-05-04 2:52 ` Bart Van Assche
2021-05-03 15:03 ` [PATCH 07/18] scsi: revamp host device handling Hannes Reinecke
` (11 subsequent siblings)
17 siblings, 1 reply; 55+ messages in thread
From: Hannes Reinecke @ 2021-05-03 15:03 UTC (permalink / raw)
To: Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, John Garry, linux-scsi,
Hannes Reinecke
Attach the dummy inquiry data to the scsi host device and use it
to check if any given device is a scsi host device.
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
drivers/scsi/scsi_scan.c | 17 +++++++++++++++--
drivers/scsi/scsi_sysfs.c | 3 ++-
include/scsi/scsi_host.h | 7 ++++---
3 files changed, 21 insertions(+), 6 deletions(-)
diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
index a50abba41ac1..55fa7f7317a2 100644
--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -1940,9 +1940,11 @@ struct scsi_device *scsi_get_host_dev(struct Scsi_Host *shost)
goto out;
sdev = scsi_alloc_sdev(starget, 0, NULL);
- if (sdev)
+ if (sdev) {
sdev->borken = 0;
- else
+ sdev->inquiry = (unsigned char *)scsi_null_inquiry;
+ sdev->inquiry_len = sizeof(scsi_null_inquiry);
+ } else
scsi_target_reap(starget);
put_device(&starget->dev);
out:
@@ -1967,3 +1969,14 @@ void scsi_free_host_dev(struct scsi_device *sdev)
}
EXPORT_SYMBOL(scsi_free_host_dev);
+/**
+ * scsi_device_is_host_dev - Check if a scsi device is a host device
+ * @sdev: SCSI device to test
+ *
+ * Returns: true if @sdev is a host device, false otherwise
+ */
+bool scsi_device_is_host_dev(struct scsi_device *sdev)
+{
+ return ((const unsigned char *)sdev->inquiry == scsi_null_inquiry);
+}
+EXPORT_SYMBOL_GPL(scsi_device_is_host_dev);
diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index 6d2a28294092..d5260d1b7b38 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -498,7 +498,8 @@ static void scsi_device_dev_release_usercontext(struct work_struct *work)
kfree_rcu(vpd_pg80, rcu);
if (vpd_pg89)
kfree_rcu(vpd_pg89, rcu);
- kfree(sdev->inquiry);
+ if (!scsi_device_is_host_dev(sdev))
+ kfree(sdev->inquiry);
kfree(sdev);
if (parent)
diff --git a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h
index 3f3ebfdedeb2..f492e9034a62 100644
--- a/include/scsi/scsi_host.h
+++ b/include/scsi/scsi_host.h
@@ -804,14 +804,15 @@ void scsi_host_busy_iter(struct Scsi_Host *,
struct class_container;
/*
- * These two functions are used to allocate and free a pseudo device
- * which will connect to the host adapter itself rather than any
- * physical device. You must deallocate when you are done with the
+ * These three functions are used to allocate, free, and test for
+ * a pseudo device which will connect to the host adapter itself rather
+ * than any physical device. You must deallocate when you are done with the
* thing. This physical pseudo-device isn't real and won't be available
* from any high-level drivers.
*/
extern void scsi_free_host_dev(struct scsi_device *);
extern struct scsi_device *scsi_get_host_dev(struct Scsi_Host *);
+bool scsi_device_is_host_dev(struct scsi_device *);
/*
* DIF defines the exchange of protection information between
--
2.29.2
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [PATCH 07/18] scsi: revamp host device handling
2021-05-03 15:03 [PATCHv8 00/18] scsi: enabled reserved commands for LLDDs Hannes Reinecke
` (5 preceding siblings ...)
2021-05-03 15:03 ` [PATCH 06/18] scsi: Use dummy inquiry data for the host device Hannes Reinecke
@ 2021-05-03 15:03 ` Hannes Reinecke
2021-05-04 3:06 ` Bart Van Assche
2021-05-04 9:59 ` Christoph Hellwig
2021-05-03 15:03 ` [PATCH 08/18] snic: use reserved commands Hannes Reinecke
` (10 subsequent siblings)
17 siblings, 2 replies; 55+ messages in thread
From: Hannes Reinecke @ 2021-05-03 15:03 UTC (permalink / raw)
To: Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, John Garry, linux-scsi,
Hannes Reinecke
Ensure that the host device is excluded from scanning by setting
the BLIST_NOLUN flag, and avoid it being presented in sysfs.
Also move the device id from using the ->this_id value as target
id (which is a bit odd as it's typically is set to -1 anyway) to
using ->max_channel + 1 as the channel number and '0' as the
target id.
With that the host device is now handled like any other scsi device,
which means we can drop the scsi_put_host_dev() function and let
scsi_forget_host() etc handle the deallocation.
We only need to ensure that the host device is deallocated last
is the driver might need it to send commands during teardown.
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
drivers/scsi/scsi_devinfo.c | 1 +
drivers/scsi/scsi_scan.c | 55 +++++++++++++++++++------------------
include/scsi/scsi_host.h | 13 +++++----
3 files changed, 37 insertions(+), 32 deletions(-)
diff --git a/drivers/scsi/scsi_devinfo.c b/drivers/scsi/scsi_devinfo.c
index d92cec12454c..e70d87c5342b 100644
--- a/drivers/scsi/scsi_devinfo.c
+++ b/drivers/scsi/scsi_devinfo.c
@@ -195,6 +195,7 @@ static struct {
{"Intel", "Multi-Flex", NULL, BLIST_NO_RSOC},
{"iRiver", "iFP Mass Driver", NULL, BLIST_NOT_LOCKABLE | BLIST_INQUIRY_36},
{"LASOUND", "CDX7405", "3.10", BLIST_MAX5LUN | BLIST_SINGLELUN},
+ {"LINUX", "VIRTUALLUN", NULL, BLIST_NOLUN},
{"Marvell", "Console", NULL, BLIST_SKIP_VPD_PAGES},
{"Marvell", "91xx Config", "1.01", BLIST_SKIP_VPD_PAGES},
{"MATSHITA", "PD-1", NULL, BLIST_FORCELUN | BLIST_SINGLELUN},
diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
index 55fa7f7317a2..0b6753c1dd4d 100644
--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -1113,6 +1113,15 @@ static int scsi_probe_and_add_lun(struct scsi_target *starget,
if (!sdev)
goto out;
+ if (scsi_device_is_host_dev(sdev)) {
+ bflags = scsi_get_device_flags(sdev,
+ sdev->vendor,
+ sdev->model);
+ if (bflagsp)
+ *bflagsp = bflags;
+ return SCSI_SCAN_LUN_PRESENT;
+ }
+
result = kmalloc(result_len, GFP_KERNEL |
((shost->unchecked_isa_dma) ? __GFP_DMA : 0));
if (!result)
@@ -1731,6 +1740,9 @@ static void scsi_sysfs_add_devices(struct Scsi_Host *shost)
/* If device is already visible, skip adding it to sysfs */
if (sdev->is_visible)
continue;
+ /* Host devices should never be visible in sysfs */
+ if (scsi_device_is_host_dev(sdev))
+ continue;
if (!scsi_host_scan_allowed(shost) ||
scsi_sysfs_add_sdev(sdev) != 0)
__scsi_remove_device(sdev);
@@ -1895,12 +1907,16 @@ EXPORT_SYMBOL(scsi_scan_host);
void scsi_forget_host(struct Scsi_Host *shost)
{
- struct scsi_device *sdev;
+ struct scsi_device *sdev, *host_sdev = NULL;
unsigned long flags;
restart:
spin_lock_irqsave(shost->host_lock, flags);
list_for_each_entry(sdev, &shost->__devices, siblings) {
+ if (scsi_device_is_host_dev(sdev)) {
+ host_sdev = sdev;
+ continue;
+ }
if (sdev->sdev_state == SDEV_DEL)
continue;
spin_unlock_irqrestore(shost->host_lock, flags);
@@ -1908,10 +1924,13 @@ void scsi_forget_host(struct Scsi_Host *shost)
goto restart;
}
spin_unlock_irqrestore(shost->host_lock, flags);
+ /* Remove host device last, might be needed to send commands */
+ if (host_sdev)
+ __scsi_remove_device(host_sdev);
}
/**
- * scsi_get_host_dev - Create a scsi_device that points to the host adapter itself
+ * scsi_get_host_dev - Create a virtual scsi_device to the host adapter
* @shost: Host that needs a scsi_device
*
* Lock status: None assumed.
@@ -1919,13 +1938,12 @@ void scsi_forget_host(struct Scsi_Host *shost)
* Returns: The scsi_device or NULL
*
* Notes:
- * Attach a single scsi_device to the Scsi_Host - this should
- * be made to look like a "pseudo-device" that points to the
- * HA itself.
- *
- * Note - this device is not accessible from any high-level
- * drivers (including generics), which is probably not
- * optimal. We can add hooks later to attach.
+ * Attach a single scsi_device to the Scsi_Host. The primary aim
+ * for this device is to serve as a container from which valid
+ * scsi commands can be allocated from. Each scsi command will carry
+ * an unused/free command tag, which then can be used by the LLDD to
+ * send internal or passthrough commands without having to find a
+ * valid command tag internally.
*/
struct scsi_device *scsi_get_host_dev(struct Scsi_Host *shost)
{
@@ -1935,7 +1953,8 @@ struct scsi_device *scsi_get_host_dev(struct Scsi_Host *shost)
mutex_lock(&shost->scan_mutex);
if (!scsi_host_scan_allowed(shost))
goto out;
- starget = scsi_alloc_target(&shost->shost_gendev, 0, shost->this_id);
+ starget = scsi_alloc_target(&shost->shost_gendev,
+ shost->max_channel + 1, 0);
if (!starget)
goto out;
@@ -1953,22 +1972,6 @@ struct scsi_device *scsi_get_host_dev(struct Scsi_Host *shost)
}
EXPORT_SYMBOL(scsi_get_host_dev);
-/**
- * scsi_free_host_dev - Free a scsi_device that points to the host adapter itself
- * @sdev: Host device to be freed
- *
- * Lock status: None assumed.
- *
- * Returns: Nothing
- */
-void scsi_free_host_dev(struct scsi_device *sdev)
-{
- BUG_ON(sdev->id != sdev->host->this_id);
-
- __scsi_remove_device(sdev);
-}
-EXPORT_SYMBOL(scsi_free_host_dev);
-
/**
* scsi_device_is_host_dev - Check if a scsi device is a host device
* @sdev: SCSI device to test
diff --git a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h
index f492e9034a62..f115150559ca 100644
--- a/include/scsi/scsi_host.h
+++ b/include/scsi/scsi_host.h
@@ -804,14 +804,15 @@ void scsi_host_busy_iter(struct Scsi_Host *,
struct class_container;
/*
- * These three functions are used to allocate, free, and test for
- * a pseudo device which will connect to the host adapter itself rather
- * than any physical device. You must deallocate when you are done with the
- * thing. This physical pseudo-device isn't real and won't be available
+ * These functions are used to allocate and test a pseudo device
+ * which will refer to the host adapter itself rather than any
+ * physical device. The device will be deallocated together with
+ * all other scsi devices, so there is no need to have a separate
+ * function to free it.
+ * This device will not show up in sysfs and won't be available
* from any high-level drivers.
*/
-extern void scsi_free_host_dev(struct scsi_device *);
-extern struct scsi_device *scsi_get_host_dev(struct Scsi_Host *);
+struct scsi_device *scsi_get_host_dev(struct Scsi_Host *);
bool scsi_device_is_host_dev(struct scsi_device *);
/*
--
2.29.2
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [PATCH 08/18] snic: use reserved commands
2021-05-03 15:03 [PATCHv8 00/18] scsi: enabled reserved commands for LLDDs Hannes Reinecke
` (6 preceding siblings ...)
2021-05-03 15:03 ` [PATCH 07/18] scsi: revamp host device handling Hannes Reinecke
@ 2021-05-03 15:03 ` Hannes Reinecke
2021-05-04 3:12 ` Bart Van Assche
2021-05-03 15:03 ` [PATCH 09/18] snic: use tagset iter for traversing commands Hannes Reinecke
` (9 subsequent siblings)
17 siblings, 1 reply; 55+ messages in thread
From: Hannes Reinecke @ 2021-05-03 15:03 UTC (permalink / raw)
To: Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, John Garry, linux-scsi,
Hannes Reinecke
Allocate a host device and use internal commands
for host and device reset.
This allows us to remove the special handling of
host and bus reset commands.
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
drivers/scsi/snic/snic.h | 4 +-
drivers/scsi/snic/snic_main.c | 7 ++
drivers/scsi/snic/snic_scsi.c | 160 ++++++++++++++--------------------
3 files changed, 74 insertions(+), 97 deletions(-)
diff --git a/drivers/scsi/snic/snic.h b/drivers/scsi/snic/snic.h
index f4c666285bba..5a2428033190 100644
--- a/drivers/scsi/snic/snic.h
+++ b/drivers/scsi/snic/snic.h
@@ -59,7 +59,6 @@
*/
#define SNIC_TAG_ABORT BIT(30) /* Tag indicating abort */
#define SNIC_TAG_DEV_RST BIT(29) /* Tag for device reset */
-#define SNIC_TAG_IOCTL_DEV_RST BIT(28) /* Tag for User Device Reset */
#define SNIC_TAG_MASK (BIT(24) - 1) /* Mask for lookup */
#define SNIC_NO_TAG -1
@@ -278,6 +277,7 @@ struct snic {
/* Scsi Host info */
struct Scsi_Host *shost;
+ struct scsi_device *shost_dev;
/* vnic related structures */
struct vnic_dev_bar bar0;
@@ -380,7 +380,7 @@ int snic_queuecommand(struct Scsi_Host *, struct scsi_cmnd *);
int snic_abort_cmd(struct scsi_cmnd *);
int snic_device_reset(struct scsi_cmnd *);
int snic_host_reset(struct scsi_cmnd *);
-int snic_reset(struct Scsi_Host *, struct scsi_cmnd *);
+int snic_reset(struct Scsi_Host *);
void snic_shutdown_scsi_cleanup(struct snic *);
diff --git a/drivers/scsi/snic/snic_main.c b/drivers/scsi/snic/snic_main.c
index 14f4ce665e58..ce0e7ab4ef1d 100644
--- a/drivers/scsi/snic/snic_main.c
+++ b/drivers/scsi/snic/snic_main.c
@@ -303,6 +303,7 @@ static int
snic_add_host(struct Scsi_Host *shost, struct pci_dev *pdev)
{
int ret = 0;
+ struct snic *snic = shost_priv(shost);
ret = scsi_add_host(shost, &pdev->dev);
if (ret) {
@@ -313,6 +314,12 @@ snic_add_host(struct Scsi_Host *shost, struct pci_dev *pdev)
return ret;
}
+ snic->shost_dev = scsi_get_host_dev(shost);
+ if (!snic->shost_dev) {
+ SNIC_HOST_ERR(shost,
+ "snic: scsi_get_virtual_dev failed\n");
+ return -ENOMEM;
+ }
SNIC_BUG_ON(shost->work_q != NULL);
snprintf(shost->work_q_name, sizeof(shost->work_q_name), "scsi_wq_%d",
shost->host_no);
diff --git a/drivers/scsi/snic/snic_scsi.c b/drivers/scsi/snic/snic_scsi.c
index 6dd0ff188bb4..63cb9f81f82b 100644
--- a/drivers/scsi/snic/snic_scsi.c
+++ b/drivers/scsi/snic/snic_scsi.c
@@ -77,7 +77,7 @@ static const char * const snic_io_status_str[] = {
[SNIC_STAT_FATAL_ERROR] = "SNIC_STAT_FATAL_ERROR",
};
-static void snic_scsi_cleanup(struct snic *, int);
+static void snic_scsi_cleanup(struct snic *);
const char *
snic_state_to_str(unsigned int state)
@@ -867,7 +867,6 @@ snic_process_itmf_cmpl(struct snic *snic,
break;
case SNIC_TAG_DEV_RST:
- case SNIC_TAG_DEV_RST | SNIC_TAG_IOCTL_DEV_RST:
snic_proc_dr_cmpl_locked(snic, fwreq, cmpl_stat, cmnd_id, sc);
spin_unlock_irqrestore(io_lock, flags);
ret = 0;
@@ -920,7 +919,6 @@ static void
snic_itmf_cmpl_handler(struct snic *snic, struct snic_fw_req *fwreq)
{
struct scsi_cmnd *sc = NULL;
- struct snic_req_info *rqi = NULL;
struct snic_itmf_cmpl *itmf_cmpl = NULL;
ulong ctx;
u32 cmnd_id;
@@ -938,14 +936,6 @@ snic_itmf_cmpl_handler(struct snic *snic, struct snic_fw_req *fwreq)
"Itmf_cmpl: nterm %u , flags 0x%x\n",
le32_to_cpu(itmf_cmpl->nterminated), itmf_cmpl->flags);
- /* spl case, dev reset issued through ioctl */
- if (cmnd_id & SNIC_TAG_IOCTL_DEV_RST) {
- rqi = (struct snic_req_info *) ctx;
- sc = rqi->sc;
-
- goto ioctl_dev_rst;
- }
-
if ((cmnd_id & SNIC_TAG_MASK) >= snic->max_tag_id) {
SNIC_HOST_ERR(snic->shost,
"Itmf_cmpl: Tag 0x%x out of Range,HdrStat %s\n",
@@ -958,7 +948,6 @@ snic_itmf_cmpl_handler(struct snic *snic, struct snic_fw_req *fwreq)
sc = scsi_host_find_tag(snic->shost, cmnd_id & SNIC_TAG_MASK);
WARN_ON_ONCE(!sc);
-ioctl_dev_rst:
if (!sc) {
atomic64_inc(&snic->s_stats.io.sc_null);
SNIC_HOST_ERR(snic->shost,
@@ -974,13 +963,13 @@ snic_itmf_cmpl_handler(struct snic *snic, struct snic_fw_req *fwreq)
static void
-snic_hba_reset_scsi_cleanup(struct snic *snic, struct scsi_cmnd *sc)
+snic_hba_reset_scsi_cleanup(struct snic *snic)
{
struct snic_stats *st = &snic->s_stats;
long act_ios = 0, act_fwreqs = 0;
SNIC_SCSI_DBG(snic->shost, "HBA Reset scsi cleanup.\n");
- snic_scsi_cleanup(snic, snic_cmd_tag(sc));
+ snic_scsi_cleanup(snic);
/* Update stats on pending IOs */
act_ios = atomic64_read(&st->io.active);
@@ -1021,17 +1010,6 @@ snic_hba_reset_cmpl_handler(struct snic *snic, struct snic_fw_req *fwreq)
"reset_cmpl: type = %x, hdr_stat = %x, cmnd_id = %x, hid = %x, ctx = %lx\n",
typ, hdr_stat, cmnd_id, hid, ctx);
- /* spl case, host reset issued through ioctl */
- if (cmnd_id == SCSI_NO_TAG) {
- rqi = (struct snic_req_info *) ctx;
- SNIC_HOST_INFO(snic->shost,
- "reset_cmpl:Tag %d ctx %lx cmpl stat %s\n",
- cmnd_id, ctx, snic_io_status_to_str(hdr_stat));
- sc = rqi->sc;
-
- goto ioctl_hba_rst;
- }
-
if (cmnd_id >= snic->max_tag_id) {
SNIC_HOST_ERR(snic->shost,
"reset_cmpl: Tag 0x%x out of Range,HdrStat %s\n",
@@ -1042,7 +1020,6 @@ snic_hba_reset_cmpl_handler(struct snic *snic, struct snic_fw_req *fwreq)
}
sc = scsi_host_find_tag(snic->shost, cmnd_id);
-ioctl_hba_rst:
if (!sc) {
atomic64_inc(&snic->s_stats.io.sc_null);
SNIC_HOST_ERR(snic->shost,
@@ -1089,7 +1066,7 @@ snic_hba_reset_cmpl_handler(struct snic *snic, struct snic_fw_req *fwreq)
spin_unlock_irqrestore(io_lock, flags);
/* scsi cleanup */
- snic_hba_reset_scsi_cleanup(snic, sc);
+ snic_hba_reset_scsi_cleanup(snic);
SNIC_BUG_ON(snic_get_state(snic) != SNIC_OFFLINE &&
snic_get_state(snic) != SNIC_FWRESET);
@@ -1359,7 +1336,7 @@ snic_issue_tm_req(struct snic *snic,
int tmf)
{
struct snic_host_req *tmreq = NULL;
- int req_id = 0, tag = snic_cmd_tag(sc);
+ int tag = snic_cmd_tag(sc);
int ret = 0;
if (snic_get_state(snic) == SNIC_FWRESET)
@@ -1372,13 +1349,10 @@ snic_issue_tm_req(struct snic *snic,
tmf, rqi, tag);
- if (tmf == SNIC_ITMF_LUN_RESET) {
+ if (tmf == SNIC_ITMF_LUN_RESET)
tmreq = snic_dr_req_init(snic, rqi);
- req_id = SCSI_NO_TAG;
- } else {
+ else
tmreq = snic_abort_req_init(snic, rqi);
- req_id = tag;
- }
if (!tmreq) {
ret = -ENOMEM;
@@ -1386,17 +1360,17 @@ snic_issue_tm_req(struct snic *snic,
goto tmreq_err;
}
- ret = snic_queue_itmf_req(snic, tmreq, sc, tmf, req_id);
+ ret = snic_queue_itmf_req(snic, tmreq, sc, tmf, tag);
tmreq_err:
if (ret) {
SNIC_HOST_ERR(snic->shost,
- "issu_tmreq: Queueing ITMF(%d) Req, sc %p rqi %p req_id %d tag %x fails err = %d\n",
- tmf, sc, rqi, req_id, tag, ret);
+ "issu_tmreq: Queing ITMF(%d) Req, sc %p rqi %p tag %x fails err = %d\n",
+ tmf, sc, rqi, tag, ret);
} else {
SNIC_SCSI_DBG(snic->shost,
- "issu_tmreq: Queueing ITMF(%d) Req, sc %p, rqi %p, req_id %d tag %x - Success.\n",
- tmf, sc, rqi, req_id, tag);
+ "issu_tmreq: Queuing ITMF(%d) Req, sc %p, rqi %p, tag %x - Success.\n",
+ tmf, sc, rqi, tag);
}
atomic_dec(&snic->ios_inflight);
@@ -2146,7 +2120,7 @@ snic_device_reset(struct scsi_cmnd *sc)
struct Scsi_Host *shost = sc->device->host;
struct snic *snic = shost_priv(shost);
struct snic_req_info *rqi = NULL;
- int tag = snic_cmd_tag(sc);
+ struct scsi_cmnd *reset_sc = NULL;
int start_time = jiffies;
int ret = FAILED;
int dr_supp = 0;
@@ -2170,42 +2144,42 @@ snic_device_reset(struct scsi_cmnd *sc)
goto dev_rst_end;
}
- /* There is no tag when lun reset is issue through ioctl. */
- if (unlikely(tag <= SNIC_NO_TAG)) {
- SNIC_HOST_INFO(snic->shost,
- "Devrst: LUN Reset Recvd thru IOCTL.\n");
+ reset_sc = scsi_get_internal_cmd(sc->device, REQ_OP_SCSI_IN,
+ BLK_MQ_REQ_NOWAIT);
+ if (!reset_sc)
+ goto dev_rst_end;
- rqi = snic_req_init(snic, 0);
- if (!rqi)
- goto dev_rst_end;
+ rqi = snic_req_init(snic, 0);
+ if (!rqi)
+ goto dev_rst_end;
- memset(scsi_cmd_priv(sc), 0,
- sizeof(struct snic_internal_io_state));
- CMD_SP(sc) = (char *)rqi;
- CMD_FLAGS(sc) = SNIC_NO_FLAGS;
+ memset(scsi_cmd_priv(reset_sc), 0,
+ sizeof(struct snic_internal_io_state));
+ CMD_SP(reset_sc) = (char *)rqi;
+ CMD_FLAGS(reset_sc) = SNIC_NO_FLAGS;
- /* Add special tag for dr coming from user spc */
- rqi->tm_tag = SNIC_TAG_IOCTL_DEV_RST;
- rqi->sc = sc;
- }
+ rqi->sc = reset_sc;
- ret = snic_send_dr_and_wait(snic, sc);
+ ret = snic_send_dr_and_wait(snic, reset_sc);
if (ret) {
SNIC_HOST_ERR(snic->shost,
"Devrst: IO w/ Tag %x Failed w/ err = %d\n",
- tag, ret);
+ snic_cmd_tag(reset_sc), ret);
- snic_unlink_and_release_req(snic, sc, 0);
+ snic_unlink_and_release_req(snic, reset_sc, 0);
goto dev_rst_end;
}
- ret = snic_dr_finish(snic, sc);
+ ret = snic_dr_finish(snic, reset_sc);
dev_rst_end:
- SNIC_TRC(snic->shost->host_no, tag, (ulong) sc,
- jiffies_to_msecs(jiffies - start_time),
- 0, SNIC_TRC_CMD(sc), SNIC_TRC_CMD_STATE_FLAGS(sc));
+ if (reset_sc) {
+ SNIC_TRC(snic->shost->host_no, snic_cmd_tag(reset_sc), (ulong) reset_sc,
+ jiffies_to_msecs(jiffies - start_time),
+ 0, SNIC_TRC_CMD(reset_sc), SNIC_TRC_CMD_STATE_FLAGS(reset_sc));
+ scsi_put_internal_cmd(reset_sc);
+ }
SNIC_SCSI_DBG(snic->shost,
"Devrst: Returning from Device Reset : %s\n",
@@ -2225,10 +2199,11 @@ snic_device_reset(struct scsi_cmnd *sc)
* snic_issue_hba_reset : Queues FW Reset Request.
*/
static int
-snic_issue_hba_reset(struct snic *snic, struct scsi_cmnd *sc)
+snic_issue_hba_reset(struct snic *snic)
{
struct snic_req_info *rqi = NULL;
struct snic_host_req *req = NULL;
+ struct scsi_cmnd *reset_sc;
spinlock_t *io_lock = NULL;
DECLARE_COMPLETION_ONSTACK(wait);
unsigned long flags;
@@ -2237,30 +2212,31 @@ snic_issue_hba_reset(struct snic *snic, struct scsi_cmnd *sc)
rqi = snic_req_init(snic, 0);
if (!rqi) {
ret = -ENOMEM;
-
goto hba_rst_end;
}
- if (snic_cmd_tag(sc) == SCSI_NO_TAG) {
- memset(scsi_cmd_priv(sc), 0,
- sizeof(struct snic_internal_io_state));
- SNIC_HOST_INFO(snic->shost, "issu_hr:Host reset thru ioctl.\n");
- rqi->sc = sc;
+ reset_sc = scsi_get_internal_cmd(snic->shost_dev,
+ DMA_NONE, REQ_NOWAIT);
+ if (!reset_sc) {
+ ret = -EBUSY;
+ goto hba_rst_end_put;
}
-
+ memset(scsi_cmd_priv(reset_sc), 0,
+ sizeof(struct snic_internal_io_state));
+ rqi->sc = reset_sc;
req = rqi_to_req(rqi);
- io_lock = snic_io_lock_hash(snic, sc);
+ io_lock = snic_io_lock_hash(snic, reset_sc);
spin_lock_irqsave(io_lock, flags);
- SNIC_BUG_ON(CMD_SP(sc) != NULL);
- CMD_STATE(sc) = SNIC_IOREQ_PENDING;
- CMD_SP(sc) = (char *) rqi;
- CMD_FLAGS(sc) |= SNIC_IO_INITIALIZED;
+ SNIC_BUG_ON(CMD_SP(reset_sc) != NULL);
+ CMD_STATE(reset_sc) = SNIC_IOREQ_PENDING;
+ CMD_SP(reset_sc) = (char *) rqi;
+ CMD_FLAGS(reset_sc) |= SNIC_IO_INITIALIZED;
snic->remove_wait = &wait;
spin_unlock_irqrestore(io_lock, flags);
/* Initialize Request */
- snic_io_hdr_enc(&req->hdr, SNIC_REQ_HBA_RESET, 0, snic_cmd_tag(sc),
+ snic_io_hdr_enc(&req->hdr, SNIC_REQ_HBA_RESET, 0, snic_cmd_tag(reset_sc),
snic->config.hid, 0, (ulong) rqi);
req->u.reset.flags = 0;
@@ -2275,7 +2251,7 @@ snic_issue_hba_reset(struct snic *snic, struct scsi_cmnd *sc)
}
spin_lock_irqsave(io_lock, flags);
- CMD_FLAGS(sc) |= SNIC_HOST_RESET_ISSUED;
+ CMD_FLAGS(reset_sc) |= SNIC_HOST_RESET_ISSUED;
spin_unlock_irqrestore(io_lock, flags);
atomic64_inc(&snic->s_stats.reset.hba_resets);
SNIC_HOST_INFO(snic->shost, "Queued HBA Reset Successfully.\n");
@@ -2292,13 +2268,14 @@ snic_issue_hba_reset(struct snic *snic, struct scsi_cmnd *sc)
spin_lock_irqsave(io_lock, flags);
snic->remove_wait = NULL;
- rqi = (struct snic_req_info *) CMD_SP(sc);
- CMD_SP(sc) = NULL;
+ rqi = (struct snic_req_info *) CMD_SP(reset_sc);
+ CMD_SP(reset_sc) = NULL;
spin_unlock_irqrestore(io_lock, flags);
if (rqi)
snic_req_free(snic, rqi);
+ scsi_put_internal_cmd(reset_sc);
ret = 0;
return ret;
@@ -2306,10 +2283,13 @@ snic_issue_hba_reset(struct snic *snic, struct scsi_cmnd *sc)
hba_rst_err:
spin_lock_irqsave(io_lock, flags);
snic->remove_wait = NULL;
- rqi = (struct snic_req_info *) CMD_SP(sc);
- CMD_SP(sc) = NULL;
+ rqi = (struct snic_req_info *) CMD_SP(reset_sc);
+ CMD_SP(reset_sc) = NULL;
spin_unlock_irqrestore(io_lock, flags);
+hba_rst_end_put:
+ if (reset_sc)
+ scsi_put_internal_cmd(reset_sc);
if (rqi)
snic_req_free(snic, rqi);
@@ -2322,7 +2302,7 @@ snic_issue_hba_reset(struct snic *snic, struct scsi_cmnd *sc)
} /* end of snic_issue_hba_reset */
int
-snic_reset(struct Scsi_Host *shost, struct scsi_cmnd *sc)
+snic_reset(struct Scsi_Host *shost)
{
struct snic *snic = shost_priv(shost);
enum snic_state sv_state;
@@ -2351,7 +2331,7 @@ snic_reset(struct Scsi_Host *shost, struct scsi_cmnd *sc)
while (atomic_read(&snic->ios_inflight))
schedule_timeout(msecs_to_jiffies(1));
- ret = snic_issue_hba_reset(snic, sc);
+ ret = snic_issue_hba_reset(snic);
if (ret) {
SNIC_HOST_ERR(shost,
"reset:Host Reset Failed w/ err %d.\n",
@@ -2390,7 +2370,7 @@ snic_host_reset(struct scsi_cmnd *sc)
sc, sc->cmnd[0], sc->request,
snic_cmd_tag(sc), CMD_FLAGS(sc));
- ret = snic_reset(shost, sc);
+ ret = snic_reset(shost);
SNIC_TRC(shost->host_no, snic_cmd_tag(sc), (ulong) sc,
jiffies_to_msecs(jiffies - start_time),
@@ -2432,7 +2412,7 @@ snic_cmpl_pending_tmreq(struct snic *snic, struct scsi_cmnd *sc)
* snic_scsi_cleanup: Walks through tag map and releases the reqs
*/
static void
-snic_scsi_cleanup(struct snic *snic, int ex_tag)
+snic_scsi_cleanup(struct snic *snic)
{
struct snic_req_info *rqi = NULL;
struct scsi_cmnd *sc = NULL;
@@ -2444,19 +2424,9 @@ snic_scsi_cleanup(struct snic *snic, int ex_tag)
SNIC_SCSI_DBG(snic->shost, "sc_clean: scsi cleanup.\n");
for (tag = 0; tag < snic->max_tag_id; tag++) {
- /* Skip ex_tag */
- if (tag == ex_tag)
- continue;
-
io_lock = snic_io_lock_tag(snic, tag);
spin_lock_irqsave(io_lock, flags);
sc = scsi_host_find_tag(snic->shost, tag);
- if (!sc) {
- spin_unlock_irqrestore(io_lock, flags);
-
- continue;
- }
-
if (unlikely(snic_tmreq_pending(sc))) {
/*
* When FW Completes reset w/o sending completions
@@ -2516,7 +2486,7 @@ snic_shutdown_scsi_cleanup(struct snic *snic)
{
SNIC_HOST_INFO(snic->shost, "Shutdown time SCSI Cleanup.\n");
- snic_scsi_cleanup(snic, SCSI_NO_TAG);
+ snic_scsi_cleanup(snic);
} /* end of snic_shutdown_scsi_cleanup */
/*
--
2.29.2
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [PATCH 09/18] snic: use tagset iter for traversing commands
2021-05-03 15:03 [PATCHv8 00/18] scsi: enabled reserved commands for LLDDs Hannes Reinecke
` (7 preceding siblings ...)
2021-05-03 15:03 ` [PATCH 08/18] snic: use reserved commands Hannes Reinecke
@ 2021-05-03 15:03 ` Hannes Reinecke
2021-05-04 9:59 ` Christoph Hellwig
2021-05-03 15:03 ` [PATCH 10/18] scsi: implement reserved command handling Hannes Reinecke
` (8 subsequent siblings)
17 siblings, 1 reply; 55+ messages in thread
From: Hannes Reinecke @ 2021-05-03 15:03 UTC (permalink / raw)
To: Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, John Garry, linux-scsi,
Hannes Reinecke
Use the tagset iter to traverse active commands during device and
hba reset.
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
drivers/scsi/snic/snic_scsi.c | 366 +++++++++++++++++-----------------
1 file changed, 187 insertions(+), 179 deletions(-)
diff --git a/drivers/scsi/snic/snic_scsi.c b/drivers/scsi/snic/snic_scsi.c
index 63cb9f81f82b..7bffad6626f8 100644
--- a/drivers/scsi/snic/snic_scsi.c
+++ b/drivers/scsi/snic/snic_scsi.c
@@ -1641,90 +1641,97 @@ snic_abort_cmd(struct scsi_cmnd *sc)
return ret;
}
+struct snic_cmd_pending_iter_data {
+ struct snic *snic;
+ struct scsi_device *sdev;
+ int ret;
+};
-
-static int
-snic_is_abts_pending(struct snic *snic, struct scsi_cmnd *lr_sc)
+static bool
+snic_is_abts_pending_iter(struct scsi_cmnd *sc, void *data, bool reserved)
{
+ struct snic_cmd_pending_iter_data *iter_data = data;
struct snic_req_info *rqi = NULL;
- struct scsi_cmnd *sc = NULL;
- struct scsi_device *lr_sdev = NULL;
spinlock_t *io_lock = NULL;
- u32 tag;
unsigned long flags;
- if (lr_sc)
- lr_sdev = lr_sc->device;
+ if (reserved)
+ return true;
- /* walk through the tag map, an dcheck if IOs are still pending in fw*/
- for (tag = 0; tag < snic->max_tag_id; tag++) {
- io_lock = snic_io_lock_tag(snic, tag);
-
- spin_lock_irqsave(io_lock, flags);
- sc = scsi_host_find_tag(snic->shost, tag);
-
- if (!sc || (lr_sc && (sc->device != lr_sdev || sc == lr_sc))) {
- spin_unlock_irqrestore(io_lock, flags);
+ if (iter_data->sdev && iter_data->sdev != sc->device)
+ return true;
- continue;
- }
+ io_lock = snic_io_lock_hash(iter_data->snic, sc);
+ spin_lock_irqsave(io_lock, flags);
- rqi = (struct snic_req_info *) CMD_SP(sc);
- if (!rqi) {
- spin_unlock_irqrestore(io_lock, flags);
+ rqi = (struct snic_req_info *) CMD_SP(sc);
+ if (!rqi) {
+ spin_unlock_irqrestore(io_lock, flags);
+ return true;
+ }
- continue;
- }
+ /*
+ * Found IO that is still pending w/ firmware and belongs to
+ * the LUN that is under reset, if lr_sc != NULL
+ */
+ SNIC_SCSI_DBG(iter_data->snic->shost, "Found IO in %s on LUN\n",
+ snic_ioreq_state_to_str(CMD_STATE(sc)));
- /*
- * Found IO that is still pending w/ firmware and belongs to
- * the LUN that is under reset, if lr_sc != NULL
- */
- SNIC_SCSI_DBG(snic->shost, "Found IO in %s on LUN\n",
- snic_ioreq_state_to_str(CMD_STATE(sc)));
+ if (CMD_STATE(sc) == SNIC_IOREQ_ABTS_PENDING)
+ iter_data->ret = 1;
+ spin_unlock_irqrestore(io_lock, flags);
- if (CMD_STATE(sc) == SNIC_IOREQ_ABTS_PENDING) {
- spin_unlock_irqrestore(io_lock, flags);
+ return true;
+}
- return 1;
- }
+static int
+snic_is_abts_pending(struct snic *snic, struct scsi_device *lr_sdev)
+{
+ struct snic_cmd_pending_iter_data iter_data = {
+ .snic = snic,
+ .sdev = lr_sdev,
+ .ret = 0,
+ };
- spin_unlock_irqrestore(io_lock, flags);
- }
+ /* walk through the tag map, an dcheck if IOs are still pending in fw*/
+ scsi_host_busy_iter(snic->shost,
+ snic_is_abts_pending_iter, &iter_data);
- return 0;
+ return iter_data.ret;
} /* end of snic_is_abts_pending */
-static int
-snic_dr_clean_single_req(struct snic *snic,
- u32 tag,
- struct scsi_device *lr_sdev)
+static bool
+snic_dr_clean_single_req(struct scsi_cmnd *sc, void *data, bool reserved)
{
struct snic_req_info *rqi = NULL;
struct snic_tgt *tgt = NULL;
- struct scsi_cmnd *sc = NULL;
spinlock_t *io_lock = NULL;
u32 sv_state = 0, tmf = 0;
DECLARE_COMPLETION_ONSTACK(tm_done);
unsigned long flags;
int ret = 0;
+ struct snic_cmd_pending_iter_data *iter_data = data;
+ struct snic *snic = iter_data->snic;
- io_lock = snic_io_lock_tag(snic, tag);
- spin_lock_irqsave(io_lock, flags);
- sc = scsi_host_find_tag(snic->shost, tag);
+ if (reserved)
+ return true;
- /* Ignore Cmd that don't belong to Lun Reset device */
- if (!sc || sc->device != lr_sdev)
- goto skip_clean;
+ if (sc->device != iter_data->sdev)
+ return true;
+ io_lock = snic_io_lock_hash(snic, sc);
+ spin_lock_irqsave(io_lock, flags);
rqi = (struct snic_req_info *) CMD_SP(sc);
-
- if (!rqi)
- goto skip_clean;
+ if (!rqi) {
+ spin_unlock_irqrestore(io_lock, flags);
+ return true;
+ }
- if (CMD_STATE(sc) == SNIC_IOREQ_ABTS_PENDING)
- goto skip_clean;
+ if (CMD_STATE(sc) == SNIC_IOREQ_ABTS_PENDING) {
+ spin_unlock_irqrestore(io_lock, flags);
+ return true;
+ }
if ((CMD_FLAGS(sc) & SNIC_DEVICE_RESET) &&
@@ -1733,8 +1740,8 @@ snic_dr_clean_single_req(struct snic *snic,
SNIC_SCSI_DBG(snic->shost,
"clean_single_req: devrst is not pending sc 0x%p\n",
sc);
-
- goto skip_clean;
+ spin_unlock_irqrestore(io_lock, flags);
+ return true;
}
SNIC_SCSI_DBG(snic->shost,
@@ -1777,7 +1784,7 @@ snic_dr_clean_single_req(struct snic *snic,
if (ret) {
SNIC_HOST_ERR(snic->shost,
"clean_single_req_err:sc %p, tag %d abt failed. tm_tag %d flags 0x%llx\n",
- sc, tag, rqi->tm_tag, CMD_FLAGS(sc));
+ sc, snic_cmd_tag(sc), rqi->tm_tag, CMD_FLAGS(sc));
spin_lock_irqsave(io_lock, flags);
rqi = (struct snic_req_info *) CMD_SP(sc);
@@ -1788,8 +1795,9 @@ snic_dr_clean_single_req(struct snic *snic,
if (CMD_STATE(sc) == SNIC_IOREQ_ABTS_PENDING)
CMD_STATE(sc) = sv_state;
- ret = 1;
- goto skip_clean;
+ iter_data->ret = 1;
+ spin_unlock_irqrestore(io_lock, flags);
+ return false;
}
spin_lock_irqsave(io_lock, flags);
@@ -1806,7 +1814,8 @@ snic_dr_clean_single_req(struct snic *snic,
rqi = (struct snic_req_info *) CMD_SP(sc);
if (!rqi) {
CMD_FLAGS(sc) |= SNIC_IO_ABTS_TERM_REQ_NULL;
- goto skip_clean;
+ spin_unlock_irqrestore(io_lock, flags);
+ return true;;
}
rqi->abts_done = NULL;
@@ -1814,14 +1823,13 @@ snic_dr_clean_single_req(struct snic *snic,
if (CMD_ABTS_STATUS(sc) == SNIC_INVALID_CODE) {
SNIC_HOST_ERR(snic->shost,
"clean_single_req_err:sc %p tag %d abt still pending w/ fw, tm_tag %d flags 0x%llx\n",
- sc, tag, rqi->tm_tag, CMD_FLAGS(sc));
+ sc, snic_cmd_tag(sc), rqi->tm_tag, CMD_FLAGS(sc));
CMD_FLAGS(sc) |= SNIC_IO_ABTS_TERM_DONE;
- ret = 1;
-
- goto skip_clean;
+ iter_data->ret = 1;
+ spin_unlock_irqrestore(io_lock, flags);
+ return false;
}
-
CMD_STATE(sc) = SNIC_IOREQ_ABTS_COMPLETE;
CMD_SP(sc) = NULL;
spin_unlock_irqrestore(io_lock, flags);
@@ -1831,39 +1839,31 @@ snic_dr_clean_single_req(struct snic *snic,
sc->result = (DID_ERROR << 16);
sc->scsi_done(sc);
- ret = 0;
-
- return ret;
-
-skip_clean:
- spin_unlock_irqrestore(io_lock, flags);
-
- return ret;
+ return true;
} /* end of snic_dr_clean_single_req */
static int
-snic_dr_clean_pending_req(struct snic *snic, struct scsi_cmnd *lr_sc)
+snic_dr_clean_pending_req(struct snic *snic, struct scsi_device *lr_sdev)
{
- struct scsi_device *lr_sdev = lr_sc->device;
- u32 tag = 0;
int ret = FAILED;
+ struct snic_cmd_pending_iter_data iter_data = {
+ .snic = snic,
+ .sdev = lr_sdev,
+ .ret = 0,
+ };
- for (tag = 0; tag < snic->max_tag_id; tag++) {
- if (tag == snic_cmd_tag(lr_sc))
- continue;
+ scsi_host_busy_iter(snic->shost,
+ snic_dr_clean_single_req, &iter_data);
+ if (iter_data.ret) {
+ SNIC_HOST_ERR(snic->shost, "clean_err = %d\n", iter_data.ret);
- ret = snic_dr_clean_single_req(snic, tag, lr_sdev);
- if (ret) {
- SNIC_HOST_ERR(snic->shost, "clean_err:tag = %d\n", tag);
-
- goto clean_err;
- }
+ goto clean_err;
}
schedule_timeout(msecs_to_jiffies(100));
/* Walk through all the cmds and check abts status. */
- if (snic_is_abts_pending(snic, lr_sc)) {
+ if (snic_is_abts_pending(snic, lr_sdev)) {
ret = FAILED;
goto clean_err;
@@ -1950,7 +1950,7 @@ snic_dr_finish(struct snic *snic, struct scsi_cmnd *sc)
* succeeds.
*/
- ret = snic_dr_clean_pending_req(snic, sc);
+ ret = snic_dr_clean_pending_req(snic, sc->device);
if (ret) {
spin_lock_irqsave(io_lock, flags);
SNIC_SCSI_DBG(snic->shost,
@@ -2408,77 +2408,79 @@ snic_cmpl_pending_tmreq(struct snic *snic, struct scsi_cmnd *sc)
complete(rqi->abts_done);
}
-/*
- * snic_scsi_cleanup: Walks through tag map and releases the reqs
- */
-static void
-snic_scsi_cleanup(struct snic *snic)
+static bool
+snic_scsi_cleanup_iter(struct scsi_cmnd *sc, void *data, bool rsvd)
{
+ struct snic *snic = data;
struct snic_req_info *rqi = NULL;
- struct scsi_cmnd *sc = NULL;
spinlock_t *io_lock = NULL;
unsigned long flags;
- int tag;
+ int tag = snic_cmd_tag(sc);
u64 st_time = 0;
- SNIC_SCSI_DBG(snic->shost, "sc_clean: scsi cleanup.\n");
-
- for (tag = 0; tag < snic->max_tag_id; tag++) {
- io_lock = snic_io_lock_tag(snic, tag);
- spin_lock_irqsave(io_lock, flags);
- sc = scsi_host_find_tag(snic->shost, tag);
- if (unlikely(snic_tmreq_pending(sc))) {
- /*
- * When FW Completes reset w/o sending completions
- * for outstanding ios.
- */
- snic_cmpl_pending_tmreq(snic, sc);
- spin_unlock_irqrestore(io_lock, flags);
-
- continue;
- }
-
- rqi = (struct snic_req_info *) CMD_SP(sc);
- if (!rqi) {
- spin_unlock_irqrestore(io_lock, flags);
+ io_lock = snic_io_lock_hash(snic, sc);
+ spin_lock_irqsave(io_lock, flags);
+ if (unlikely(snic_tmreq_pending(sc))) {
+ /*
+ * When FW Completes reset w/o sending completions
+ * for outstanding ios.
+ */
+ snic_cmpl_pending_tmreq(snic, sc);
+ spin_unlock_irqrestore(io_lock, flags);
- goto cleanup;
- }
+ return true;
+ }
+ rqi = (struct snic_req_info *) CMD_SP(sc);
+ if (!rqi) {
+ spin_unlock_irqrestore(io_lock, flags);
+ goto cleanup;
+ }
- SNIC_SCSI_DBG(snic->shost,
- "sc_clean: sc %p, rqi %p, tag %d flags 0x%llx\n",
- sc, rqi, tag, CMD_FLAGS(sc));
+ SNIC_SCSI_DBG(snic->shost,
+ "sc_clean: sc %p, rqi %p, tag %d flags 0x%llx\n",
+ sc, rqi, tag, CMD_FLAGS(sc));
- CMD_SP(sc) = NULL;
- CMD_FLAGS(sc) |= SNIC_SCSI_CLEANUP;
- spin_unlock_irqrestore(io_lock, flags);
- st_time = rqi->start_time;
+ CMD_SP(sc) = NULL;
+ CMD_FLAGS(sc) |= SNIC_SCSI_CLEANUP;
+ spin_unlock_irqrestore(io_lock, flags);
+ st_time = rqi->start_time;
- SNIC_HOST_INFO(snic->shost,
- "sc_clean: Releasing rqi %p : flags 0x%llx\n",
- rqi, CMD_FLAGS(sc));
+ SNIC_HOST_INFO(snic->shost,
+ "sc_clean: Releasing rqi %p : flags 0x%llx\n",
+ rqi, CMD_FLAGS(sc));
- snic_release_req_buf(snic, rqi, sc);
+ snic_release_req_buf(snic, rqi, sc);
cleanup:
- sc->result = DID_TRANSPORT_DISRUPTED << 16;
- SNIC_HOST_INFO(snic->shost,
- "sc_clean: DID_TRANSPORT_DISRUPTED for sc %p, Tag %d flags 0x%llx rqi %p duration %u msecs\n",
- sc, sc->request->tag, CMD_FLAGS(sc), rqi,
- jiffies_to_msecs(jiffies - st_time));
+ sc->result = DID_TRANSPORT_DISRUPTED << 16;
+ SNIC_HOST_INFO(snic->shost,
+ "sc_clean: DID_TRANSPORT_DISRUPTED for sc %p, Tag %d flags 0x%llx rqi %p duration %u msecs\n",
+ sc, tag, CMD_FLAGS(sc), rqi,
+ jiffies_to_msecs(jiffies - st_time));
- /* Update IO stats */
- snic_stats_update_io_cmpl(&snic->s_stats);
+ /* Update IO stats */
+ snic_stats_update_io_cmpl(&snic->s_stats);
- if (sc->scsi_done) {
- SNIC_TRC(snic->shost->host_no, tag, (ulong) sc,
- jiffies_to_msecs(jiffies - st_time), 0,
- SNIC_TRC_CMD(sc),
- SNIC_TRC_CMD_STATE_FLAGS(sc));
+ if (sc->scsi_done) {
+ SNIC_TRC(snic->shost->host_no, tag, (ulong) sc,
+ jiffies_to_msecs(jiffies - st_time), 0,
+ SNIC_TRC_CMD(sc),
+ SNIC_TRC_CMD_STATE_FLAGS(sc));
- sc->scsi_done(sc);
- }
+ sc->scsi_done(sc);
}
+ return true;
+}
+
+/*
+ * snic_scsi_cleanup: Walks through tag map and releases the reqs
+ */
+static void
+snic_scsi_cleanup(struct snic *snic)
+{
+ SNIC_SCSI_DBG(snic->shost, "sc_clean: scsi cleanup.\n");
+
+ scsi_host_busy_iter(snic->shost, snic_scsi_cleanup_iter, snic);
} /* end of snic_scsi_cleanup */
void
@@ -2575,6 +2577,40 @@ snic_internal_abort_io(struct snic *snic, struct scsi_cmnd *sc, int tmf)
return ret;
} /* end of snic_internal_abort_io */
+struct snic_tgt_scsi_abort_io_iter_data {
+ struct snic *snic;
+ struct snic_tgt *tgt;
+ int tmf;
+ int abt_cnt;
+};
+
+static bool
+snic_tgt_scsi_abort_io_iter(struct scsi_cmnd *sc, void *data, bool reserved)
+{
+ struct snic_tgt_scsi_abort_io_iter_data *iter_data = data;
+ struct snic_tgt *sc_tgt = NULL;
+ int ret;
+
+ if (reserved)
+ return true;
+
+ sc_tgt = starget_to_tgt(scsi_target(sc->device));
+ if (sc_tgt != iter_data->tgt)
+ return true;
+
+ ret = snic_internal_abort_io(iter_data->snic, sc, iter_data->tmf);
+ if (ret < 0) {
+ SNIC_HOST_ERR(iter_data->snic->shost,
+ "tgt_abt_io: Tag %x, Failed w err = %d\n",
+ snic_cmd_tag(sc), ret);
+ return true;
+ }
+
+ if (ret == SUCCESS)
+ iter_data->abt_cnt++;
+ return true;
+}
+
/*
* snic_tgt_scsi_abort_io : called by snic_tgt_del
*/
@@ -2582,11 +2618,10 @@ int
snic_tgt_scsi_abort_io(struct snic_tgt *tgt)
{
struct snic *snic = NULL;
- struct scsi_cmnd *sc = NULL;
- struct snic_tgt *sc_tgt = NULL;
- spinlock_t *io_lock = NULL;
- unsigned long flags;
- int ret = 0, tag, abt_cnt = 0, tmf = 0;
+ struct snic_tgt_scsi_abort_io_iter_data iter_data = {
+ .tgt = tgt,
+ .abt_cnt = 0,
+ };
if (!tgt)
return -1;
@@ -2595,43 +2630,16 @@ snic_tgt_scsi_abort_io(struct snic_tgt *tgt)
SNIC_SCSI_DBG(snic->shost, "tgt_abt_io: Cleaning Pending IOs.\n");
if (tgt->tdata.typ == SNIC_TGT_DAS)
- tmf = SNIC_ITMF_ABTS_TASK;
+ iter_data.tmf = SNIC_ITMF_ABTS_TASK;
else
- tmf = SNIC_ITMF_ABTS_TASK_TERM;
-
- for (tag = 0; tag < snic->max_tag_id; tag++) {
- io_lock = snic_io_lock_tag(snic, tag);
-
- spin_lock_irqsave(io_lock, flags);
- sc = scsi_host_find_tag(snic->shost, tag);
- if (!sc) {
- spin_unlock_irqrestore(io_lock, flags);
-
- continue;
- }
+ iter_data.tmf = SNIC_ITMF_ABTS_TASK_TERM;
+ iter_data.snic = snic;
- sc_tgt = starget_to_tgt(scsi_target(sc->device));
- if (sc_tgt != tgt) {
- spin_unlock_irqrestore(io_lock, flags);
-
- continue;
- }
- spin_unlock_irqrestore(io_lock, flags);
-
- ret = snic_internal_abort_io(snic, sc, tmf);
- if (ret < 0) {
- SNIC_HOST_ERR(snic->shost,
- "tgt_abt_io: Tag %x, Failed w err = %d\n",
- tag, ret);
-
- continue;
- }
-
- if (ret == SUCCESS)
- abt_cnt++;
- }
+ scsi_host_busy_iter(snic->shost,
+ snic_tgt_scsi_abort_io_iter, &iter_data);
- SNIC_SCSI_DBG(snic->shost, "tgt_abt_io: abt_cnt = %d\n", abt_cnt);
+ SNIC_SCSI_DBG(snic->shost, "tgt_abt_io: abt_cnt = %d\n",
+ iter_data.abt_cnt);
return 0;
} /* end of snic_tgt_scsi_abort_io */
--
2.29.2
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [PATCH 10/18] scsi: implement reserved command handling
2021-05-03 15:03 [PATCHv8 00/18] scsi: enabled reserved commands for LLDDs Hannes Reinecke
` (8 preceding siblings ...)
2021-05-03 15:03 ` [PATCH 09/18] snic: use tagset iter for traversing commands Hannes Reinecke
@ 2021-05-03 15:03 ` Hannes Reinecke
2021-05-04 3:20 ` Bart Van Assche
2021-05-05 0:45 ` Bart Van Assche
2021-05-03 15:03 ` [PATCH 11/18] hpsa: move hpsa_hba_inquiry after scsi_add_host() Hannes Reinecke
` (7 subsequent siblings)
17 siblings, 2 replies; 55+ messages in thread
From: Hannes Reinecke @ 2021-05-03 15:03 UTC (permalink / raw)
To: Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, John Garry, linux-scsi,
Hannes Reinecke
Quite some drivers are using management commands internally, which
typically use the same hardware tag pool (ie they are being allocated
from the same hardware resources) as the 'normal' I/O commands.
These commands are set aside before allocating the block-mq tag bitmap,
so they'll never show up as busy in the tag map.
The block-layer, OTOH, already has 'reserved_tags' to handle precisely
this situation.
So this patch adds a new field 'nr_reserved_cmds' to the SCSI host
template to instruct the block layer to set aside a tag space for these
management commands by using reserved tags.
Signed-off-by: Hannes Reinecke <hare@suse.de>
Reviewed-by: John Garry <john.garry@huawei.com>
---
drivers/scsi/hosts.c | 3 +++
drivers/scsi/scsi_lib.c | 10 +++++++++-
drivers/scsi/scsi_sysfs.c | 2 ++
include/scsi/scsi_host.h | 22 +++++++++++++++++++++-
4 files changed, 35 insertions(+), 2 deletions(-)
diff --git a/drivers/scsi/hosts.c b/drivers/scsi/hosts.c
index 2f162603876f..661ed7696562 100644
--- a/drivers/scsi/hosts.c
+++ b/drivers/scsi/hosts.c
@@ -469,6 +469,9 @@ struct Scsi_Host *scsi_host_alloc(struct scsi_host_template *sht, int privsize)
if (sht->virt_boundary_mask)
shost->virt_boundary_mask = sht->virt_boundary_mask;
+ if (sht->nr_reserved_cmds)
+ shost->nr_reserved_cmds = sht->nr_reserved_cmds;
+
device_initialize(&shost->shost_gendev);
dev_set_name(&shost->shost_gendev, "host%d", shost->host_no);
shost->shost_gendev.bus = &scsi_bus_type;
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index f83b04e49bae..3c83b0fabefb 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -1971,7 +1971,8 @@ int scsi_mq_setup_tags(struct Scsi_Host *shost)
tag_set->ops = &scsi_mq_ops_no_commit;
tag_set->nr_hw_queues = shost->nr_hw_queues ? : 1;
tag_set->nr_maps = shost->nr_maps ? : 1;
- tag_set->queue_depth = shost->can_queue;
+ tag_set->queue_depth = shost->can_queue + shost->nr_reserved_cmds;
+ tag_set->reserved_tags = shost->nr_reserved_cmds;
tag_set->cmd_size = cmd_size;
tag_set->numa_node = NUMA_NO_NODE;
tag_set->flags = BLK_MQ_F_SHOULD_MERGE;
@@ -1996,6 +1997,9 @@ void scsi_mq_destroy_tags(struct Scsi_Host *shost)
* @flags: BLK_MQ_REQ_* flags, e.g. BLK_MQ_REQ_NOWAIT.
*
* Allocates a SCSI command for internal LLDD use.
+ * If 'nr_reserved_commands' is specified by the host the
+ * command will be allocated from the reserved tag pool;
+ * otherwise the normal tag pool will be used.
*/
struct scsi_cmnd *scsi_get_internal_cmd(struct scsi_device *sdev,
unsigned int op, blk_mq_req_flags_t flags)
@@ -2005,6 +2009,10 @@ struct scsi_cmnd *scsi_get_internal_cmd(struct scsi_device *sdev,
WARN_ON_ONCE(((op & REQ_OP_MASK) != REQ_OP_SCSI_IN) &&
((op & REQ_OP_MASK) != REQ_OP_SCSI_OUT));
+
+ if (sdev->host->nr_reserved_cmds)
+ flags |= BLK_MQ_REQ_RESERVED;
+
rq = blk_mq_alloc_request(sdev->request_queue, op, flags);
if (IS_ERR(rq))
return NULL;
diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index d5260d1b7b38..f4119999a402 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -371,6 +371,7 @@ static DEVICE_ATTR(eh_deadline, S_IRUGO | S_IWUSR, show_shost_eh_deadline, store
shost_rd_attr(unique_id, "%u\n");
shost_rd_attr(cmd_per_lun, "%hd\n");
shost_rd_attr(can_queue, "%d\n");
+shost_rd_attr(nr_reserved_cmds, "%d\n");
shost_rd_attr(sg_tablesize, "%hu\n");
shost_rd_attr(sg_prot_tablesize, "%hu\n");
shost_rd_attr(unchecked_isa_dma, "%d\n");
@@ -422,6 +423,7 @@ static struct attribute *scsi_sysfs_shost_attrs[] = {
&dev_attr_host_reset.attr,
&dev_attr_eh_deadline.attr,
&dev_attr_nr_hw_queues.attr,
+ &dev_attr_nr_reserved_cmds.attr,
NULL
};
diff --git a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h
index f115150559ca..0831b33ee186 100644
--- a/include/scsi/scsi_host.h
+++ b/include/scsi/scsi_host.h
@@ -367,10 +367,19 @@ struct scsi_host_template {
/*
* This determines if we will use a non-interrupt driven
* or an interrupt driven scheme. It is set to the maximum number
- * of simultaneous commands a single hw queue in HBA will accept.
+ * of simultaneous commands a single hw queue in HBA will accept
+ * excluding internal commands.
*/
int can_queue;
+ /*
+ * This determines how many commands the HBA will set aside
+ * for internal commands. This number will be added to
+ * @can_queue to calculate the maximum number of simultaneous
+ * commands sent to the host.
+ */
+ int nr_reserved_cmds;
+
/*
* In many instances, especially where disconnect / reconnect are
* supported, our host also has an ID on the SCSI bus. If this is
@@ -614,6 +623,11 @@ struct Scsi_Host {
unsigned short max_cmd_len;
int this_id;
+
+ /*
+ * Number of commands this host can handle at the same time.
+ * This excludes reserved commands as specified by nr_reserved_cmds.
+ */
int can_queue;
short cmd_per_lun;
short unsigned int sg_tablesize;
@@ -632,6 +646,12 @@ struct Scsi_Host {
*/
unsigned nr_hw_queues;
unsigned nr_maps;
+
+ /*
+ * Number of reserved commands to allocate, if any.
+ */
+ unsigned nr_reserved_cmds;
+
unsigned active_mode:2;
unsigned unchecked_isa_dma:1;
--
2.29.2
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [PATCH 11/18] hpsa: move hpsa_hba_inquiry after scsi_add_host()
2021-05-03 15:03 [PATCHv8 00/18] scsi: enabled reserved commands for LLDDs Hannes Reinecke
` (9 preceding siblings ...)
2021-05-03 15:03 ` [PATCH 10/18] scsi: implement reserved command handling Hannes Reinecke
@ 2021-05-03 15:03 ` Hannes Reinecke
2021-05-03 15:03 ` [PATCH 12/18] hpsa: use reserved commands Hannes Reinecke
` (6 subsequent siblings)
17 siblings, 0 replies; 55+ messages in thread
From: Hannes Reinecke @ 2021-05-03 15:03 UTC (permalink / raw)
To: Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, John Garry, linux-scsi,
Hannes Reinecke, Don Brace
Move hpsa_hba_inquiry to after scsi_add_host() so that the host
is fully initialized.
Signed-off-by: Hannes Reinecke <hare@suse.de>
Acked-by: Don Brace <don.brace@microchip.com>
Tested-by: Don Brace <don.brace@microchip.com>
---
drivers/scsi/hpsa.c | 37 +++++++++++++++++++------------------
1 file changed, 19 insertions(+), 18 deletions(-)
diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
index 38369766511c..c82f218cd1f6 100644
--- a/drivers/scsi/hpsa.c
+++ b/drivers/scsi/hpsa.c
@@ -5874,6 +5874,22 @@ static int hpsa_scsi_host_alloc(struct ctlr_info *h)
return 0;
}
+static void hpsa_hba_inquiry(struct ctlr_info *h)
+{
+ int rc;
+
+#define HBA_INQUIRY_BYTE_COUNT 64
+ h->hba_inquiry_data = kmalloc(HBA_INQUIRY_BYTE_COUNT, GFP_KERNEL);
+ if (!h->hba_inquiry_data)
+ return;
+ rc = hpsa_scsi_do_inquiry(h, RAID_CTLR_LUNID, 0,
+ h->hba_inquiry_data, HBA_INQUIRY_BYTE_COUNT);
+ if (rc != 0) {
+ kfree(h->hba_inquiry_data);
+ h->hba_inquiry_data = NULL;
+ }
+}
+
static int hpsa_scsi_add_host(struct ctlr_info *h)
{
int rv;
@@ -5883,6 +5899,9 @@ static int hpsa_scsi_add_host(struct ctlr_info *h)
dev_err(&h->pdev->dev, "scsi_add_host failed\n");
return rv;
}
+
+ hpsa_hba_inquiry(h);
+
scsi_scan_host(h->scsi_host);
return 0;
}
@@ -7949,22 +7968,6 @@ static int hpsa_pci_init(struct ctlr_info *h)
return err;
}
-static void hpsa_hba_inquiry(struct ctlr_info *h)
-{
- int rc;
-
-#define HBA_INQUIRY_BYTE_COUNT 64
- h->hba_inquiry_data = kmalloc(HBA_INQUIRY_BYTE_COUNT, GFP_KERNEL);
- if (!h->hba_inquiry_data)
- return;
- rc = hpsa_scsi_do_inquiry(h, RAID_CTLR_LUNID, 0,
- h->hba_inquiry_data, HBA_INQUIRY_BYTE_COUNT);
- if (rc != 0) {
- kfree(h->hba_inquiry_data);
- h->hba_inquiry_data = NULL;
- }
-}
-
static int hpsa_init_reset_devices(struct pci_dev *pdev, u32 board_id)
{
int rc, i;
@@ -8869,8 +8872,6 @@ static int hpsa_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
/* Turn the interrupts on so we can service requests */
h->access.set_intr_mask(h, HPSA_INTR_ON);
- hpsa_hba_inquiry(h);
-
h->lastlogicals = kzalloc(sizeof(*(h->lastlogicals)), GFP_KERNEL);
if (!h->lastlogicals)
dev_info(&h->pdev->dev,
--
2.29.2
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [PATCH 12/18] hpsa: use reserved commands
2021-05-03 15:03 [PATCHv8 00/18] scsi: enabled reserved commands for LLDDs Hannes Reinecke
` (10 preceding siblings ...)
2021-05-03 15:03 ` [PATCH 11/18] hpsa: move hpsa_hba_inquiry after scsi_add_host() Hannes Reinecke
@ 2021-05-03 15:03 ` Hannes Reinecke
2021-05-03 15:03 ` [PATCH 13/18] hpsa: use scsi_host_busy_iter() to traverse outstanding commands Hannes Reinecke
` (5 subsequent siblings)
17 siblings, 0 replies; 55+ messages in thread
From: Hannes Reinecke @ 2021-05-03 15:03 UTC (permalink / raw)
To: Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, John Garry, linux-scsi,
Hannes Reinecke, Don Brace
Enable the use of reserved commands, and drop the hand-crafted
command allocation.
Signed-off-by: Hannes Reinecke <hare@suse.de>
Acked-by: Don Brace <don.brace@microchip.com>
Tested-by: Don Brace <don.brace@microchip.com>
---
drivers/scsi/hpsa.c | 202 ++++++++++++++++----------------------------
drivers/scsi/hpsa.h | 3 +-
2 files changed, 76 insertions(+), 129 deletions(-)
diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
index c82f218cd1f6..7a4fcd40364b 100644
--- a/drivers/scsi/hpsa.c
+++ b/drivers/scsi/hpsa.c
@@ -245,10 +245,6 @@ static struct hpsa_scsi_dev_t
*hpsa_find_device_by_sas_rphy(struct ctlr_info *h,
struct sas_rphy *rphy);
-#define SCSI_CMD_BUSY ((struct scsi_cmnd *)&hpsa_cmd_busy)
-static const struct scsi_cmnd hpsa_cmd_busy;
-#define SCSI_CMD_IDLE ((struct scsi_cmnd *)&hpsa_cmd_idle)
-static const struct scsi_cmnd hpsa_cmd_idle;
static int number_of_controllers;
static irqreturn_t do_hpsa_intr_intx(int irq, void *dev_id);
@@ -266,7 +262,7 @@ static int hpsa_compat_ioctl(struct scsi_device *dev, unsigned int cmd,
#endif
static void cmd_free(struct ctlr_info *h, struct CommandList *c);
-static struct CommandList *cmd_alloc(struct ctlr_info *h);
+static struct CommandList *cmd_alloc(struct ctlr_info *h, u8 direction);
static void cmd_tagged_free(struct ctlr_info *h, struct CommandList *c);
static struct CommandList *cmd_tagged_alloc(struct ctlr_info *h,
struct scsi_cmnd *scmd);
@@ -347,7 +343,7 @@ static inline struct ctlr_info *shost_to_hba(struct Scsi_Host *sh)
static inline bool hpsa_is_cmd_idle(struct CommandList *c)
{
- return c->scsi_cmd == SCSI_CMD_IDLE;
+ return c->scsi_cmd == NULL;
}
/* extract sense key, asc, and ascq from sense data. -1 means invalid. */
@@ -2462,7 +2458,12 @@ static void hpsa_cmd_resolve_events(struct ctlr_info *h,
* this command has completed. Then, check to see if the handler is
* waiting for this command, and, if so, wake it.
*/
- c->scsi_cmd = SCSI_CMD_IDLE;
+ if (c->scsi_cmd && c->cmd_type == CMD_IOCTL_PEND) {
+ struct scsi_cmnd *scmd = c->scsi_cmd;
+
+ scsi_put_internal_cmd(scmd);
+ }
+ c->scsi_cmd = NULL;
mb(); /* Declare command idle before checking for pending events. */
if (dev) {
atomic_dec(&dev->commands_outstanding);
@@ -3004,7 +3005,7 @@ static int hpsa_do_receive_diagnostic(struct ctlr_info *h, u8 *scsi3addr,
struct CommandList *c;
struct ErrorInfo *ei;
- c = cmd_alloc(h);
+ c = cmd_alloc(h, XFER_READ);
if (fill_cmd(c, RECEIVE_DIAGNOSTIC, h, buf, bufsize,
page, scsi3addr, TYPE_CMD)) {
rc = -1;
@@ -3056,7 +3057,7 @@ static int hpsa_scsi_do_inquiry(struct ctlr_info *h, unsigned char *scsi3addr,
struct CommandList *c;
struct ErrorInfo *ei;
- c = cmd_alloc(h);
+ c = cmd_alloc(h, XFER_READ);
if (fill_cmd(c, HPSA_INQUIRY, h, buf, bufsize,
page, scsi3addr, TYPE_CMD)) {
@@ -3084,7 +3085,7 @@ static int hpsa_send_reset(struct ctlr_info *h, struct hpsa_scsi_dev_t *dev,
struct CommandList *c;
struct ErrorInfo *ei;
- c = cmd_alloc(h);
+ c = cmd_alloc(h, XFER_NONE);
c->device = dev;
/* fill_cmd can't fail here, no data buffer to map. */
@@ -3310,7 +3311,7 @@ static int hpsa_get_raid_map(struct ctlr_info *h,
struct CommandList *c;
struct ErrorInfo *ei;
- c = cmd_alloc(h);
+ c = cmd_alloc(h, XFER_READ);
if (fill_cmd(c, HPSA_GET_RAID_MAP, h, &this_device->raid_map,
sizeof(this_device->raid_map), 0,
@@ -3352,7 +3353,7 @@ static int hpsa_bmic_sense_subsystem_information(struct ctlr_info *h,
struct CommandList *c;
struct ErrorInfo *ei;
- c = cmd_alloc(h);
+ c = cmd_alloc(h, XFER_READ);
rc = fill_cmd(c, BMIC_SENSE_SUBSYSTEM_INFORMATION, h, buf, bufsize,
0, RAID_CTLR_LUNID, TYPE_CMD);
@@ -3383,7 +3384,7 @@ static int hpsa_bmic_id_controller(struct ctlr_info *h,
struct CommandList *c;
struct ErrorInfo *ei;
- c = cmd_alloc(h);
+ c = cmd_alloc(h, XFER_READ);
rc = fill_cmd(c, BMIC_IDENTIFY_CONTROLLER, h, buf, bufsize,
0, RAID_CTLR_LUNID, TYPE_CMD);
@@ -3412,7 +3413,7 @@ static int hpsa_bmic_id_physical_device(struct ctlr_info *h,
struct CommandList *c;
struct ErrorInfo *ei;
- c = cmd_alloc(h);
+ c = cmd_alloc(h, XFER_READ);
rc = fill_cmd(c, BMIC_IDENTIFY_PHYSICAL_DEVICE, h, buf, bufsize,
0, RAID_CTLR_LUNID, TYPE_CMD);
if (rc)
@@ -3489,7 +3490,7 @@ static void hpsa_get_enclosure_info(struct ctlr_info *h,
goto out;
}
- c = cmd_alloc(h);
+ c = cmd_alloc(h, XFER_READ);
rc = fill_cmd(c, BMIC_SENSE_STORAGE_BOX_PARAMS, h, bssbp,
sizeof(*bssbp), 0, RAID_CTLR_LUNID, TYPE_CMD);
@@ -3745,7 +3746,7 @@ static int hpsa_scsi_do_report_luns(struct ctlr_info *h, int logical,
unsigned char scsi3addr[8];
struct ErrorInfo *ei;
- c = cmd_alloc(h);
+ c = cmd_alloc(h, XFER_READ);
/* address the controller */
memset(scsi3addr, 0, sizeof(scsi3addr));
@@ -3886,7 +3887,7 @@ static unsigned char hpsa_volume_offline(struct ctlr_info *h,
#define ASCQ_LUN_NOT_READY_FORMAT_IN_PROGRESS 0x04
#define ASCQ_LUN_NOT_READY_INITIALIZING_CMD_REQ 0x02
- c = cmd_alloc(h);
+ c = cmd_alloc(h, XFER_NONE);
(void) fill_cmd(c, TEST_UNIT_READY, h, NULL, 0, 0, scsi3addr, TYPE_CMD);
rc = hpsa_scsi_do_simple_cmd(h, c, DEFAULT_REPLY_QUEUE,
@@ -5542,7 +5543,6 @@ static void hpsa_cmd_init(struct ctlr_info *h, int index,
c->ErrDesc.Addr = cpu_to_le64((u64) err_dma_handle);
c->ErrDesc.Len = cpu_to_le32((u32) sizeof(*c->err_info));
c->h = h;
- c->scsi_cmd = SCSI_CMD_IDLE;
}
static void hpsa_preinitialize_commands(struct ctlr_info *h)
@@ -5857,12 +5857,12 @@ static int hpsa_scsi_host_alloc(struct ctlr_info *h)
sh->io_port = 0;
sh->n_io_port = 0;
- sh->this_id = -1;
sh->max_channel = 3;
sh->max_cmd_len = MAX_COMMAND_SIZE;
sh->max_lun = HPSA_MAX_LUN;
sh->max_id = HPSA_MAX_LUN;
sh->can_queue = h->nr_cmds - HPSA_NRESERVED_CMDS;
+ sh->nr_reserved_cmds = HPSA_NRESERVED_CMDS;
sh->cmd_per_lun = sh->can_queue;
sh->sg_tablesize = h->maxsgentries;
sh->transportt = hpsa_sas_transport_template;
@@ -5899,30 +5899,18 @@ static int hpsa_scsi_add_host(struct ctlr_info *h)
dev_err(&h->pdev->dev, "scsi_add_host failed\n");
return rv;
}
-
+ h->raid_ctrl_sdev = scsi_get_host_dev(h->scsi_host);
+ if (!h->raid_ctrl_sdev) {
+ dev_err(&h->pdev->dev,
+ "allocate raid controller device failed\n");
+ return -ENOMEM;
+ }
hpsa_hba_inquiry(h);
scsi_scan_host(h->scsi_host);
return 0;
}
-/*
- * The block layer has already gone to the trouble of picking out a unique,
- * small-integer tag for this request. We use an offset from that value as
- * an index to select our command block. (The offset allows us to reserve the
- * low-numbered entries for our own uses.)
- */
-static int hpsa_get_cmd_index(struct scsi_cmnd *scmd)
-{
- int idx = scmd->request->tag;
-
- if (idx < 0)
- return idx;
-
- /* Offset to leave space for internal cmds. */
- return idx += HPSA_NRESERVED_CMDS;
-}
-
/*
* Send a TEST_UNIT_READY command to the specified LUN using the specified
* reply queue; returns zero if the unit is ready, and non-zero otherwise.
@@ -6006,7 +5994,7 @@ static int wait_for_device_to_become_ready(struct ctlr_info *h,
int rc = 0;
struct CommandList *c;
- c = cmd_alloc(h);
+ c = cmd_alloc(h, XFER_NONE);
/*
* If no specific reply queue was requested, then send the TUR
@@ -6079,7 +6067,7 @@ static int hpsa_eh_device_reset_handler(struct scsi_cmnd *scsicmd)
if (lockup_detected(h)) {
snprintf(msg, sizeof(msg),
"cmd %d RESET FAILED, lockup detected",
- hpsa_get_cmd_index(scsicmd));
+ scsicmd->request->tag);
hpsa_show_dev_msg(KERN_WARNING, h, dev, msg);
rc = FAILED;
goto return_reset_status;
@@ -6089,7 +6077,7 @@ static int hpsa_eh_device_reset_handler(struct scsi_cmnd *scsicmd)
if (detect_controller_lockup(h)) {
snprintf(msg, sizeof(msg),
"cmd %d RESET FAILED, new lockup detected",
- hpsa_get_cmd_index(scsicmd));
+ scsicmd->request->tag);
hpsa_show_dev_msg(KERN_WARNING, h, dev, msg);
rc = FAILED;
goto return_reset_status;
@@ -6152,12 +6140,12 @@ static int hpsa_eh_device_reset_handler(struct scsi_cmnd *scsicmd)
static struct CommandList *cmd_tagged_alloc(struct ctlr_info *h,
struct scsi_cmnd *scmd)
{
- int idx = hpsa_get_cmd_index(scmd);
+ int idx = scmd->request->tag;
struct CommandList *c = h->cmd_pool + idx;
- if (idx < HPSA_NRESERVED_CMDS || idx >= h->nr_cmds) {
+ if (idx < 0 || idx >= h->nr_cmds) {
dev_err(&h->pdev->dev, "Bad block tag: %d not in [%d..%d]\n",
- idx, HPSA_NRESERVED_CMDS, h->nr_cmds - 1);
+ idx, 0, h->nr_cmds - 1);
/* The index value comes from the block layer, so if it's out of
* bounds, it's probably not our bug.
*/
@@ -6200,62 +6188,32 @@ static void cmd_tagged_free(struct ctlr_info *h, struct CommandList *c)
* else to free it, because it is accessed by index.
*/
(void)atomic_dec(&c->refcount);
+ c->scsi_cmd = NULL;
}
-/*
- * For operations that cannot sleep, a command block is allocated at init,
- * and managed by cmd_alloc() and cmd_free() using a simple bitmap to track
- * which ones are free or in use. Lock must be held when calling this.
- * cmd_free() is the complement.
- * This function never gives up and returns NULL. If it hangs,
- * another thread must call cmd_free() to free some tags.
- */
-
-static struct CommandList *cmd_alloc(struct ctlr_info *h)
+static struct CommandList *cmd_alloc(struct ctlr_info *h, u8 direction)
{
+ struct scsi_cmnd *scmd;
struct CommandList *c;
- int refcount, i;
- int offset = 0;
+ int idx;
- /*
- * There is some *extremely* small but non-zero chance that that
- * multiple threads could get in here, and one thread could
- * be scanning through the list of bits looking for a free
- * one, but the free ones are always behind him, and other
- * threads sneak in behind him and eat them before he can
- * get to them, so that while there is always a free one, a
- * very unlucky thread might be starved anyway, never able to
- * beat the other threads. In reality, this happens so
- * infrequently as to be indistinguishable from never.
- *
- * Note that we start allocating commands before the SCSI host structure
- * is initialized. Since the search starts at bit zero, this
- * all works, since we have at least one command structure available;
- * however, it means that the structures with the low indexes have to be
- * reserved for driver-initiated requests, while requests from the block
- * layer will use the higher indexes.
- */
-
- for (;;) {
- i = find_next_zero_bit(h->cmd_pool_bits,
- HPSA_NRESERVED_CMDS,
- offset);
- if (unlikely(i >= HPSA_NRESERVED_CMDS)) {
- offset = 0;
- continue;
- }
- c = h->cmd_pool + i;
- refcount = atomic_inc_return(&c->refcount);
- if (unlikely(refcount > 1)) {
- cmd_free(h, c); /* already in use */
- offset = (i + 1) % HPSA_NRESERVED_CMDS;
- continue;
- }
- set_bit(i & (BITS_PER_LONG - 1),
- h->cmd_pool_bits + (i / BITS_PER_LONG));
- break; /* it's ours now. */
+ scmd = scsi_get_internal_cmd(h->raid_ctrl_sdev,
+ (direction & XFER_WRITE) ?
+ REQ_OP_SCSI_OUT : REQ_OP_SCSI_IN, 0);
+ if (!scmd) {
+ dev_warn(&h->pdev->dev, "failed to allocate reserved cmd\n");
+ return NULL;
}
- hpsa_cmd_partial_init(h, i, c);
+ idx = scmd->request->tag;
+ c = cmd_tagged_alloc(h, scmd);
+ if (!c) {
+ dev_warn(&h->pdev->dev, "failed to allocate reserved cmd %u\n",
+ idx);
+ scsi_put_internal_cmd(scmd);
+ return NULL;
+ }
+ hpsa_cmd_partial_init(h, idx, c);
+ c->scsi_cmd = scmd;
c->device = NULL;
/*
@@ -6263,24 +6221,24 @@ static struct CommandList *cmd_alloc(struct ctlr_info *h)
* retried.
*/
c->retry_pending = false;
-
+ c->cmd_type = CMD_IOCTL_PEND;
+ dev_dbg(&h->pdev->dev, "using reserved cmd %u\n", idx);
return c;
}
-/*
- * This is the complementary operation to cmd_alloc(). Note, however, in some
- * corner cases it may also be used to free blocks allocated by
- * cmd_tagged_alloc() in which case the ref-count decrement does the trick and
- * the clear-bit is harmless.
- */
static void cmd_free(struct ctlr_info *h, struct CommandList *c)
{
- if (atomic_dec_and_test(&c->refcount)) {
- int i;
+ struct scsi_cmnd *scmd = c->scsi_cmd;
- i = c - h->cmd_pool;
- clear_bit(i & (BITS_PER_LONG - 1),
- h->cmd_pool_bits + (i / BITS_PER_LONG));
+ if (!scmd) {
+ dev_warn(&h->pdev->dev, "freeing idle cmd\n");
+ return;
+ }
+ cmd_tagged_free(h, c);
+ if (c->cmd_type == CMD_IOCTL_PEND) {
+ dev_dbg(&h->pdev->dev, "returning reserved cmd %u\n",
+ scmd->request->tag);
+ scsi_put_internal_cmd(scmd);
}
}
@@ -6447,11 +6405,8 @@ static int hpsa_passthru_ioctl(struct ctlr_info *h,
memset(buff, 0, iocommand->buf_size);
}
}
- c = cmd_alloc(h);
+ c = cmd_alloc(h, iocommand->Request.Type.Direction);
- /* Fill in the command type */
- c->cmd_type = CMD_IOCTL_PEND;
- c->scsi_cmd = SCSI_CMD_BUSY;
/* Fill in Command Header */
c->Header.ReplyQueue = 0; /* unused in simple mode */
if (iocommand->buf_size > 0) { /* buffer to fill */
@@ -6564,10 +6519,8 @@ static int hpsa_big_passthru_ioctl(struct ctlr_info *h,
data_ptr += sz;
sg_used++;
}
- c = cmd_alloc(h);
+ c = cmd_alloc(h, ioc->Request.Type.Direction);
- c->cmd_type = CMD_IOCTL_PEND;
- c->scsi_cmd = SCSI_CMD_BUSY;
c->Header.ReplyQueue = 0;
c->Header.SGList = (u8) sg_used;
c->Header.SGTotal = cpu_to_le16(sg_used);
@@ -6698,7 +6651,7 @@ static void hpsa_send_host_reset(struct ctlr_info *h, u8 reset_type)
{
struct CommandList *c;
- c = cmd_alloc(h);
+ c = cmd_alloc(h, XFER_NONE);
/* fill_cmd can't fail here, no data buffer to map */
(void) fill_cmd(c, HPSA_DEVICE_RESET_MSG, h, NULL, 0, 0,
@@ -6719,8 +6672,6 @@ static int fill_cmd(struct CommandList *c, u8 cmd, struct ctlr_info *h,
{
enum dma_data_direction dir = DMA_NONE;
- c->cmd_type = CMD_IOCTL_PEND;
- c->scsi_cmd = SCSI_CMD_BUSY;
c->Header.ReplyQueue = 0;
if (buff != NULL && size > 0) {
c->Header.SGList = 1;
@@ -8032,8 +7983,6 @@ static int hpsa_init_reset_devices(struct pci_dev *pdev, u32 board_id)
static void hpsa_free_cmd_pool(struct ctlr_info *h)
{
- kfree(h->cmd_pool_bits);
- h->cmd_pool_bits = NULL;
if (h->cmd_pool) {
dma_free_coherent(&h->pdev->dev,
h->nr_cmds * sizeof(struct CommandList),
@@ -8054,17 +8003,13 @@ static void hpsa_free_cmd_pool(struct ctlr_info *h)
static int hpsa_alloc_cmd_pool(struct ctlr_info *h)
{
- h->cmd_pool_bits = kcalloc(DIV_ROUND_UP(h->nr_cmds, BITS_PER_LONG),
- sizeof(unsigned long),
- GFP_KERNEL);
h->cmd_pool = dma_alloc_coherent(&h->pdev->dev,
h->nr_cmds * sizeof(*h->cmd_pool),
&h->cmd_pool_dhandle, GFP_KERNEL);
h->errinfo_pool = dma_alloc_coherent(&h->pdev->dev,
h->nr_cmds * sizeof(*h->errinfo_pool),
&h->errinfo_pool_dhandle, GFP_KERNEL);
- if ((h->cmd_pool_bits == NULL)
- || (h->cmd_pool == NULL)
+ if ((h->cmd_pool == NULL)
|| (h->errinfo_pool == NULL)) {
dev_err(&h->pdev->dev, "out of memory in %s", __func__);
goto clean_up;
@@ -8945,7 +8890,7 @@ static void hpsa_flush_cache(struct ctlr_info *h)
if (!flush_buf)
return;
- c = cmd_alloc(h);
+ c = cmd_alloc(h, XFER_NONE);
if (fill_cmd(c, HPSA_CACHE_FLUSH, h, flush_buf, 4, 0,
RAID_CTLR_LUNID, TYPE_CMD)) {
@@ -8980,7 +8925,7 @@ static void hpsa_disable_rld_caching(struct ctlr_info *h)
if (!options)
return;
- c = cmd_alloc(h);
+ c = cmd_alloc(h, XFER_READ);
/* first, get the current diag options settings */
if (fill_cmd(c, BMIC_SENSE_DIAG_OPTIONS, h, options, 4, 0,
@@ -9030,11 +8975,10 @@ static void __hpsa_shutdown(struct pci_dev *pdev)
struct ctlr_info *h;
h = pci_get_drvdata(pdev);
- /* Turn board interrupts off and send the flush cache command
- * sendcmd will turn off interrupt, and send the flush...
- * To write all data in the battery backed cache to disks
+ /*
+ * Turn board interrupts off;
+ * flush cache command has already been sent.
*/
- hpsa_flush_cache(h);
h->access.set_intr_mask(h, HPSA_INTR_OFF);
hpsa_free_irqs(h); /* init_one 4 */
hpsa_disable_interrupt_mode(h); /* pci_init 2 */
@@ -9042,6 +8986,7 @@ static void __hpsa_shutdown(struct pci_dev *pdev)
static void hpsa_shutdown(struct pci_dev *pdev)
{
+ hpsa_flush_cache(pci_get_drvdata(pdev));
__hpsa_shutdown(pdev);
pci_disable_device(pdev);
}
@@ -9086,6 +9031,7 @@ static void hpsa_remove_one(struct pci_dev *pdev)
* when multipath is enabled. There can be SYNCHRONIZE CACHE
* operations which cannot complete and will hang the system.
*/
+ hpsa_flush_cache(h);
if (h->scsi_host)
scsi_remove_host(h->scsi_host); /* init_one 8 */
/* includes hpsa_free_irqs - init_one 4 */
diff --git a/drivers/scsi/hpsa.h b/drivers/scsi/hpsa.h
index 99b0750850b2..93e9fd9b931c 100644
--- a/drivers/scsi/hpsa.h
+++ b/drivers/scsi/hpsa.h
@@ -206,7 +206,6 @@ struct ctlr_info {
dma_addr_t ioaccel2_cmd_pool_dhandle;
struct ErrorInfo *errinfo_pool;
dma_addr_t errinfo_pool_dhandle;
- unsigned long *cmd_pool_bits;
int scan_finished;
u8 scan_waiting : 1;
spinlock_t scan_lock;
@@ -216,6 +215,8 @@ struct ctlr_info {
spinlock_t devlock; /* to protect hba[ctlr]->dev[]; */
int ndevices; /* number of used elements in .dev[] array. */
struct hpsa_scsi_dev_t *dev[HPSA_MAX_DEVICES];
+ struct scsi_device *raid_ctrl_sdev;
+
/*
* Performant mode tables.
*/
--
2.29.2
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [PATCH 13/18] hpsa: use scsi_host_busy_iter() to traverse outstanding commands
2021-05-03 15:03 [PATCHv8 00/18] scsi: enabled reserved commands for LLDDs Hannes Reinecke
` (11 preceding siblings ...)
2021-05-03 15:03 ` [PATCH 12/18] hpsa: use reserved commands Hannes Reinecke
@ 2021-05-03 15:03 ` Hannes Reinecke
2021-05-03 15:03 ` [PATCH 14/18] hpsa: drop refcount field from CommandList Hannes Reinecke
` (4 subsequent siblings)
17 siblings, 0 replies; 55+ messages in thread
From: Hannes Reinecke @ 2021-05-03 15:03 UTC (permalink / raw)
To: Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, John Garry, linux-scsi,
Hannes Reinecke, Don Brace
Replace all hand-crafted command iterations with
scsi_host_busy_iter() calls.
Signed-off-by: Hannes Reinecke <hare@suse.de>
Acked-by: Don Brace <don.brace@microchip.com>
Tested-by: Don Brace <don.brace@microchip.com>
---
drivers/scsi/hpsa.c | 117 +++++++++++++++++++++++++-------------------
1 file changed, 67 insertions(+), 50 deletions(-)
diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
index 7a4fcd40364b..282338944a85 100644
--- a/drivers/scsi/hpsa.c
+++ b/drivers/scsi/hpsa.c
@@ -1820,30 +1820,26 @@ static int hpsa_add_device(struct ctlr_info *h, struct hpsa_scsi_dev_t *device)
return rc;
}
-static int hpsa_find_outstanding_commands_for_dev(struct ctlr_info *h,
- struct hpsa_scsi_dev_t *dev)
-{
- int i;
- int count = 0;
-
- for (i = 0; i < h->nr_cmds; i++) {
- struct CommandList *c = h->cmd_pool + i;
- int refcount = atomic_inc_return(&c->refcount);
-
- if (refcount > 1 && hpsa_cmd_dev_match(h, c, dev,
- dev->scsi3addr)) {
- unsigned long flags;
+struct hpsa_command_iter_data {
+ struct ctlr_info *h;
+ struct hpsa_scsi_dev_t *dev;
+ unsigned char *scsi3addr;
+ int count;
+};
- spin_lock_irqsave(&h->lock, flags); /* Implied MB */
- if (!hpsa_is_cmd_idle(c))
- ++count;
- spin_unlock_irqrestore(&h->lock, flags);
- }
+static bool hpsa_find_outstanding_commands_iter(struct scsi_cmnd *sc,
+ void *data, bool reserved)
+{
+ struct hpsa_command_iter_data *iter_data = data;
+ struct ctlr_info *h = iter_data->h;
+ struct hpsa_scsi_dev_t *dev = iter_data->dev;
+ struct CommandList *c = h->cmd_pool + sc->request->tag;
- cmd_free(h, c);
+ if (hpsa_cmd_dev_match(h, c, dev, dev->scsi3addr)) {
+ iter_data->count++;
+ return false;
}
-
- return count;
+ return true;
}
#define NUM_WAIT 20
@@ -1853,13 +1849,20 @@ static void hpsa_wait_for_outstanding_commands_for_dev(struct ctlr_info *h,
int cmds = 0;
int waits = 0;
int num_wait = NUM_WAIT;
+ struct hpsa_command_iter_data iter_data = {
+ .h = h,
+ .dev = device,
+ };
if (device->external)
num_wait = HPSA_EH_PTRAID_TIMEOUT;
while (1) {
- cmds = hpsa_find_outstanding_commands_for_dev(h, device);
- if (cmds == 0)
+ iter_data.count = 0;
+ scsi_host_busy_iter(h->scsi_host,
+ hpsa_find_outstanding_commands_iter,
+ &iter_data);
+ if (iter_data.count == 0)
break;
if (++waits > num_wait)
break;
@@ -8180,27 +8183,34 @@ static void hpsa_undo_allocations_after_kdump_soft_reset(struct ctlr_info *h)
kfree(h); /* init_one 1 */
}
+static bool fail_all_outstanding_cmds_iter(struct scsi_cmnd *sc, void *data,
+ bool reserved)
+{
+ struct hpsa_command_iter_data *iter_data = data;
+ struct ctlr_info *h = iter_data->h;
+ struct CommandList *c = h->cmd_pool + sc->request->tag;
+
+ c->err_info->CommandStatus = CMD_CTLR_LOCKUP;
+ finish_cmd(c);
+ atomic_dec(&h->commands_outstanding);
+ iter_data->count++;
+
+ return true;
+}
+
/* Called when controller lockup detected. */
static void fail_all_outstanding_cmds(struct ctlr_info *h)
{
- int i, refcount;
- struct CommandList *c;
- int failcount = 0;
+ struct hpsa_command_iter_data iter_data = {
+ .h = h,
+ .count = 0,
+ };
flush_workqueue(h->resubmit_wq); /* ensure all cmds are fully built */
- for (i = 0; i < h->nr_cmds; i++) {
- c = h->cmd_pool + i;
- refcount = atomic_inc_return(&c->refcount);
- if (refcount > 1) {
- c->err_info->CommandStatus = CMD_CTLR_LOCKUP;
- finish_cmd(c);
- atomic_dec(&h->commands_outstanding);
- failcount++;
- }
- cmd_free(h, c);
- }
+ scsi_host_busy_iter(h->scsi_host,
+ fail_all_outstanding_cmds_iter, &iter_data);
dev_warn(&h->pdev->dev,
- "failed %d commands in fail_all\n", failcount);
+ "failed %d commands in fail_all\n", iter_data.count);
}
static void set_lockup_detected_for_all_cpus(struct ctlr_info *h, u32 value)
@@ -9499,22 +9509,29 @@ static int is_accelerated_cmd(struct CommandList *c)
return c->cmd_type == CMD_IOACCEL1 || c->cmd_type == CMD_IOACCEL2;
}
+static bool hpsa_drain_accel_commands_iter(struct scsi_cmnd *sc, void *data,
+ bool reserved)
+{
+ struct hpsa_command_iter_data *iter_data = data;
+ struct ctlr_info *h = iter_data->h;
+ struct CommandList *c = h->cmd_pool + sc->request->tag;
+
+ iter_data->count += is_accelerated_cmd(c);
+ return true;
+}
+
static void hpsa_drain_accel_commands(struct ctlr_info *h)
{
- struct CommandList *c = NULL;
- int i, accel_cmds_out;
- int refcount;
+ struct hpsa_command_iter_data iter_data = {
+ .h = h,
+ };
do { /* wait for all outstanding ioaccel commands to drain out */
- accel_cmds_out = 0;
- for (i = 0; i < h->nr_cmds; i++) {
- c = h->cmd_pool + i;
- refcount = atomic_inc_return(&c->refcount);
- if (refcount > 1) /* Command is allocated */
- accel_cmds_out += is_accelerated_cmd(c);
- cmd_free(h, c);
- }
- if (accel_cmds_out <= 0)
+ iter_data.count = 0;
+ scsi_host_busy_iter(h->scsi_host,
+ hpsa_drain_accel_commands_iter,
+ &iter_data);
+ if (iter_data.count <= 0)
break;
msleep(100);
} while (1);
--
2.29.2
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [PATCH 14/18] hpsa: drop refcount field from CommandList
2021-05-03 15:03 [PATCHv8 00/18] scsi: enabled reserved commands for LLDDs Hannes Reinecke
` (12 preceding siblings ...)
2021-05-03 15:03 ` [PATCH 13/18] hpsa: use scsi_host_busy_iter() to traverse outstanding commands Hannes Reinecke
@ 2021-05-03 15:03 ` Hannes Reinecke
2021-05-03 15:03 ` [PATCH 15/18] aacraid: move scsi_add_host() Hannes Reinecke
` (3 subsequent siblings)
17 siblings, 0 replies; 55+ messages in thread
From: Hannes Reinecke @ 2021-05-03 15:03 UTC (permalink / raw)
To: Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, John Garry, linux-scsi,
Hannes Reinecke, Hannes Reinecke, Don Brace
Field is now unused, so drop it.
Signed-off-by: Hannes Reinecke <hare@suse.com>
Acked-by: Don Brace <don.brace@microchip.com>
Tested-by: Don Brace <don.brace@microchip.com>
---
drivers/scsi/hpsa.c | 11 ++---------
drivers/scsi/hpsa_cmd.h | 10 ----------
2 files changed, 2 insertions(+), 19 deletions(-)
diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
index 282338944a85..61f993704e23 100644
--- a/drivers/scsi/hpsa.c
+++ b/drivers/scsi/hpsa.c
@@ -5533,8 +5533,8 @@ static void hpsa_cmd_init(struct ctlr_info *h, int index,
{
dma_addr_t cmd_dma_handle, err_dma_handle;
- /* Zero out all of commandlist except the last field, refcount */
- memset(c, 0, offsetof(struct CommandList, refcount));
+ /* Zero out all of commandlist */
+ memset(c, 0, sizeof(struct CommandList));
c->Header.tag = cpu_to_le64((u64) (index << DIRECT_LOOKUP_SHIFT));
cmd_dma_handle = h->cmd_pool_dhandle + index * sizeof(*c);
c->err_info = h->errinfo_pool + index;
@@ -5556,7 +5556,6 @@ static void hpsa_preinitialize_commands(struct ctlr_info *h)
struct CommandList *c = h->cmd_pool + i;
hpsa_cmd_init(h, i, c);
- atomic_set(&c->refcount, 0);
}
}
@@ -6172,7 +6171,6 @@ static struct CommandList *cmd_tagged_alloc(struct ctlr_info *h,
return NULL;
}
- atomic_inc(&c->refcount);
hpsa_cmd_partial_init(h, idx, c);
/*
@@ -6186,11 +6184,6 @@ static struct CommandList *cmd_tagged_alloc(struct ctlr_info *h,
static void cmd_tagged_free(struct ctlr_info *h, struct CommandList *c)
{
- /*
- * Release our reference to the block. We don't need to do anything
- * else to free it, because it is accessed by index.
- */
- (void)atomic_dec(&c->refcount);
c->scsi_cmd = NULL;
}
diff --git a/drivers/scsi/hpsa_cmd.h b/drivers/scsi/hpsa_cmd.h
index ba6a3aa8d954..04c92c94cc6c 100644
--- a/drivers/scsi/hpsa_cmd.h
+++ b/drivers/scsi/hpsa_cmd.h
@@ -454,18 +454,8 @@ struct CommandList {
bool retry_pending;
struct hpsa_scsi_dev_t *device;
- atomic_t refcount; /* Must be last to avoid memset in hpsa_cmd_init() */
} __aligned(COMMANDLIST_ALIGNMENT);
-/*
- * Make sure our embedded atomic variable is aligned. Otherwise we break atomic
- * operations on architectures that don't support unaligned atomics like IA64.
- *
- * The assert guards against reintroductin against unwanted __packed to
- * the struct CommandList.
- */
-static_assert(offsetof(struct CommandList, refcount) % __alignof__(atomic_t) == 0);
-
/* Max S/G elements in I/O accelerator command */
#define IOACCEL1_MAXSGENTRIES 24
#define IOACCEL2_MAXSGENTRIES 28
--
2.29.2
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [PATCH 15/18] aacraid: move scsi_add_host()
2021-05-03 15:03 [PATCHv8 00/18] scsi: enabled reserved commands for LLDDs Hannes Reinecke
` (13 preceding siblings ...)
2021-05-03 15:03 ` [PATCH 14/18] hpsa: drop refcount field from CommandList Hannes Reinecke
@ 2021-05-03 15:03 ` Hannes Reinecke
2021-05-03 15:03 ` [PATCH 16/18] aacraid: store target id in host_scribble Hannes Reinecke
` (2 subsequent siblings)
17 siblings, 0 replies; 55+ messages in thread
From: Hannes Reinecke @ 2021-05-03 15:03 UTC (permalink / raw)
To: Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, John Garry, linux-scsi,
Hannes Reinecke, Hannes Reinecke
Move the call to scsi_add_host() so that the Scsi_Host structure
is initialized before any I/O is sent.
Signed-off-by: Hannes Reinecke <hare@suse.com>
---
drivers/scsi/aacraid/linit.c | 31 +++++++++++++++----------------
1 file changed, 15 insertions(+), 16 deletions(-)
diff --git a/drivers/scsi/aacraid/linit.c b/drivers/scsi/aacraid/linit.c
index 3168915adaa7..e5d89b309c3a 100644
--- a/drivers/scsi/aacraid/linit.c
+++ b/drivers/scsi/aacraid/linit.c
@@ -1639,6 +1639,9 @@ static int aac_probe_one(struct pci_dev *pdev, const struct pci_device_id *id)
shost->irq = pdev->irq;
shost->unique_id = unique_id;
shost->max_cmd_len = 16;
+ shost->max_id = MAXIMUM_NUM_CONTAINERS;
+ shost->max_lun = AAC_MAX_LUN;
+ shost->sg_tablesize = HBA_MAX_SG_SEPARATE;
if (aac_cfg_major == AAC_CHARDEV_NEEDS_REINIT)
aac_init_char();
@@ -1677,7 +1680,7 @@ static int aac_probe_one(struct pci_dev *pdev, const struct pci_device_id *id)
aac->base_size = AAC_MIN_FOOTPRINT_SIZE;
if ((*aac_drivers[index].init)(aac)) {
error = -ENODEV;
- goto out_unmap;
+ goto out_free_fibs;
}
if (aac->sync_mode) {
@@ -1703,9 +1706,15 @@ static int aac_probe_one(struct pci_dev *pdev, const struct pci_device_id *id)
printk(KERN_ERR "aacraid: Unable to create command thread.\n");
error = PTR_ERR(aac->thread);
aac->thread = NULL;
- goto out_deinit;
+ goto out_unmap;
}
+ pci_set_drvdata(pdev, shost);
+
+ error = scsi_add_host(shost, &pdev->dev);
+ if (error)
+ goto out_deinit;
+
aac->maximum_num_channels = aac_drivers[index].channels;
error = aac_get_adapter_info(aac);
if (error < 0)
@@ -1764,18 +1773,6 @@ static int aac_probe_one(struct pci_dev *pdev, const struct pci_device_id *id)
if (!aac->sa_firmware && aac_drivers[index].quirks & AAC_QUIRK_SRC)
aac_intr_normal(aac, 0, 2, 0, NULL);
- /*
- * dmb - we may need to move the setting of these parms somewhere else once
- * we get a fib that can report the actual numbers
- */
- shost->max_lun = AAC_MAX_LUN;
-
- pci_set_drvdata(pdev, shost);
-
- error = scsi_add_host(shost, &pdev->dev);
- if (error)
- goto out_deinit;
-
aac_scan_host(aac);
pci_enable_pcie_error_reporting(pdev);
@@ -1792,10 +1789,12 @@ static int aac_probe_one(struct pci_dev *pdev, const struct pci_device_id *id)
aac->comm_addr, aac->comm_phys);
kfree(aac->queues);
aac_adapter_ioremap(aac, 0);
- kfree(aac->fibs);
kfree(aac->fsa_dev);
+ out_free_fibs:
+ kfree(aac->fibs);
out_free_host:
scsi_host_put(shost);
+ pci_set_drvdata(pdev, NULL);
out_disable_pdev:
pci_disable_device(pdev);
out:
@@ -1903,9 +1902,9 @@ static void aac_remove_one(struct pci_dev *pdev)
struct aac_dev *aac = (struct aac_dev *)shost->hostdata;
aac_cancel_rescan_worker(aac);
- scsi_remove_host(shost);
__aac_shutdown(aac);
+ scsi_remove_host(shost);
aac_fib_map_free(aac);
dma_free_coherent(&aac->pdev->dev, aac->comm_size, aac->comm_addr,
aac->comm_phys);
--
2.29.2
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [PATCH 16/18] aacraid: store target id in host_scribble
2021-05-03 15:03 [PATCHv8 00/18] scsi: enabled reserved commands for LLDDs Hannes Reinecke
` (14 preceding siblings ...)
2021-05-03 15:03 ` [PATCH 15/18] aacraid: move scsi_add_host() Hannes Reinecke
@ 2021-05-03 15:03 ` Hannes Reinecke
2021-05-04 3:22 ` Bart Van Assche
2021-05-03 15:03 ` [PATCH 17/18] aacraid: use scsi_get_internal_cmd() Hannes Reinecke
2021-05-03 15:03 ` [PATCH 18/18] aacraid: use scsi_host_busy_iter() to traverse outstanding commands Hannes Reinecke
17 siblings, 1 reply; 55+ messages in thread
From: Hannes Reinecke @ 2021-05-03 15:03 UTC (permalink / raw)
To: Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, John Garry, linux-scsi,
Hannes Reinecke
The probe_container mechanism requires a target id to be present,
even if the device itself isn't present (yet).
As we're now allocating internal commands the target id of the
device is immutable, so store the requested target id in the
host_scribble field.
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
drivers/scsi/aacraid/aachba.c | 53 +++++++++++++++++++++++++----------
1 file changed, 38 insertions(+), 15 deletions(-)
diff --git a/drivers/scsi/aacraid/aachba.c b/drivers/scsi/aacraid/aachba.c
index f1f62b5da8b7..ef59303db9b9 100644
--- a/drivers/scsi/aacraid/aachba.c
+++ b/drivers/scsi/aacraid/aachba.c
@@ -609,9 +609,11 @@ static int aac_get_container_name(struct scsi_cmnd * scsicmd)
static int aac_probe_container_callback2(struct scsi_cmnd * scsicmd)
{
- struct fsa_dev_info *fsa_dev_ptr = ((struct aac_dev *)(scsicmd->device->host->hostdata))->fsa_dev;
+ struct aac_dev *dev = (struct aac_dev *)(scsicmd->device->host->hostdata);
+ struct fsa_dev_info *fsa_dev_ptr = dev->fsa_dev;
- if ((fsa_dev_ptr[scmd_id(scsicmd)].valid & 1))
+ if (scmd_id(scsicmd) < dev->maximum_num_containers &&
+ (fsa_dev_ptr[scmd_id(scsicmd)].valid & 1))
return aac_scsi_cmd(scsicmd);
scsicmd->result = DID_NO_CONNECT << 16;
@@ -624,6 +626,7 @@ static void _aac_probe_container2(void * context, struct fib * fibptr)
struct fsa_dev_info *fsa_dev_ptr;
int (*callback)(struct scsi_cmnd *);
struct scsi_cmnd * scsicmd = (struct scsi_cmnd *)context;
+ int cid = scmd_id(scsicmd);
int i;
@@ -631,12 +634,15 @@ static void _aac_probe_container2(void * context, struct fib * fibptr)
return;
scsicmd->SCp.Status = 0;
+ if (scsicmd->host_scribble)
+ cid = *(int *)scsicmd->host_scribble;
+
fsa_dev_ptr = fibptr->dev->fsa_dev;
- if (fsa_dev_ptr) {
+ if (fsa_dev_ptr && cid < fibptr->dev->maximum_num_containers) {
struct aac_mount * dresp = (struct aac_mount *) fib_data(fibptr);
__le32 sup_options2;
- fsa_dev_ptr += scmd_id(scsicmd);
+ fsa_dev_ptr += cid;
sup_options2 =
fibptr->dev->supplement_adapter_info.supported_options2;
@@ -671,7 +677,6 @@ static void _aac_probe_container2(void * context, struct fib * fibptr)
scsicmd->SCp.Status = le32_to_cpu(dresp->count);
}
aac_fib_complete(fibptr);
- aac_fib_free(fibptr);
callback = (int (*)(struct scsi_cmnd *))(scsicmd->SCp.ptr);
scsicmd->SCp.ptr = NULL;
(*callback)(scsicmd);
@@ -683,6 +688,7 @@ static void _aac_probe_container1(void * context, struct fib * fibptr)
struct scsi_cmnd * scsicmd;
struct aac_mount * dresp;
struct aac_query_mount *dinfo;
+ int cid;
int status;
dresp = (struct aac_mount *) fib_data(fibptr);
@@ -695,10 +701,15 @@ static void _aac_probe_container1(void * context, struct fib * fibptr)
}
}
scsicmd = (struct scsi_cmnd *) context;
-
if (!aac_valid_context(scsicmd, fibptr))
return;
-
+ cid = scmd_id(scsicmd);
+ if (scsicmd->host_scribble)
+ cid = *(int *)scsicmd->host_scribble;
+ if (cid >= fibptr->dev->maximum_num_containers) {
+ _aac_probe_container2(context, fibptr);
+ return;
+ }
aac_fib_init(fibptr);
dinfo = (struct aac_query_mount *)fib_data(fibptr);
@@ -709,7 +720,7 @@ static void _aac_probe_container1(void * context, struct fib * fibptr)
else
dinfo->command = cpu_to_le32(VM_NameServe64);
- dinfo->count = cpu_to_le32(scmd_id(scsicmd));
+ dinfo->count = cpu_to_le32(cid);
dinfo->type = cpu_to_le32(FT_FILESYS);
scsicmd->SCp.phase = AAC_OWNER_FIRMWARE;
@@ -732,10 +743,20 @@ static void _aac_probe_container1(void * context, struct fib * fibptr)
static int _aac_probe_container(struct scsi_cmnd * scsicmd, int (*callback)(struct scsi_cmnd *))
{
+ struct aac_dev * dev;
struct fib * fibptr;
int status = -ENOMEM;
+ int cid = scmd_id(scsicmd);
- if ((fibptr = aac_fib_alloc((struct aac_dev *)scsicmd->device->host->hostdata))) {
+ dev = (struct aac_dev *)scsicmd->device->host->hostdata;
+ if (scsicmd->host_scribble) {
+ cid = *(int *)scsicmd->host_scribble;
+ if (cid > dev->maximum_num_containers) {
+ status = -ENODEV;
+ goto out_status;
+ }
+ }
+ if ((fibptr = aac_fib_alloc(dev))) {
struct aac_query_mount *dinfo;
aac_fib_init(fibptr);
@@ -748,7 +769,7 @@ static int _aac_probe_container(struct scsi_cmnd * scsicmd, int (*callback)(stru
else
dinfo->command = cpu_to_le32(VM_NameServe);
- dinfo->count = cpu_to_le32(scmd_id(scsicmd));
+ dinfo->count = cpu_to_le32(cid);
dinfo->type = cpu_to_le32(FT_FILESYS);
scsicmd->SCp.ptr = (char *)callback;
scsicmd->SCp.phase = AAC_OWNER_FIRMWARE;
@@ -772,10 +793,11 @@ static int _aac_probe_container(struct scsi_cmnd * scsicmd, int (*callback)(stru
aac_fib_free(fibptr);
}
}
+out_status:
if (status < 0) {
- struct fsa_dev_info *fsa_dev_ptr = ((struct aac_dev *)(scsicmd->device->host->hostdata))->fsa_dev;
- if (fsa_dev_ptr) {
- fsa_dev_ptr += scmd_id(scsicmd);
+ struct fsa_dev_info *fsa_dev_ptr = dev->fsa_dev;
+ if (fsa_dev_ptr && cid < dev->maximum_num_containers) {
+ fsa_dev_ptr += cid;
if ((fsa_dev_ptr->valid & 1) == 0) {
fsa_dev_ptr->valid = 0;
return (*callback)(scsicmd);
@@ -794,7 +816,7 @@ static int _aac_probe_container(struct scsi_cmnd * scsicmd, int (*callback)(stru
*/
static int aac_probe_container_callback1(struct scsi_cmnd * scsicmd)
{
- scsicmd->device = NULL;
+ scsicmd->host_scribble = NULL;
return 0;
}
@@ -815,6 +837,7 @@ int aac_probe_container(struct aac_dev *dev, int cid)
return -ENOMEM;
}
scsicmd->scsi_done = aac_probe_container_scsi_done;
+ scsicmd->host_scribble = (unsigned char *)&cid;
scsicmd->device = scsidev;
scsidev->sdev_state = 0;
@@ -822,7 +845,7 @@ int aac_probe_container(struct aac_dev *dev, int cid)
scsidev->host = dev->scsi_host_ptr;
if (_aac_probe_container(scsicmd, aac_probe_container_callback1) == 0)
- while (scsicmd->device == scsidev)
+ while (scsicmd->host_scribble == (unsigned char *)&cid)
schedule();
kfree(scsidev);
status = scsicmd->SCp.Status;
--
2.29.2
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [PATCH 17/18] aacraid: use scsi_get_internal_cmd()
2021-05-03 15:03 [PATCHv8 00/18] scsi: enabled reserved commands for LLDDs Hannes Reinecke
` (15 preceding siblings ...)
2021-05-03 15:03 ` [PATCH 16/18] aacraid: store target id in host_scribble Hannes Reinecke
@ 2021-05-03 15:03 ` Hannes Reinecke
2021-05-03 15:03 ` [PATCH 18/18] aacraid: use scsi_host_busy_iter() to traverse outstanding commands Hannes Reinecke
17 siblings, 0 replies; 55+ messages in thread
From: Hannes Reinecke @ 2021-05-03 15:03 UTC (permalink / raw)
To: Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, John Garry, linux-scsi,
Hannes Reinecke
From: Hannes Reinecke <hare@suse.com>
Use scsi_get_internal_cmd() to allocate internal commands.
Internal commands are marked with a new FIB_CONTEXT_FLAG_INTERNAL_CMD,
so we need to ensure that the 'flags' field is not cleared
completely in aac_fib_send() to avoid command allocation failures.
Signed-off-by: Hannes Reinecke <hare@suse.com>
---
drivers/scsi/aacraid/aachba.c | 92 +++++++++++++++------------------
drivers/scsi/aacraid/aacraid.h | 10 ++--
drivers/scsi/aacraid/commctrl.c | 25 +++++----
drivers/scsi/aacraid/comminit.c | 2 +-
drivers/scsi/aacraid/commsup.c | 69 ++++++++++++-------------
drivers/scsi/aacraid/dpcsup.c | 2 +-
drivers/scsi/aacraid/linit.c | 13 +++--
7 files changed, 108 insertions(+), 105 deletions(-)
diff --git a/drivers/scsi/aacraid/aachba.c b/drivers/scsi/aacraid/aachba.c
index ef59303db9b9..ce60b7344be9 100644
--- a/drivers/scsi/aacraid/aachba.c
+++ b/drivers/scsi/aacraid/aachba.c
@@ -360,7 +360,7 @@ int aac_get_config_status(struct aac_dev *dev, int commit_flag)
int status = 0;
struct fib * fibptr;
- if (!(fibptr = aac_fib_alloc(dev)))
+ if (!(fibptr = aac_fib_alloc(dev, DMA_FROM_DEVICE)))
return -ENOMEM;
aac_fib_init(fibptr);
@@ -457,7 +457,7 @@ int aac_get_containers(struct aac_dev *dev)
struct aac_get_container_count_resp *dresp;
int maximum_num_containers = MAXIMUM_NUM_CONTAINERS;
- if (!(fibptr = aac_fib_alloc(dev)))
+ if (!(fibptr = aac_fib_alloc(dev, DMA_FROM_DEVICE)))
return -ENOMEM;
aac_fib_init(fibptr);
@@ -741,61 +741,56 @@ static void _aac_probe_container1(void * context, struct fib * fibptr)
}
}
-static int _aac_probe_container(struct scsi_cmnd * scsicmd, int (*callback)(struct scsi_cmnd *))
+static int _aac_probe_container(struct fib * fibptr, int (*callback)(struct scsi_cmnd *))
{
+ struct scsi_cmnd *scsicmd = fibptr->scmd;
struct aac_dev * dev;
- struct fib * fibptr;
int status = -ENOMEM;
+ struct aac_query_mount *dinfo;
int cid = scmd_id(scsicmd);
dev = (struct aac_dev *)scsicmd->device->host->hostdata;
if (scsicmd->host_scribble) {
- cid = *(int *)scsicmd->host_scribble;
- if (cid > dev->maximum_num_containers) {
+ cid = *(unsigned int *)scsicmd->host_scribble;
+ if (cid >= dev->maximum_num_containers) {
status = -ENODEV;
goto out_status;
}
}
- if ((fibptr = aac_fib_alloc(dev))) {
- struct aac_query_mount *dinfo;
- aac_fib_init(fibptr);
+ aac_fib_init(fibptr);
- dinfo = (struct aac_query_mount *)fib_data(fibptr);
+ dinfo = (struct aac_query_mount *)fib_data(fibptr);
- if (fibptr->dev->supplement_adapter_info.supported_options2 &
- AAC_OPTION_VARIABLE_BLOCK_SIZE)
- dinfo->command = cpu_to_le32(VM_NameServeAllBlk);
- else
- dinfo->command = cpu_to_le32(VM_NameServe);
+ if (fibptr->dev->supplement_adapter_info.supported_options2 &
+ AAC_OPTION_VARIABLE_BLOCK_SIZE)
+ dinfo->command = cpu_to_le32(VM_NameServeAllBlk);
+ else
+ dinfo->command = cpu_to_le32(VM_NameServe);
- dinfo->count = cpu_to_le32(cid);
- dinfo->type = cpu_to_le32(FT_FILESYS);
- scsicmd->SCp.ptr = (char *)callback;
- scsicmd->SCp.phase = AAC_OWNER_FIRMWARE;
+ dinfo->count = cpu_to_le32(cid);
+ dinfo->type = cpu_to_le32(FT_FILESYS);
+ scsicmd->SCp.ptr = (char *)callback;
+ scsicmd->SCp.phase = AAC_OWNER_FIRMWARE;
- status = aac_fib_send(ContainerCommand,
+ status = aac_fib_send(ContainerCommand,
fibptr,
sizeof(struct aac_query_mount),
FsaNormal,
0, 1,
_aac_probe_container1,
(void *) scsicmd);
- /*
- * Check that the command queued to the controller
- */
- if (status == -EINPROGRESS)
- return 0;
-
- if (status < 0) {
- scsicmd->SCp.ptr = NULL;
- aac_fib_complete(fibptr);
- aac_fib_free(fibptr);
- }
- }
+ /*
+ * Check that the command queued to the controller
+ */
+ if (status == -EINPROGRESS)
+ return 0;
out_status:
if (status < 0) {
struct fsa_dev_info *fsa_dev_ptr = dev->fsa_dev;
+
+ scsicmd->SCp.ptr = NULL;
+ aac_fib_complete(fibptr);
if (fsa_dev_ptr && cid < dev->maximum_num_containers) {
fsa_dev_ptr += cid;
if ((fsa_dev_ptr->valid & 1) == 0) {
@@ -827,29 +822,23 @@ static void aac_probe_container_scsi_done(struct scsi_cmnd *scsi_cmnd)
int aac_probe_container(struct aac_dev *dev, int cid)
{
- struct scsi_cmnd *scsicmd = kmalloc(sizeof(*scsicmd), GFP_KERNEL);
- struct scsi_device *scsidev = kmalloc(sizeof(*scsidev), GFP_KERNEL);
+ struct fib *fibptr;
+ struct scsi_cmnd *scsicmd;
int status;
- if (!scsicmd || !scsidev) {
- kfree(scsicmd);
- kfree(scsidev);
+ fibptr = aac_fib_alloc(dev, DMA_FROM_DEVICE);
+ if (!fibptr)
return -ENOMEM;
- }
+
+ scsicmd = fibptr->scmd;
scsicmd->scsi_done = aac_probe_container_scsi_done;
scsicmd->host_scribble = (unsigned char *)&cid;
- scsicmd->device = scsidev;
- scsidev->sdev_state = 0;
- scsidev->id = cid;
- scsidev->host = dev->scsi_host_ptr;
-
- if (_aac_probe_container(scsicmd, aac_probe_container_callback1) == 0)
+ if (_aac_probe_container(fibptr, aac_probe_container_callback1) == 0)
while (scsicmd->host_scribble == (unsigned char *)&cid)
schedule();
- kfree(scsidev);
status = scsicmd->SCp.Status;
- kfree(scsicmd);
+ aac_fib_free(fibptr);
return status;
}
@@ -1694,7 +1683,7 @@ static int aac_send_safw_bmic_cmd(struct aac_dev *dev,
return 0;
/* allocate FIB */
- fibptr = aac_fib_alloc(dev);
+ fibptr = aac_fib_alloc(dev, DMA_BIDIRECTIONAL);
if (!fibptr)
return -ENOMEM;
@@ -2054,7 +2043,7 @@ int aac_get_adapter_info(struct aac_dev* dev)
struct aac_bus_info *command;
struct aac_bus_info_response *bus_info;
- if (!(fibptr = aac_fib_alloc(dev)))
+ if (!(fibptr = aac_fib_alloc(dev, DMA_FROM_DEVICE)))
return -ENOMEM;
aac_fib_init(fibptr);
@@ -2101,7 +2090,7 @@ int aac_get_adapter_info(struct aac_dev* dev)
if (rcode >= 0)
memcpy(&dev->supplement_adapter_info, sinfo, sizeof(*sinfo));
if (rcode == -ERESTARTSYS) {
- fibptr = aac_fib_alloc(dev);
+ fibptr = aac_fib_alloc(dev, DMA_FROM_DEVICE);
if (!fibptr)
return -ENOMEM;
}
@@ -2811,6 +2800,8 @@ int aac_scsi_cmd(struct scsi_cmnd * scsicmd)
if (((fsa_dev_ptr[cid].valid & 1) == 0) ||
(fsa_dev_ptr[cid].sense_data.sense_key ==
NOT_READY)) {
+ struct fib * fibptr;
+
switch (scsicmd->cmnd[0]) {
case SERVICE_ACTION_IN_16:
if (!(dev->raw_io_interface) ||
@@ -2823,7 +2814,8 @@ int aac_scsi_cmd(struct scsi_cmnd * scsicmd)
case TEST_UNIT_READY:
if (dev->in_reset)
return -1;
- return _aac_probe_container(scsicmd,
+ fibptr = aac_fib_alloc_tag(dev, scsicmd);
+ return _aac_probe_container(fibptr,
aac_probe_container_callback2);
default:
break;
diff --git a/drivers/scsi/aacraid/aacraid.h b/drivers/scsi/aacraid/aacraid.h
index e3e4ecbea726..3cd7f33497d8 100644
--- a/drivers/scsi/aacraid/aacraid.h
+++ b/drivers/scsi/aacraid/aacraid.h
@@ -1291,13 +1291,16 @@ struct fsa_dev_info {
};
struct fib {
- void *next; /* this is used by the allocator */
s16 type;
s16 size;
/*
* The Adapter that this I/O is destined for.
*/
struct aac_dev *dev;
+ /*
+ * The associated scsi command
+ */
+ struct scsi_cmnd *scmd;
/*
* This is the event the sendfib routine will wait on if the
* caller did not pass one and this is synch io.
@@ -1552,7 +1555,6 @@ struct aac_dev
*/
struct fib *fibs;
- struct fib *free_fib;
spinlock_t fib_lock;
struct mutex ioctl_mutex;
@@ -1597,6 +1599,7 @@ struct aac_dev
size_t comm_size;
struct Scsi_Host *scsi_host_ptr;
+ struct scsi_device *scsi_host_dev;
int maximum_num_containers;
int maximum_num_physicals;
int maximum_num_channels;
@@ -1729,6 +1732,7 @@ struct aac_dev
#define FIB_CONTEXT_FLAG_NATIVE_HBA_TMF (0x00000020)
#define FIB_CONTEXT_FLAG_SCSI_CMD (0x00000040)
#define FIB_CONTEXT_FLAG_EH_RESET (0x00000080)
+#define FIB_CONTEXT_FLAG_INTERNAL_CMD (0x00000100)
/*
* Define the command values
@@ -2686,7 +2690,7 @@ void aac_free_irq(struct aac_dev *dev);
int aac_setup_safw_adapter(struct aac_dev *dev);
const char *aac_driverinfo(struct Scsi_Host *);
void aac_fib_vector_assign(struct aac_dev *dev);
-struct fib *aac_fib_alloc(struct aac_dev *dev);
+struct fib *aac_fib_alloc(struct aac_dev *dev, int direction);
struct fib *aac_fib_alloc_tag(struct aac_dev *dev, struct scsi_cmnd *scmd);
int aac_fib_setup(struct aac_dev *dev);
void aac_fib_map_free(struct aac_dev *dev);
diff --git a/drivers/scsi/aacraid/commctrl.c b/drivers/scsi/aacraid/commctrl.c
index e7cc927ed952..bc5b81696476 100644
--- a/drivers/scsi/aacraid/commctrl.c
+++ b/drivers/scsi/aacraid/commctrl.c
@@ -55,7 +55,7 @@ static int ioctl_send_fib(struct aac_dev * dev, void __user *arg)
if (dev->in_reset) {
return -EBUSY;
}
- fibptr = aac_fib_alloc(dev);
+ fibptr = aac_fib_alloc(dev, DMA_BIDIRECTIONAL);
if(fibptr == NULL) {
return -ENOMEM;
}
@@ -478,7 +478,7 @@ static int check_revision(struct aac_dev *dev, void __user *arg)
*/
static int aac_send_raw_srb(struct aac_dev* dev, void __user * arg)
{
- struct fib* srbfib;
+ struct fib* srbfib = NULL;
int status;
struct aac_srb *srbcmd = NULL;
struct aac_hba_cmd_req *hbacmd = NULL;
@@ -509,12 +509,6 @@ static int aac_send_raw_srb(struct aac_dev* dev, void __user * arg)
dprintk((KERN_DEBUG"aacraid: No permission to send raw srb\n"));
return -EPERM;
}
- /*
- * Allocate and initialize a Fib then setup a SRB command
- */
- if (!(srbfib = aac_fib_alloc(dev))) {
- return -ENOMEM;
- }
memset(sg_list, 0, sizeof(sg_list)); /* cleanup may take issue */
if(copy_from_user(&fibsize, &user_srb->count,sizeof(u32))){
@@ -561,6 +555,15 @@ static int aac_send_raw_srb(struct aac_dev* dev, void __user * arg)
rcode = -EINVAL;
goto cleanup;
}
+
+ /*
+ * Allocate and initialize a Fib
+ */
+ if (!(srbfib = aac_fib_alloc(dev, data_dir))) {
+ rcode = -ENOMEM;
+ goto cleanup;
+ }
+
actual_fibsize = sizeof(struct aac_srb) - sizeof(struct sgentry) +
((user_srbcmd->sg.count & 0xff) * sizeof(struct sgentry));
actual_fibsize64 = actual_fibsize + (user_srbcmd->sg.count & 0xff) *
@@ -988,8 +991,10 @@ static int aac_send_raw_srb(struct aac_dev* dev, void __user * arg)
if (rcode != -ERESTARTSYS) {
for (i = 0; i <= sg_indx; i++)
kfree(sg_list[i]);
- aac_fib_complete(srbfib);
- aac_fib_free(srbfib);
+ if (srbfib) {
+ aac_fib_complete(srbfib);
+ aac_fib_free(srbfib);
+ }
}
return rcode;
diff --git a/drivers/scsi/aacraid/comminit.c b/drivers/scsi/aacraid/comminit.c
index 355b16f0b145..0d5be76891b0 100644
--- a/drivers/scsi/aacraid/comminit.c
+++ b/drivers/scsi/aacraid/comminit.c
@@ -327,7 +327,7 @@ int aac_send_shutdown(struct aac_dev * dev)
aac_wait_for_io_completion(dev);
- fibctx = aac_fib_alloc(dev);
+ fibctx = aac_fib_alloc(dev, DMA_NONE);
if (!fibctx)
return -ENOMEM;
aac_fib_init(fibctx);
diff --git a/drivers/scsi/aacraid/commsup.c b/drivers/scsi/aacraid/commsup.c
index 54eb4d41bc2c..6e2e381b12d4 100644
--- a/drivers/scsi/aacraid/commsup.c
+++ b/drivers/scsi/aacraid/commsup.c
@@ -173,7 +173,6 @@ int aac_fib_setup(struct aac_dev * dev)
fibptr->dev = dev;
fibptr->hw_fib_va = hw_fib;
fibptr->data = (void *) fibptr->hw_fib_va->data;
- fibptr->next = fibptr+1; /* Forward chain the fibs */
init_completion(&fibptr->event_wait);
spin_lock_init(&fibptr->event_lock);
hw_fib->header.XferState = cpu_to_le32(0xffffffff);
@@ -200,14 +199,6 @@ int aac_fib_setup(struct aac_dev * dev)
*/
aac_fib_vector_assign(dev);
- /*
- * Add the fib chain to the free list
- */
- dev->fibs[dev->scsi_host_ptr->can_queue + AAC_NUM_MGT_FIB - 1].next = NULL;
- /*
- * Set 8 fibs aside for management tools
- */
- dev->free_fib = &dev->fibs[dev->scsi_host_ptr->can_queue];
return 0;
}
@@ -233,7 +224,7 @@ struct fib *aac_fib_alloc_tag(struct aac_dev *dev, struct scsi_cmnd *scmd)
fibptr->type = FSAFS_NTC_FIB_CONTEXT;
fibptr->callback_data = NULL;
fibptr->callback = NULL;
- fibptr->flags = 0;
+ fibptr->scmd = scmd;
return fibptr;
}
@@ -241,36 +232,32 @@ struct fib *aac_fib_alloc_tag(struct aac_dev *dev, struct scsi_cmnd *scmd)
/**
* aac_fib_alloc - allocate a fib
* @dev: Adapter to allocate the fib for
+ * @direction: DMA data direction
*
* Allocate a fib from the adapter fib pool. If the pool is empty we
* return NULL.
*/
-struct fib *aac_fib_alloc(struct aac_dev *dev)
+struct fib *aac_fib_alloc(struct aac_dev *dev, int direction)
{
- struct fib * fibptr;
+ struct scsi_cmnd *scmd;
+ struct fib * fibptr = NULL;
unsigned long flags;
+
+ scmd = scsi_get_internal_cmd(dev->scsi_host_dev,
+ direction == DMA_TO_DEVICE ?
+ REQ_OP_SCSI_OUT : REQ_OP_SCSI_IN,
+ BLK_MQ_REQ_NOWAIT);
spin_lock_irqsave(&dev->fib_lock, flags);
- fibptr = dev->free_fib;
- if(!fibptr){
- spin_unlock_irqrestore(&dev->fib_lock, flags);
- return fibptr;
+ if (scmd) {
+ fibptr = aac_fib_alloc_tag(dev, scmd);
+ fibptr->flags |= FIB_CONTEXT_FLAG_INTERNAL_CMD;
}
- dev->free_fib = fibptr->next;
spin_unlock_irqrestore(&dev->fib_lock, flags);
- /*
- * Set the proper node type code and node byte size
- */
- fibptr->type = FSAFS_NTC_FIB_CONTEXT;
+ if (!fibptr)
+ return NULL;
+
fibptr->size = sizeof(struct fib);
- /*
- * Null out fields that depend on being zero at the start of
- * each I/O
- */
- fibptr->hw_fib_va->header.XferState = 0;
- fibptr->flags = 0;
- fibptr->callback = NULL;
- fibptr->callback_data = NULL;
return fibptr;
}
@@ -298,8 +285,16 @@ void aac_fib_free(struct fib *fibptr)
(void*)fibptr,
le32_to_cpu(fibptr->hw_fib_va->header.XferState));
}
- fibptr->next = fibptr->dev->free_fib;
- fibptr->dev->free_fib = fibptr;
+ if (fibptr->scmd) {
+ struct scsi_cmnd *scmd = fibptr->scmd;
+
+ fibptr->scmd = NULL;
+ if (fibptr->flags & FIB_CONTEXT_FLAG_INTERNAL_CMD) {
+ scsi_put_internal_cmd(scmd);
+ fibptr->flags &= ~FIB_CONTEXT_FLAG_INTERNAL_CMD;
+ }
+ }
+ fibptr->flags = 0;
spin_unlock_irqrestore(&fibptr->dev->fib_lock, flags);
}
@@ -507,7 +502,7 @@ int aac_fib_send(u16 command, struct fib *fibptr, unsigned long size,
* will have a debug mode where the adapter can notify the host
* it had a problem and the host can log that fact.
*/
- fibptr->flags = 0;
+ fibptr->flags &= FIB_CONTEXT_FLAG_INTERNAL_CMD;
if (wait && !reply) {
return -EINVAL;
} else if (!wait && reply) {
@@ -562,7 +557,7 @@ int aac_fib_send(u16 command, struct fib *fibptr, unsigned long size,
if (!wait) {
fibptr->callback = callback;
fibptr->callback_data = callback_data;
- fibptr->flags = FIB_CONTEXT_FLAG;
+ fibptr->flags |= FIB_CONTEXT_FLAG;
}
fibptr->done = 0;
@@ -714,7 +709,7 @@ int aac_hba_send(u8 command, struct fib *fibptr, fib_callback callback,
struct aac_hba_cmd_req *hbacmd = (struct aac_hba_cmd_req *)
fibptr->hw_fib_va;
- fibptr->flags = (FIB_CONTEXT_FLAG | FIB_CONTEXT_FLAG_NATIVE_HBA);
+ fibptr->flags |= (FIB_CONTEXT_FLAG | FIB_CONTEXT_FLAG_NATIVE_HBA);
if (callback) {
wait = 0;
fibptr->callback = callback;
@@ -1663,7 +1658,7 @@ int aac_reset_adapter(struct aac_dev *aac, int forced, u8 reset_type)
retval = unblock_retval;
if ((forced < 2) && (retval == -ENODEV)) {
/* Unwind aac_send_shutdown() IOP_RESET unsupported/disabled */
- struct fib * fibctx = aac_fib_alloc(aac);
+ struct fib * fibctx = aac_fib_alloc(aac, DMA_NONE);
if (fibctx) {
struct aac_pause *cmd;
int status;
@@ -2290,7 +2285,7 @@ static int aac_send_wellness_command(struct aac_dev *dev, char *wellness_str,
int ret = -ENOMEM;
u32 vbus, vid;
- fibptr = aac_fib_alloc(dev);
+ fibptr = aac_fib_alloc(dev, DMA_TO_DEVICE);
if (!fibptr)
goto out;
@@ -2388,7 +2383,7 @@ static int aac_send_hosttime(struct aac_dev *dev, struct timespec64 *now)
struct fib *fibptr;
__le32 *info;
- fibptr = aac_fib_alloc(dev);
+ fibptr = aac_fib_alloc(dev, DMA_TO_DEVICE);
if (!fibptr)
goto out;
diff --git a/drivers/scsi/aacraid/dpcsup.c b/drivers/scsi/aacraid/dpcsup.c
index fbe334c59f37..9424792432ec 100644
--- a/drivers/scsi/aacraid/dpcsup.c
+++ b/drivers/scsi/aacraid/dpcsup.c
@@ -315,7 +315,7 @@ unsigned int aac_intr_normal(struct aac_dev *dev, u32 index, int isAif,
struct fib *fibctx;
struct aac_aifcmd *cmd;
- fibctx = aac_fib_alloc(dev);
+ fibctx = aac_fib_alloc(dev, DMA_FROM_DEVICE);
if (!fibctx)
return 1;
aac_fib_init(fibctx);
diff --git a/drivers/scsi/aacraid/linit.c b/drivers/scsi/aacraid/linit.c
index e5d89b309c3a..ab3762c01738 100644
--- a/drivers/scsi/aacraid/linit.c
+++ b/drivers/scsi/aacraid/linit.c
@@ -714,7 +714,7 @@ static int aac_eh_abort(struct scsi_cmnd* cmd)
return ret;
/* start a HBA_TMF_ABORT_TASK TMF request */
- fib = aac_fib_alloc(aac);
+ fib = aac_fib_alloc(aac, DMA_NONE);
if (!fib)
return ret;
@@ -925,7 +925,7 @@ static int aac_eh_dev_reset(struct scsi_cmnd *cmd)
pr_err("%s: Host device reset request. SCSI hang ?\n",
AAC_DRIVERNAME);
- fib = aac_fib_alloc(aac);
+ fib = aac_fib_alloc(aac, DMA_NONE);
if (!fib)
return ret;
@@ -988,7 +988,7 @@ static int aac_eh_target_reset(struct scsi_cmnd *cmd)
pr_err("%s: Host target reset request. SCSI hang ?\n",
AAC_DRIVERNAME);
- fib = aac_fib_alloc(aac);
+ fib = aac_fib_alloc(aac, DMA_NONE);
if (!fib)
return ret;
@@ -1641,6 +1641,7 @@ static int aac_probe_one(struct pci_dev *pdev, const struct pci_device_id *id)
shost->max_cmd_len = 16;
shost->max_id = MAXIMUM_NUM_CONTAINERS;
shost->max_lun = AAC_MAX_LUN;
+ shost->nr_reserved_cmds = AAC_NUM_MGT_FIB;
shost->sg_tablesize = HBA_MAX_SG_SEPARATE;
if (aac_cfg_major == AAC_CHARDEV_NEEDS_REINIT)
@@ -1715,6 +1716,10 @@ static int aac_probe_one(struct pci_dev *pdev, const struct pci_device_id *id)
if (error)
goto out_deinit;
+ aac->scsi_host_dev = scsi_get_host_dev(shost);
+ if (!aac->scsi_host_dev)
+ goto out_remove_host;
+
aac->maximum_num_channels = aac_drivers[index].channels;
error = aac_get_adapter_info(aac);
if (error < 0)
@@ -1790,6 +1795,8 @@ static int aac_probe_one(struct pci_dev *pdev, const struct pci_device_id *id)
kfree(aac->queues);
aac_adapter_ioremap(aac, 0);
kfree(aac->fsa_dev);
+ out_remove_host:
+ scsi_remove_host(shost);
out_free_fibs:
kfree(aac->fibs);
out_free_host:
--
2.29.2
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [PATCH 18/18] aacraid: use scsi_host_busy_iter() to traverse outstanding commands
2021-05-03 15:03 [PATCHv8 00/18] scsi: enabled reserved commands for LLDDs Hannes Reinecke
` (16 preceding siblings ...)
2021-05-03 15:03 ` [PATCH 17/18] aacraid: use scsi_get_internal_cmd() Hannes Reinecke
@ 2021-05-03 15:03 ` Hannes Reinecke
17 siblings, 0 replies; 55+ messages in thread
From: Hannes Reinecke @ 2021-05-03 15:03 UTC (permalink / raw)
To: Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, John Garry, linux-scsi,
Hannes Reinecke
Instead of walking the array of potential commands and trying to figure out
which one might be pending the driver should be using
scsi_host_busy_iter() to traverse all outstanding commands.
And for command abort we can now lookup the fibs directly as we now have
a 1:1 mapping between request tags and fibs.
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
drivers/scsi/aacraid/commsup.c | 49 ++++++------
drivers/scsi/aacraid/linit.c | 131 ++++++++++++++-------------------
2 files changed, 84 insertions(+), 96 deletions(-)
diff --git a/drivers/scsi/aacraid/commsup.c b/drivers/scsi/aacraid/commsup.c
index 6e2e381b12d4..73a3a90d8114 100644
--- a/drivers/scsi/aacraid/commsup.c
+++ b/drivers/scsi/aacraid/commsup.c
@@ -1468,6 +1468,32 @@ static void aac_schedule_bus_scan(struct aac_dev *aac)
aac_schedule_src_reinit_aif_worker(aac);
}
+static bool aac_close_sync_fib_iter(struct scsi_cmnd *command, void *data,
+ bool reserved)
+{
+ struct Scsi_Host *host = command->device->host;
+ struct aac_dev *aac = (struct aac_dev *)host->hostdata;
+ struct fib *fib = &aac->fibs[command->request->tag];
+ int *retval = data;
+ __le32 XferState = fib->hw_fib_va->header.XferState;
+ bool is_response_expected = false;
+
+ if (!(XferState & cpu_to_le32(NoResponseExpected | Async)) &&
+ (XferState & cpu_to_le32(ResponseExpected)))
+ is_response_expected = true;
+
+ if (is_response_expected
+ || fib->flags & FIB_CONTEXT_FLAG_WAIT) {
+ unsigned long flagv;
+ spin_lock_irqsave(&fib->event_lock, flagv);
+ complete(&fib->event_wait);
+ spin_unlock_irqrestore(&fib->event_lock, flagv);
+ schedule();
+ *retval = 0;
+ }
+ return true;
+}
+
static int _aac_reset_adapter(struct aac_dev *aac, int forced, u8 reset_type)
{
int index, quirks;
@@ -1476,7 +1502,6 @@ static int _aac_reset_adapter(struct aac_dev *aac, int forced, u8 reset_type)
int jafo = 0;
int bled;
u64 dmamask;
- int num_of_fibs = 0;
/*
* Assumptions:
@@ -1510,27 +1535,7 @@ static int _aac_reset_adapter(struct aac_dev *aac, int forced, u8 reset_type)
* Loop through the fibs, close the synchronous FIBS
*/
retval = 1;
- num_of_fibs = aac->scsi_host_ptr->can_queue + AAC_NUM_MGT_FIB;
- for (index = 0; index < num_of_fibs; index++) {
-
- struct fib *fib = &aac->fibs[index];
- __le32 XferState = fib->hw_fib_va->header.XferState;
- bool is_response_expected = false;
-
- if (!(XferState & cpu_to_le32(NoResponseExpected | Async)) &&
- (XferState & cpu_to_le32(ResponseExpected)))
- is_response_expected = true;
-
- if (is_response_expected
- || fib->flags & FIB_CONTEXT_FLAG_WAIT) {
- unsigned long flagv;
- spin_lock_irqsave(&fib->event_lock, flagv);
- complete(&fib->event_wait);
- spin_unlock_irqrestore(&fib->event_lock, flagv);
- schedule();
- retval = 0;
- }
- }
+ scsi_host_busy_iter(host, aac_close_sync_fib_iter, &retval);
/* Give some extra time for ioctls to complete. */
if (retval == 0)
ssleep(2);
diff --git a/drivers/scsi/aacraid/linit.c b/drivers/scsi/aacraid/linit.c
index ab3762c01738..52c29cc55846 100644
--- a/drivers/scsi/aacraid/linit.c
+++ b/drivers/scsi/aacraid/linit.c
@@ -681,7 +681,8 @@ static int aac_eh_abort(struct scsi_cmnd* cmd)
struct scsi_device * dev = cmd->device;
struct Scsi_Host * host = dev->host;
struct aac_dev * aac = (struct aac_dev *)host->hostdata;
- int count, found;
+ struct fib *fib;
+ int count;
u32 bus, cid;
int ret = FAILED;
@@ -691,26 +692,20 @@ static int aac_eh_abort(struct scsi_cmnd* cmd)
bus = aac_logical_to_phys(scmd_channel(cmd));
cid = scmd_id(cmd);
if (aac->hba_map[bus][cid].devtype == AAC_DEVTYPE_NATIVE_RAW) {
- struct fib *fib;
struct aac_hba_tm_req *tmf;
int status;
u64 address;
pr_err("%s: Host adapter abort request (%d,%d,%d,%d)\n",
- AAC_DRIVERNAME,
- host->host_no, sdev_channel(dev), sdev_id(dev), (int)dev->lun);
-
- found = 0;
- for (count = 0; count < (host->can_queue + AAC_NUM_MGT_FIB); ++count) {
- fib = &aac->fibs[count];
- if (*(u8 *)fib->hw_fib_va != 0 &&
- (fib->flags & FIB_CONTEXT_FLAG_NATIVE_HBA) &&
- (fib->callback_data == cmd)) {
- found = 1;
- break;
- }
- }
- if (!found)
+ AAC_DRIVERNAME, host->host_no,
+ sdev_channel(dev), sdev_id(dev), (int)dev->lun);
+
+ fib = &aac->fibs[cmd->request->tag];
+ if (*(u8 *)fib->hw_fib_va != 0 &&
+ (fib->flags & FIB_CONTEXT_FLAG_NATIVE_HBA) &&
+ (fib->callback_data == cmd))
+ ret = SUCCESS;
+ if (ret == FAILED)
return ret;
/* start a HBA_TMF_ABORT_TASK TMF request */
@@ -772,20 +767,13 @@ static int aac_eh_abort(struct scsi_cmnd* cmd)
* Mark associated FIB to not complete,
* eh handler does this
*/
- for (count = 0;
- count < (host->can_queue + AAC_NUM_MGT_FIB);
- ++count) {
- struct fib *fib = &aac->fibs[count];
-
- if (fib->hw_fib_va->header.XferState &&
- (fib->flags & FIB_CONTEXT_FLAG) &&
- (fib->callback_data == cmd)) {
- fib->flags |=
- FIB_CONTEXT_FLAG_TIMED_OUT;
- cmd->SCp.phase =
- AAC_OWNER_ERROR_HANDLER;
- ret = SUCCESS;
- }
+ fib = &aac->fibs[cmd->request->tag];
+ if (fib->hw_fib_va->header.XferState &&
+ (fib->flags & FIB_CONTEXT_FLAG) &&
+ (fib->callback_data == cmd)) {
+ fib->flags |= FIB_CONTEXT_FLAG_TIMED_OUT;
+ cmd->SCp.phase = AAC_OWNER_ERROR_HANDLER;
+ ret = SUCCESS;
}
break;
case TEST_UNIT_READY:
@@ -793,27 +781,14 @@ static int aac_eh_abort(struct scsi_cmnd* cmd)
* Mark associated FIB to not complete,
* eh handler does this
*/
- for (count = 0;
- count < (host->can_queue + AAC_NUM_MGT_FIB);
- ++count) {
- struct scsi_cmnd *command;
- struct fib *fib = &aac->fibs[count];
-
- command = fib->callback_data;
-
- if ((fib->hw_fib_va->header.XferState &
- cpu_to_le32
- (Async | NoResponseExpected)) &&
- (fib->flags & FIB_CONTEXT_FLAG) &&
- ((command)) &&
- (command->device == cmd->device)) {
- fib->flags |=
- FIB_CONTEXT_FLAG_TIMED_OUT;
- command->SCp.phase =
- AAC_OWNER_ERROR_HANDLER;
- if (command == cmd)
- ret = SUCCESS;
- }
+ fib = &aac->fibs[cmd->request->tag];
+ if ((fib->hw_fib_va->header.XferState &
+ cpu_to_le32(Async | NoResponseExpected)) &&
+ (fib->flags & FIB_CONTEXT_FLAG) &&
+ (fib->callback_data == cmd)) {
+ fib->flags |= FIB_CONTEXT_FLAG_TIMED_OUT;
+ cmd->SCp.phase = AAC_OWNER_ERROR_HANDLER;
+ ret = SUCCESS;
}
break;
}
@@ -1021,6 +996,36 @@ static int aac_eh_target_reset(struct scsi_cmnd *cmd)
return ret;
}
+static bool aac_eh_bus_reset_iter(struct scsi_cmnd *cmd, void *data,
+ bool reserved)
+{
+ struct Scsi_Host *host = cmd->device->host;
+ struct aac_dev *aac = (struct aac_dev *)host->hostdata;
+ struct fib *fib = &aac->fibs[cmd->request->tag];
+ int *cmd_bus = data;
+
+ if (fib->hw_fib_va->header.XferState &&
+ (fib->flags & FIB_CONTEXT_FLAG) &&
+ (fib->flags & FIB_CONTEXT_FLAG_SCSI_CMD)) {
+ struct aac_hba_map_info *info;
+ u32 bus, cid;
+
+ if (cmd != (struct scsi_cmnd *)fib->callback_data)
+ return true;
+ bus = aac_logical_to_phys(scmd_channel(cmd));
+ if (bus != *cmd_bus)
+ return true;
+ cid = scmd_id(cmd);
+ info = &aac->hba_map[bus][cid];
+ if (bus >= AAC_MAX_BUSES || cid >= AAC_MAX_TARGETS ||
+ info->devtype != AAC_DEVTYPE_NATIVE_RAW) {
+ fib->flags |= FIB_CONTEXT_FLAG_EH_RESET;
+ cmd->SCp.phase = AAC_OWNER_ERROR_HANDLER;
+ }
+ }
+ return true;
+}
+
/*
* aac_eh_bus_reset - Bus reset command handling
* @scsi_cmd: SCSI command block causing the reset
@@ -1038,29 +1043,7 @@ static int aac_eh_bus_reset(struct scsi_cmnd* cmd)
cmd_bus = aac_logical_to_phys(scmd_channel(cmd));
/* Mark the assoc. FIB to not complete, eh handler does this */
- for (count = 0; count < (host->can_queue + AAC_NUM_MGT_FIB); ++count) {
- struct fib *fib = &aac->fibs[count];
-
- if (fib->hw_fib_va->header.XferState &&
- (fib->flags & FIB_CONTEXT_FLAG) &&
- (fib->flags & FIB_CONTEXT_FLAG_SCSI_CMD)) {
- struct aac_hba_map_info *info;
- u32 bus, cid;
-
- cmd = (struct scsi_cmnd *)fib->callback_data;
- bus = aac_logical_to_phys(scmd_channel(cmd));
- if (bus != cmd_bus)
- continue;
- cid = scmd_id(cmd);
- info = &aac->hba_map[bus][cid];
- if (bus >= AAC_MAX_BUSES || cid >= AAC_MAX_TARGETS ||
- info->devtype != AAC_DEVTYPE_NATIVE_RAW) {
- fib->flags |= FIB_CONTEXT_FLAG_EH_RESET;
- cmd->SCp.phase = AAC_OWNER_ERROR_HANDLER;
- }
- }
- }
-
+ scsi_host_busy_iter(host, aac_eh_bus_reset_iter, &cmd_bus);
pr_err("%s: Host bus reset request. SCSI hang ?\n", AAC_DRIVERNAME);
/*
--
2.29.2
^ permalink raw reply related [flat|nested] 55+ messages in thread
* Re: [PATCH 01/18] fnic: kill 'exclude_id' argument to fnic_cleanup_io()
2021-05-03 15:03 ` [PATCH 01/18] fnic: kill 'exclude_id' argument to fnic_cleanup_io() Hannes Reinecke
@ 2021-05-04 2:12 ` Bart Van Assche
2021-05-04 9:49 ` Christoph Hellwig
1 sibling, 0 replies; 55+ messages in thread
From: Bart Van Assche @ 2021-05-04 2:12 UTC (permalink / raw)
To: Hannes Reinecke, Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, John Garry, linux-scsi
On 5/3/21 8:03 AM, Hannes Reinecke wrote:
> 'exclude_id' is always SCSI_NO_TAG, which will never be reached
> when traversing the list of tags.
Reviewed-by: Bart Van Assche <bvanassche@acm.org>
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 03/18] scsi: add scsi_{get,put}_internal_cmd() helper
2021-05-03 15:03 ` [PATCH 03/18] scsi: add scsi_{get,put}_internal_cmd() helper Hannes Reinecke
@ 2021-05-04 2:21 ` Bart Van Assche
2021-05-04 6:12 ` Hannes Reinecke
2021-05-04 9:53 ` Christoph Hellwig
2021-06-23 16:09 ` Bart Van Assche
2 siblings, 1 reply; 55+ messages in thread
From: Bart Van Assche @ 2021-05-04 2:21 UTC (permalink / raw)
To: Hannes Reinecke, Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, John Garry, linux-scsi
On 5/3/21 8:03 AM, Hannes Reinecke wrote:
> +/**
> + * scsi_get_internal_cmd - allocate an internal SCSI command
> + * @sdev: SCSI device from which to allocate the command
> + * @op: operation (REQ_OP_SCSI_IN or REQ_OP_SCSI_OUT)
> + * @flags: BLK_MQ_REQ_* flags, e.g. BLK_MQ_REQ_NOWAIT.
> + *
> + * Allocates a SCSI command for internal LLDD use.
> + */
> +struct scsi_cmnd *scsi_get_internal_cmd(struct scsi_device *sdev,
> + unsigned int op, blk_mq_req_flags_t flags)
> +{
> + struct request *rq;
> + struct scsi_cmnd *scmd;
> +
> + WARN_ON_ONCE(((op & REQ_OP_MASK) != REQ_OP_SCSI_IN) &&
> + ((op & REQ_OP_MASK) != REQ_OP_SCSI_OUT));
> + rq = blk_mq_alloc_request(sdev->request_queue, op, flags);
> + if (IS_ERR(rq))
> + return NULL;
> + scmd = blk_mq_rq_to_pdu(rq);
> + scmd->request = rq;
> + scmd->device = sdev;
> + return scmd;
> +}
> +EXPORT_SYMBOL_GPL(scsi_get_internal_cmd);
Multiple fields that are initialized by the regular command submission
path are not initialized by the above function, e.g. scmd->tag. Has it
been considered to call scsi_init_command() instead of adding yet
another code path that initializes scmd->request?
Thanks,
Bart.
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 04/18] fnic: use internal commands
2021-05-03 15:03 ` [PATCH 04/18] fnic: use internal commands Hannes Reinecke
@ 2021-05-04 2:25 ` Bart Van Assche
2021-05-04 6:12 ` Hannes Reinecke
0 siblings, 1 reply; 55+ messages in thread
From: Bart Van Assche @ 2021-05-04 2:25 UTC (permalink / raw)
To: Hannes Reinecke, Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, John Garry, linux-scsi
On 5/3/21 8:03 AM, Hannes Reinecke wrote:
> - tag = sc->request->tag;
> - if (unlikely(tag < 0)) {
> - /*
> - * Really should fix the midlayer to pass in a proper
> - * request for ioctls...
> - */
> - tag = fnic_scsi_host_start_tag(fnic, sc);
> - if (unlikely(tag == SCSI_NO_TAG))
> - goto fnic_device_reset_end;
> - tag_gen_flag = 1;
> - new_sc = 1;
> - }
Since this patch removes the only callers of fnic_scsi_host_start_tag()
and fnic_scsi_host_end_tag(), please modify this patch such that it also
removes these functions.
Thanks,
Bart.
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 05/18] scsi: use real inquiry data when initialising devices
2021-05-03 15:03 ` [PATCH 05/18] scsi: use real inquiry data when initialising devices Hannes Reinecke
@ 2021-05-04 2:28 ` Bart Van Assche
2021-05-04 9:55 ` Christoph Hellwig
1 sibling, 0 replies; 55+ messages in thread
From: Bart Van Assche @ 2021-05-04 2:28 UTC (permalink / raw)
To: Hannes Reinecke, Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, John Garry, linux-scsi
On 5/3/21 8:03 AM, Hannes Reinecke wrote:
> Use dummy inquiry data when initialising devices and not just
> some 'nullnullnull' string.
Reviewed-by: Bart Van Assche <bvanassche@acm.org>
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 06/18] scsi: Use dummy inquiry data for the host device
2021-05-03 15:03 ` [PATCH 06/18] scsi: Use dummy inquiry data for the host device Hannes Reinecke
@ 2021-05-04 2:52 ` Bart Van Assche
2021-05-04 6:14 ` Hannes Reinecke
0 siblings, 1 reply; 55+ messages in thread
From: Bart Van Assche @ 2021-05-04 2:52 UTC (permalink / raw)
To: Hannes Reinecke, Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, John Garry, linux-scsi
On 5/3/21 8:03 AM, Hannes Reinecke wrote:
> +/**
> + * scsi_device_is_host_dev - Check if a scsi device is a host device
> + * @sdev: SCSI device to test
> + *
> + * Returns: true if @sdev is a host device, false otherwise
> + */
> +bool scsi_device_is_host_dev(struct scsi_device *sdev)
> +{
> + return ((const unsigned char *)sdev->inquiry == scsi_null_inquiry);
> +}
No parentheses around the expression in a return statement please.
> +EXPORT_SYMBOL_GPL(scsi_device_is_host_dev);
Does this mean that this function will be used outside the SCSI core?
Thanks,
Bart.
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 07/18] scsi: revamp host device handling
2021-05-03 15:03 ` [PATCH 07/18] scsi: revamp host device handling Hannes Reinecke
@ 2021-05-04 3:06 ` Bart Van Assche
2021-05-04 6:15 ` Hannes Reinecke
2021-05-04 9:59 ` Christoph Hellwig
1 sibling, 1 reply; 55+ messages in thread
From: Bart Van Assche @ 2021-05-04 3:06 UTC (permalink / raw)
To: Hannes Reinecke, Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, John Garry, linux-scsi
On 5/3/21 8:03 AM, Hannes Reinecke wrote:
> + * Attach a single scsi_device to the Scsi_Host. The primary aim
> + * for this device is to serve as a container from which valid
> + * scsi commands can be allocated from. Each scsi command will carry
> + * an unused/free command tag,
Shouldn't introducing this comment be deferred to the patch that
actually reserves a tag for internal commands (this does not mean that
I'm convinced that this is the right approach)?
> + * which then can be used by the LLDD to
> + * send internal or passthrough commands without having to find a
> + * valid command tag internally.
Please change "valid SCSI commands" into "internal SCSI commands".
Thanks,
Bart.
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 08/18] snic: use reserved commands
2021-05-03 15:03 ` [PATCH 08/18] snic: use reserved commands Hannes Reinecke
@ 2021-05-04 3:12 ` Bart Van Assche
2021-05-04 6:16 ` Hannes Reinecke
0 siblings, 1 reply; 55+ messages in thread
From: Bart Van Assche @ 2021-05-04 3:12 UTC (permalink / raw)
To: Hannes Reinecke, Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, John Garry, linux-scsi
On 5/3/21 8:03 AM, Hannes Reinecke wrote:
> @@ -2170,42 +2144,42 @@ snic_device_reset(struct scsi_cmnd *sc)
> goto dev_rst_end;
> }
>
> - /* There is no tag when lun reset is issue through ioctl. */
> - if (unlikely(tag <= SNIC_NO_TAG)) {
> - SNIC_HOST_INFO(snic->shost,
> - "Devrst: LUN Reset Recvd thru IOCTL.\n");
> + reset_sc = scsi_get_internal_cmd(sc->device, REQ_OP_SCSI_IN,
> + BLK_MQ_REQ_NOWAIT);
> + if (!reset_sc)
> + goto dev_rst_end;
The SCSI error handler may call .eh_device_reset_handler and other error
handling callbacks if no tags are available. If no tags are available,
scsi_get_internal_cmd() will fail. If scsi_get_internal_cmd() fails,
snic_device_reset() will fail. Does that count as a regression?
Thanks,
Bart.
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 10/18] scsi: implement reserved command handling
2021-05-03 15:03 ` [PATCH 10/18] scsi: implement reserved command handling Hannes Reinecke
@ 2021-05-04 3:20 ` Bart Van Assche
2021-05-04 6:17 ` Hannes Reinecke
2021-05-05 0:45 ` Bart Van Assche
1 sibling, 1 reply; 55+ messages in thread
From: Bart Van Assche @ 2021-05-04 3:20 UTC (permalink / raw)
To: Hannes Reinecke, Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, John Garry, linux-scsi
On 5/3/21 8:03 AM, Hannes Reinecke wrote:
> These commands are set aside before allocating the block-mq tag bitmap,
> so they'll never show up as busy in the tag map.
That doesn't sound correct to me. Should the above perhaps be changed
into "blk_mq_start_request() is never called for internal commands so
they'll never show up as busy in the tag map"?
Thanks,
Bart.
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 16/18] aacraid: store target id in host_scribble
2021-05-03 15:03 ` [PATCH 16/18] aacraid: store target id in host_scribble Hannes Reinecke
@ 2021-05-04 3:22 ` Bart Van Assche
2021-05-04 6:18 ` Hannes Reinecke
0 siblings, 1 reply; 55+ messages in thread
From: Bart Van Assche @ 2021-05-04 3:22 UTC (permalink / raw)
To: Hannes Reinecke, Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, John Garry, linux-scsi
On 5/3/21 8:03 AM, Hannes Reinecke wrote:
> The probe_container mechanism requires a target id to be present,
> even if the device itself isn't present (yet).
> As we're now allocating internal commands the target id of the
> device is immutable, so store the requested target id in the
> host_scribble field.
A more elegant solution is probably to introduce private data per SCSI
command and to set the .cmd_size member in the SCSI host template. I'd
like to get rid of the host_scribble field because it makes the SCSI
command data structure larger than necessary for SCSI LLDs that don't
use 'host_scribble'.
Thanks,
Bart.
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 03/18] scsi: add scsi_{get,put}_internal_cmd() helper
2021-05-04 2:21 ` Bart Van Assche
@ 2021-05-04 6:12 ` Hannes Reinecke
2021-06-23 10:57 ` John Garry
0 siblings, 1 reply; 55+ messages in thread
From: Hannes Reinecke @ 2021-05-04 6:12 UTC (permalink / raw)
To: Bart Van Assche, Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, John Garry, linux-scsi
On 5/4/21 4:21 AM, Bart Van Assche wrote:
> On 5/3/21 8:03 AM, Hannes Reinecke wrote:
>> +/**
>> + * scsi_get_internal_cmd - allocate an internal SCSI command
>> + * @sdev: SCSI device from which to allocate the command
>> + * @op: operation (REQ_OP_SCSI_IN or REQ_OP_SCSI_OUT)
>> + * @flags: BLK_MQ_REQ_* flags, e.g. BLK_MQ_REQ_NOWAIT.
>> + *
>> + * Allocates a SCSI command for internal LLDD use.
>> + */
>> +struct scsi_cmnd *scsi_get_internal_cmd(struct scsi_device *sdev,
>> + unsigned int op, blk_mq_req_flags_t flags)
>> +{
>> + struct request *rq;
>> + struct scsi_cmnd *scmd;
>> +
>> + WARN_ON_ONCE(((op & REQ_OP_MASK) != REQ_OP_SCSI_IN) &&
>> + ((op & REQ_OP_MASK) != REQ_OP_SCSI_OUT));
>> + rq = blk_mq_alloc_request(sdev->request_queue, op, flags);
>> + if (IS_ERR(rq))
>> + return NULL;
>> + scmd = blk_mq_rq_to_pdu(rq);
>> + scmd->request = rq;
>> + scmd->device = sdev;
>> + return scmd;
>> +}
>> +EXPORT_SYMBOL_GPL(scsi_get_internal_cmd);
>
> Multiple fields that are initialized by the regular command submission
> path are not initialized by the above function, e.g. scmd->tag. Has it
> been considered to call scsi_init_command() instead of adding yet
> another code path that initializes scmd->request?
>
Hmm. No, I don't think it's a good idea.
Basic idea is that the SCSI request serves as a container for (non-SCSI)
management commands, _and_ that they are submitted directly from within
the driver, ie never ever enter ->queue_rq().
As such we don't need an initialisation vie scsi_init_request(), and it
would actually be counter-productive as we would be setting up fields
which we'll never need anyway, and might need to tear down afterwards.
Cheers,
Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
hare@suse.de +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 04/18] fnic: use internal commands
2021-05-04 2:25 ` Bart Van Assche
@ 2021-05-04 6:12 ` Hannes Reinecke
0 siblings, 0 replies; 55+ messages in thread
From: Hannes Reinecke @ 2021-05-04 6:12 UTC (permalink / raw)
To: Bart Van Assche, Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, John Garry, linux-scsi
On 5/4/21 4:25 AM, Bart Van Assche wrote:
> On 5/3/21 8:03 AM, Hannes Reinecke wrote:
>> - tag = sc->request->tag;
>> - if (unlikely(tag < 0)) {
>> - /*
>> - * Really should fix the midlayer to pass in a proper
>> - * request for ioctls...
>> - */
>> - tag = fnic_scsi_host_start_tag(fnic, sc);
>> - if (unlikely(tag == SCSI_NO_TAG))
>> - goto fnic_device_reset_end;
>> - tag_gen_flag = 1;
>> - new_sc = 1;
>> - }
>
> Since this patch removes the only callers of fnic_scsi_host_start_tag()
> and fnic_scsi_host_end_tag(), please modify this patch such that it also
> removes these functions.
>
Of course.
Will do in the next round.
Cheers,
Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
hare@suse.de +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 06/18] scsi: Use dummy inquiry data for the host device
2021-05-04 2:52 ` Bart Van Assche
@ 2021-05-04 6:14 ` Hannes Reinecke
0 siblings, 0 replies; 55+ messages in thread
From: Hannes Reinecke @ 2021-05-04 6:14 UTC (permalink / raw)
To: Bart Van Assche, Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, John Garry, linux-scsi
On 5/4/21 4:52 AM, Bart Van Assche wrote:
> On 5/3/21 8:03 AM, Hannes Reinecke wrote:
>> +/**
>> + * scsi_device_is_host_dev - Check if a scsi device is a host device
>> + * @sdev: SCSI device to test
>> + *
>> + * Returns: true if @sdev is a host device, false otherwise
>> + */
>> +bool scsi_device_is_host_dev(struct scsi_device *sdev)
>> +{
>> + return ((const unsigned char *)sdev->inquiry == scsi_null_inquiry);
>> +}
>
> No parentheses around the expression in a return statement please.
>
Okay.
>> +EXPORT_SYMBOL_GPL(scsi_device_is_host_dev);
>
> Does this mean that this function will be used outside the SCSI core?
>
Will have to check; might be a left-over from previous iterations.
Cheers,
Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
hare@suse.de +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 07/18] scsi: revamp host device handling
2021-05-04 3:06 ` Bart Van Assche
@ 2021-05-04 6:15 ` Hannes Reinecke
0 siblings, 0 replies; 55+ messages in thread
From: Hannes Reinecke @ 2021-05-04 6:15 UTC (permalink / raw)
To: Bart Van Assche, Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, John Garry, linux-scsi
On 5/4/21 5:06 AM, Bart Van Assche wrote:
> On 5/3/21 8:03 AM, Hannes Reinecke wrote:
>> + * Attach a single scsi_device to the Scsi_Host. The primary aim
>> + * for this device is to serve as a container from which valid
>> + * scsi commands can be allocated from. Each scsi command will carry
>> + * an unused/free command tag,
>
> Shouldn't introducing this comment be deferred to the patch that
> actually reserves a tag for internal commands (this does not mean that
> I'm convinced that this is the right approach)?
>
Will do.
>> + * which then can be used by the LLDD to
>> + * send internal or passthrough commands without having to find a
>> + * valid command tag internally.
>
> Please change "valid SCSI commands" into "internal SCSI commands".
>
Okay.
Cheers,
Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
hare@suse.de +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 08/18] snic: use reserved commands
2021-05-04 3:12 ` Bart Van Assche
@ 2021-05-04 6:16 ` Hannes Reinecke
0 siblings, 0 replies; 55+ messages in thread
From: Hannes Reinecke @ 2021-05-04 6:16 UTC (permalink / raw)
To: Bart Van Assche, Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, John Garry, linux-scsi
On 5/4/21 5:12 AM, Bart Van Assche wrote:
> On 5/3/21 8:03 AM, Hannes Reinecke wrote:
>> @@ -2170,42 +2144,42 @@ snic_device_reset(struct scsi_cmnd *sc)
>> goto dev_rst_end;
>> }
>>
>> - /* There is no tag when lun reset is issue through ioctl. */
>> - if (unlikely(tag <= SNIC_NO_TAG)) {
>> - SNIC_HOST_INFO(snic->shost,
>> - "Devrst: LUN Reset Recvd thru IOCTL.\n");
>> + reset_sc = scsi_get_internal_cmd(sc->device, REQ_OP_SCSI_IN,
>> + BLK_MQ_REQ_NOWAIT);
>> + if (!reset_sc)
>> + goto dev_rst_end;
>
> The SCSI error handler may call .eh_device_reset_handler and other error
> handling callbacks if no tags are available. If no tags are available,
> scsi_get_internal_cmd() will fail. If scsi_get_internal_cmd() fails,
> snic_device_reset() will fail. Does that count as a regression?
>
The snic driver _requires_ a free tag to send a reset.
If no free tags are available the driver cannot send a reset, even in
the old code.
So no, this is not a regression.
Cheers,
Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
hare@suse.de +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 10/18] scsi: implement reserved command handling
2021-05-04 3:20 ` Bart Van Assche
@ 2021-05-04 6:17 ` Hannes Reinecke
2021-05-04 10:55 ` John Garry
0 siblings, 1 reply; 55+ messages in thread
From: Hannes Reinecke @ 2021-05-04 6:17 UTC (permalink / raw)
To: Bart Van Assche, Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, John Garry, linux-scsi
On 5/4/21 5:20 AM, Bart Van Assche wrote:
> On 5/3/21 8:03 AM, Hannes Reinecke wrote:
>> These commands are set aside before allocating the block-mq tag bitmap,
>> so they'll never show up as busy in the tag map.
>
> That doesn't sound correct to me. Should the above perhaps be changed
> into "blk_mq_start_request() is never called for internal commands so
> they'll never show up as busy in the tag map"?
>
Yes, will do.
Cheers,
Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
hare@suse.de +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 16/18] aacraid: store target id in host_scribble
2021-05-04 3:22 ` Bart Van Assche
@ 2021-05-04 6:18 ` Hannes Reinecke
0 siblings, 0 replies; 55+ messages in thread
From: Hannes Reinecke @ 2021-05-04 6:18 UTC (permalink / raw)
To: Bart Van Assche, Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, John Garry, linux-scsi
On 5/4/21 5:22 AM, Bart Van Assche wrote:
> On 5/3/21 8:03 AM, Hannes Reinecke wrote:
>> The probe_container mechanism requires a target id to be present,
>> even if the device itself isn't present (yet).
>> As we're now allocating internal commands the target id of the
>> device is immutable, so store the requested target id in the
>> host_scribble field.
>
> A more elegant solution is probably to introduce private data per SCSI
> command and to set the .cmd_size member in the SCSI host template. I'd
> like to get rid of the host_scribble field because it makes the SCSI
> command data structure larger than necessary for SCSI LLDs that don't
> use 'host_scribble'.
>
Ah. Good idea, both with using the .cmd_size and removing the
host_scribble field.
Cheers,
Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
hare@suse.de +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 01/18] fnic: kill 'exclude_id' argument to fnic_cleanup_io()
2021-05-03 15:03 ` [PATCH 01/18] fnic: kill 'exclude_id' argument to fnic_cleanup_io() Hannes Reinecke
2021-05-04 2:12 ` Bart Van Assche
@ 2021-05-04 9:49 ` Christoph Hellwig
1 sibling, 0 replies; 55+ messages in thread
From: Christoph Hellwig @ 2021-05-04 9:49 UTC (permalink / raw)
To: Hannes Reinecke
Cc: Martin K. Petersen, Christoph Hellwig, James Bottomley,
John Garry, linux-scsi
Looks good,
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 02/18] fnic: use scsi_host_busy_iter() to traverse commands
2021-05-03 15:03 ` [PATCH 02/18] fnic: use scsi_host_busy_iter() to traverse commands Hannes Reinecke
@ 2021-05-04 9:50 ` Christoph Hellwig
2021-05-04 12:53 ` Hannes Reinecke
0 siblings, 1 reply; 55+ messages in thread
From: Christoph Hellwig @ 2021-05-04 9:50 UTC (permalink / raw)
To: Hannes Reinecke
Cc: Martin K. Petersen, Christoph Hellwig, James Bottomley,
John Garry, linux-scsi
On Mon, May 03, 2021 at 05:03:17PM +0200, Hannes Reinecke wrote:
> Use scsi_host_busy_iter() to traverse commands instead of
> hand-crafted routines walking the command list.
While the replacement looks like the right thing to do at the micro level,
can we take one step back?
Shouldn't completing commands be left entirely to the SCSI EH code
instead of messing with it in the driver?
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 03/18] scsi: add scsi_{get,put}_internal_cmd() helper
2021-05-03 15:03 ` [PATCH 03/18] scsi: add scsi_{get,put}_internal_cmd() helper Hannes Reinecke
2021-05-04 2:21 ` Bart Van Assche
@ 2021-05-04 9:53 ` Christoph Hellwig
2021-05-04 12:54 ` Hannes Reinecke
2021-06-23 16:09 ` Bart Van Assche
2 siblings, 1 reply; 55+ messages in thread
From: Christoph Hellwig @ 2021-05-04 9:53 UTC (permalink / raw)
To: Hannes Reinecke
Cc: Martin K. Petersen, Christoph Hellwig, James Bottomley,
John Garry, linux-scsi
On Mon, May 03, 2021 at 05:03:18PM +0200, Hannes Reinecke wrote:
> +struct scsi_cmnd *scsi_get_internal_cmd(struct scsi_device *sdev,
> + unsigned int op, blk_mq_req_flags_t flags)
Weird indentation - prototype continuations either use two tabs or
are aligned after the opening brace (I generally prefer the former).
> +{
> + struct request *rq;
> + struct scsi_cmnd *scmd;
> +
> + WARN_ON_ONCE(((op & REQ_OP_MASK) != REQ_OP_SCSI_IN) &&
> + ((op & REQ_OP_MASK) != REQ_OP_SCSI_OUT));
Woudn't a simple bool write command make more sense than passing the
actual op here?
> + rq = blk_mq_alloc_request(sdev->request_queue, op, flags);
> + if (IS_ERR(rq))
> + return NULL;
> + scmd = blk_mq_rq_to_pdu(rq);
> + scmd->request = rq;
> + scmd->device = sdev;
Maybe a comment that explains what part of the scmd are initialized
and which not would be useful.
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 05/18] scsi: use real inquiry data when initialising devices
2021-05-03 15:03 ` [PATCH 05/18] scsi: use real inquiry data when initialising devices Hannes Reinecke
2021-05-04 2:28 ` Bart Van Assche
@ 2021-05-04 9:55 ` Christoph Hellwig
2021-05-04 12:57 ` Hannes Reinecke
1 sibling, 1 reply; 55+ messages in thread
From: Christoph Hellwig @ 2021-05-04 9:55 UTC (permalink / raw)
To: Hannes Reinecke
Cc: Martin K. Petersen, Christoph Hellwig, James Bottomley,
John Garry, linux-scsi
On Mon, May 03, 2021 at 05:03:20PM +0200, Hannes Reinecke wrote:
> Use dummy inquiry data when initialising devices and not just
> some 'nullnullnull' string.
Why?
> +/*
> + * Dummy inquiry for virtual LUNs:
> + *
> + * standard INQUIRY: [qualifier indicates no connected LU]
> + * PQual=1 Device_type=31 RMB=0 LU_CONG=0 version=0x05 [SPC-3]
> + * [AERC=0] [TrmTsk=0] NormACA=0 HiSUP=0 Resp_data_format=2
> + * SCCS=0 ACC=0 TPGS=0 3PC=0 Protect=0 [BQue=0]
> + * EncServ=0 MultiP=0 [MChngr=0] [ACKREQQ=0] Addr16=0
> + * [RelAdr=0] WBus16=0 Sync=0 [Linked=0] [TranDis=0] CmdQue=0
> + * length=36 (0x24) Peripheral device type: no physical device on this lu
> + * Vendor identification: LINUX
> + * Product identification: VIRTUALLUN
> + * Product revision level: 1.0
> + */
You don't juse set this up for virtual Luns, but as a default for all
scsi_devices before calling inquirty. I'd much helper with a helper
to fill out fake inquiry data rather than having seemingly valid data
for all devices before inquirty is called or if it fails.
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 07/18] scsi: revamp host device handling
2021-05-03 15:03 ` [PATCH 07/18] scsi: revamp host device handling Hannes Reinecke
2021-05-04 3:06 ` Bart Van Assche
@ 2021-05-04 9:59 ` Christoph Hellwig
2021-05-04 13:10 ` Hannes Reinecke
1 sibling, 1 reply; 55+ messages in thread
From: Christoph Hellwig @ 2021-05-04 9:59 UTC (permalink / raw)
To: Hannes Reinecke
Cc: Martin K. Petersen, Christoph Hellwig, James Bottomley,
John Garry, linux-scsi
So right now scsi_get_host_dev/scsi_free_host_dev is entirely unused.
І'd rather just kill them off rather than giving them a new life and
hacking all over the core code for them.
What do you need the scsi_device for instead of just having a
request_queue for comands to the controller? If we need a scsi_device
can we somehow make sure it doesn't hit the scanning and sysfs code
from a much higher level?
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 09/18] snic: use tagset iter for traversing commands
2021-05-03 15:03 ` [PATCH 09/18] snic: use tagset iter for traversing commands Hannes Reinecke
@ 2021-05-04 9:59 ` Christoph Hellwig
0 siblings, 0 replies; 55+ messages in thread
From: Christoph Hellwig @ 2021-05-04 9:59 UTC (permalink / raw)
To: Hannes Reinecke
Cc: Martin K. Petersen, Christoph Hellwig, James Bottomley,
John Garry, linux-scsi
Same questions as for fnic: why does the driver implements its own
command completion in the EH path?
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 10/18] scsi: implement reserved command handling
2021-05-04 6:17 ` Hannes Reinecke
@ 2021-05-04 10:55 ` John Garry
2021-05-04 13:12 ` Hannes Reinecke
0 siblings, 1 reply; 55+ messages in thread
From: John Garry @ 2021-05-04 10:55 UTC (permalink / raw)
To: Hannes Reinecke, Bart Van Assche, Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, linux-scsi
On 04/05/2021 07:17, Hannes Reinecke wrote:
> On 5/4/21 5:20 AM, Bart Van Assche wrote:
>> On 5/3/21 8:03 AM, Hannes Reinecke wrote:
>>> These commands are set aside before allocating the block-mq tag bitmap,
>>> so they'll never show up as busy in the tag map.
>>
>> That doesn't sound correct to me. Should the above perhaps be changed
>> into "blk_mq_start_request() is never called for internal commands so
>> they'll never show up as busy in the tag map"?
>>
> Yes, will do.
So why don't these - or shouldn't these - turn up in the busy tag map?
One of the motivations to use these block requests for internal commands
is that we can take advantage of the block layer handling for CPU
hotplug for MQ hosts, i.e. if blk-mq can't see these are inflight, then
they would be missed in blk_mq_hctx_notify_offline() ->
blk_mq_hctx_has_requests(), right? And who knows what else...
Thanks,
John
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 02/18] fnic: use scsi_host_busy_iter() to traverse commands
2021-05-04 9:50 ` Christoph Hellwig
@ 2021-05-04 12:53 ` Hannes Reinecke
0 siblings, 0 replies; 55+ messages in thread
From: Hannes Reinecke @ 2021-05-04 12:53 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Martin K. Petersen, James Bottomley, John Garry, linux-scsi
On 5/4/21 11:50 AM, Christoph Hellwig wrote:
> On Mon, May 03, 2021 at 05:03:17PM +0200, Hannes Reinecke wrote:
>> Use scsi_host_busy_iter() to traverse commands instead of
>> hand-crafted routines walking the command list.
>
> While the replacement looks like the right thing to do at the micro level,
> can we take one step back?
>
> Shouldn't completing commands be left entirely to the SCSI EH code
> instead of messing with it in the driver?
>
Not sure if that'd be easily possible.
Most drivers operate under the assumption that upon return from
eh_device_reset_handler() all commands for that device (except for the
reset command itself and the command inducing the reset) will have to be
aborted or completed.
As the call is synchronous, the LLDDs have to _wait_ for completion to
happen (which, incidentally, is the cause for most of the issues we're
having with SCSI EH...), and only then can they return from the callback.
So I can't see how we could delegate this to SCSI EH, as this would mean
a break with the current behaviour.
Mind you, I would _love_ to be able to do an asynchronous device reset,
seeing that we had _really_ good experience with the asynchronous
command abort, but that will be an incompatible change.
Cheers,
Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
hare@suse.de +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 03/18] scsi: add scsi_{get,put}_internal_cmd() helper
2021-05-04 9:53 ` Christoph Hellwig
@ 2021-05-04 12:54 ` Hannes Reinecke
0 siblings, 0 replies; 55+ messages in thread
From: Hannes Reinecke @ 2021-05-04 12:54 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Martin K. Petersen, James Bottomley, John Garry, linux-scsi
On 5/4/21 11:53 AM, Christoph Hellwig wrote:
> On Mon, May 03, 2021 at 05:03:18PM +0200, Hannes Reinecke wrote:
>> +struct scsi_cmnd *scsi_get_internal_cmd(struct scsi_device *sdev,
>> + unsigned int op, blk_mq_req_flags_t flags)
>
> Weird indentation - prototype continuations either use two tabs or
> are aligned after the opening brace (I generally prefer the former).
>
Yeah, I'm never sure how one should be indenting the second line for a
function declaration. But I'll fix it.
>> +{
>> + struct request *rq;
>> + struct scsi_cmnd *scmd;
>> +
>> + WARN_ON_ONCE(((op & REQ_OP_MASK) != REQ_OP_SCSI_IN) &&
>> + ((op & REQ_OP_MASK) != REQ_OP_SCSI_OUT));
>
> Woudn't a simple bool write command make more sense than passing the
> actual op here?
>
Yep, can do.
>> + rq = blk_mq_alloc_request(sdev->request_queue, op, flags);
>> + if (IS_ERR(rq))
>> + return NULL;
>> + scmd = blk_mq_rq_to_pdu(rq);
>> + scmd->request = rq;
>> + scmd->device = sdev;
>
> Maybe a comment that explains what part of the scmd are initialized
> and which not would be useful.
>
Okay.
Cheers,
Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
hare@suse.de +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 05/18] scsi: use real inquiry data when initialising devices
2021-05-04 9:55 ` Christoph Hellwig
@ 2021-05-04 12:57 ` Hannes Reinecke
0 siblings, 0 replies; 55+ messages in thread
From: Hannes Reinecke @ 2021-05-04 12:57 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Martin K. Petersen, James Bottomley, John Garry, linux-scsi
On 5/4/21 11:55 AM, Christoph Hellwig wrote:
> On Mon, May 03, 2021 at 05:03:20PM +0200, Hannes Reinecke wrote:
>> Use dummy inquiry data when initialising devices and not just
>> some 'nullnullnull' string.
>
> Why?
>
Because it's really weird if you start up scsi_debug with thousands of
devices and then call 'lsscsi' repeatedly. That will print out several
devices with 'nullnullnull', only to be replaced with the 'real' inquiry
data during device discovery.
I'd rather have a valid inquiry right from the start.
>> +/*
>> + * Dummy inquiry for virtual LUNs:
>> + *
>> + * standard INQUIRY: [qualifier indicates no connected LU]
>> + * PQual=1 Device_type=31 RMB=0 LU_CONG=0 version=0x05 [SPC-3]
>> + * [AERC=0] [TrmTsk=0] NormACA=0 HiSUP=0 Resp_data_format=2
>> + * SCCS=0 ACC=0 TPGS=0 3PC=0 Protect=0 [BQue=0]
>> + * EncServ=0 MultiP=0 [MChngr=0] [ACKREQQ=0] Addr16=0
>> + * [RelAdr=0] WBus16=0 Sync=0 [Linked=0] [TranDis=0] CmdQue=0
>> + * length=36 (0x24) Peripheral device type: no physical device on this lu
>> + * Vendor identification: LINUX
>> + * Product identification: VIRTUALLUN
>> + * Product revision level: 1.0
>> + */
>
> You don't juse set this up for virtual Luns, but as a default for all
> scsi_devices before calling inquirty. I'd much helper with a helper
> to fill out fake inquiry data rather than having seemingly valid data
> for all devices before inquirty is called or if it fails.
>
Right. Will be doing so.
Cheers,
Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
hare@suse.de +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 07/18] scsi: revamp host device handling
2021-05-04 9:59 ` Christoph Hellwig
@ 2021-05-04 13:10 ` Hannes Reinecke
0 siblings, 0 replies; 55+ messages in thread
From: Hannes Reinecke @ 2021-05-04 13:10 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Martin K. Petersen, James Bottomley, John Garry, linux-scsi
On 5/4/21 11:59 AM, Christoph Hellwig wrote:
> So right now scsi_get_host_dev/scsi_free_host_dev is entirely unused.
>
> І'd rather just kill them off rather than giving them a new life and
> hacking all over the core code for them.
>
> What do you need the scsi_device for instead of just having a
> request_queue for comands to the controller? If we need a scsi_device
> can we somehow make sure it doesn't hit the scanning and sysfs code
> from a much higher level?
>
That is what I did in v2, and got the response:
> That was just a question on why virtio uses the per-device tags, which
> didn't look like it made any sense. What I'm worried about here is
> mixing up the concept of reserved tags in the tagset, and queues to
> use them. Note that we already have the scsi_get_host_dev to allocate
> a scsi_device and thus a request_queue for the host itself. That
> seems like the better interface to use a tag for a host wide command
> vs introducing a parallel path.
(To be found at
https://lore.kernel.org/linux-scsi/20200311062228.GA13522@infradead.org/
if you are curious.)
So do you retract on that statement?
Or did I misinterpret something there?
Cheers,
Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
hare@suse.de +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 10/18] scsi: implement reserved command handling
2021-05-04 10:55 ` John Garry
@ 2021-05-04 13:12 ` Hannes Reinecke
2021-05-04 16:59 ` Bart Van Assche
0 siblings, 1 reply; 55+ messages in thread
From: Hannes Reinecke @ 2021-05-04 13:12 UTC (permalink / raw)
To: John Garry, Bart Van Assche, Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, linux-scsi
On 5/4/21 12:55 PM, John Garry wrote:
> On 04/05/2021 07:17, Hannes Reinecke wrote:
>> On 5/4/21 5:20 AM, Bart Van Assche wrote:
>>> On 5/3/21 8:03 AM, Hannes Reinecke wrote:
>>>> These commands are set aside before allocating the block-mq tag bitmap,
>>>> so they'll never show up as busy in the tag map.
>>>
>>> That doesn't sound correct to me. Should the above perhaps be changed
>>> into "blk_mq_start_request() is never called for internal commands so
>>> they'll never show up as busy in the tag map"?
>>>
>> Yes, will do.
>
> So why don't these - or shouldn't these - turn up in the busy tag map?
>
> One of the motivations to use these block requests for internal commands
> is that we can take advantage of the block layer handling for CPU
> hotplug for MQ hosts, i.e. if blk-mq can't see these are inflight, then
> they would be missed in blk_mq_hctx_notify_offline() ->
> blk_mq_hctx_has_requests(), right? And who knows what else...
>
Oh, but of course it's possible to call 'start' on these requests to
have them counted in the busy map.
I just didn't see the need for it until now, that's all.
Cheers,
Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
hare@suse.de +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 10/18] scsi: implement reserved command handling
2021-05-04 13:12 ` Hannes Reinecke
@ 2021-05-04 16:59 ` Bart Van Assche
2021-05-04 18:09 ` Hannes Reinecke
0 siblings, 1 reply; 55+ messages in thread
From: Bart Van Assche @ 2021-05-04 16:59 UTC (permalink / raw)
To: Hannes Reinecke, John Garry, Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, linux-scsi
On 5/4/21 6:12 AM, Hannes Reinecke wrote:
> On 5/4/21 12:55 PM, John Garry wrote:
>> On 04/05/2021 07:17, Hannes Reinecke wrote:
>>> On 5/4/21 5:20 AM, Bart Van Assche wrote:
>>>> On 5/3/21 8:03 AM, Hannes Reinecke wrote:
>>>>> These commands are set aside before allocating the block-mq tag
>>>>> bitmap,
>>>>> so they'll never show up as busy in the tag map.
>>>>
>>>> That doesn't sound correct to me. Should the above perhaps be changed
>>>> into "blk_mq_start_request() is never called for internal commands so
>>>> they'll never show up as busy in the tag map"?
>>>>
>>> Yes, will do.
>>
>> So why don't these - or shouldn't these - turn up in the busy tag map?
>>
>> One of the motivations to use these block requests for internal
>> commands is that we can take advantage of the block layer handling for
>> CPU hotplug for MQ hosts, i.e. if blk-mq can't see these are inflight,
>> then they would be missed in blk_mq_hctx_notify_offline() ->
>> blk_mq_hctx_has_requests(), right? And who knows what else...
>>
> Oh, but of course it's possible to call 'start' on these requests to
> have them counted in the busy map.
> I just didn't see the need for it until now, that's all.
This is possible but this will require careful review of at least the
following code paths such that nothing unexpected happens for internal
commands:
* The SCSI timeout code.
* All blk_mq_tagset_busy_iter() and scsi_host_busy_iter() callers. As an
example, scsi_host_busy() must not include LLD-internal commands.
Thanks,
Bart.
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 10/18] scsi: implement reserved command handling
2021-05-04 16:59 ` Bart Van Assche
@ 2021-05-04 18:09 ` Hannes Reinecke
0 siblings, 0 replies; 55+ messages in thread
From: Hannes Reinecke @ 2021-05-04 18:09 UTC (permalink / raw)
To: Bart Van Assche, John Garry, Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, linux-scsi
On 5/4/21 6:59 PM, Bart Van Assche wrote:
> On 5/4/21 6:12 AM, Hannes Reinecke wrote:
>> On 5/4/21 12:55 PM, John Garry wrote:
>>> On 04/05/2021 07:17, Hannes Reinecke wrote:
>>>> On 5/4/21 5:20 AM, Bart Van Assche wrote:
>>>>> On 5/3/21 8:03 AM, Hannes Reinecke wrote:
>>>>>> These commands are set aside before allocating the block-mq tag
>>>>>> bitmap,
>>>>>> so they'll never show up as busy in the tag map.
>>>>>
>>>>> That doesn't sound correct to me. Should the above perhaps be changed
>>>>> into "blk_mq_start_request() is never called for internal commands so
>>>>> they'll never show up as busy in the tag map"?
>>>>>
>>>> Yes, will do.
>>>
>>> So why don't these - or shouldn't these - turn up in the busy tag map?
>>>
>>> One of the motivations to use these block requests for internal
>>> commands is that we can take advantage of the block layer handling for
>>> CPU hotplug for MQ hosts, i.e. if blk-mq can't see these are inflight,
>>> then they would be missed in blk_mq_hctx_notify_offline() ->
>>> blk_mq_hctx_has_requests(), right? And who knows what else...
>>>
>> Oh, but of course it's possible to call 'start' on these requests to
>> have them counted in the busy map.
>> I just didn't see the need for it until now, that's all.
>
> This is possible but this will require careful review of at least the
> following code paths such that nothing unexpected happens for internal
> commands:
> * The SCSI timeout code.
> * All blk_mq_tagset_busy_iter() and scsi_host_busy_iter() callers. As an
> example, scsi_host_busy() must not include LLD-internal commands.
>
Oh, _that_ is easy. These are reserved commands, which will have the
last bool argument to the iter functions set to 'true'.
bool (*fn)(struct scsi_cmnd *, void *, bool)
So we just need to return from the iter if the last argument is true.
Cheers,
Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
hare@suse.de +49 911 74053 688
SUSE Software Solutions Germany GmbH, 90409 Nürnberg
GF: F. Imendörffer, HRB 36809 (AG Nürnberg)
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 10/18] scsi: implement reserved command handling
2021-05-03 15:03 ` [PATCH 10/18] scsi: implement reserved command handling Hannes Reinecke
2021-05-04 3:20 ` Bart Van Assche
@ 2021-05-05 0:45 ` Bart Van Assche
2021-05-05 5:56 ` Hannes Reinecke
1 sibling, 1 reply; 55+ messages in thread
From: Bart Van Assche @ 2021-05-05 0:45 UTC (permalink / raw)
To: Hannes Reinecke, Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, John Garry, linux-scsi
On 5/3/21 8:03 AM, Hannes Reinecke wrote:
> struct scsi_cmnd *scsi_get_internal_cmd(struct scsi_device *sdev,
> unsigned int op, blk_mq_req_flags_t flags)
> @@ -2005,6 +2009,10 @@ struct scsi_cmnd *scsi_get_internal_cmd(struct scsi_device *sdev,
>
> WARN_ON_ONCE(((op & REQ_OP_MASK) != REQ_OP_SCSI_IN) &&
> ((op & REQ_OP_MASK) != REQ_OP_SCSI_OUT));
> +
> + if (sdev->host->nr_reserved_cmds)
> + flags |= BLK_MQ_REQ_RESERVED;
> +
> rq = blk_mq_alloc_request(sdev->request_queue, op, flags);
> if (IS_ERR(rq))
> return NULL;
Can the if-statement be removed such that scsi_get_internal_cmd() fails
if sdev->host->nr_reserved_cmds == 0? I'm concerned that otherwise it
will be very hard to determine which requests are internal and which
ones not from inside a blk_mq_tagset_busy_iter() callback.
Thanks,
Bart.
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 10/18] scsi: implement reserved command handling
2021-05-05 0:45 ` Bart Van Assche
@ 2021-05-05 5:56 ` Hannes Reinecke
0 siblings, 0 replies; 55+ messages in thread
From: Hannes Reinecke @ 2021-05-05 5:56 UTC (permalink / raw)
To: Bart Van Assche, Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, John Garry, linux-scsi
On 5/5/21 2:45 AM, Bart Van Assche wrote:
> On 5/3/21 8:03 AM, Hannes Reinecke wrote:
>> struct scsi_cmnd *scsi_get_internal_cmd(struct scsi_device *sdev,
>> unsigned int op, blk_mq_req_flags_t flags)
>> @@ -2005,6 +2009,10 @@ struct scsi_cmnd *scsi_get_internal_cmd(struct scsi_device *sdev,
>>
>> WARN_ON_ONCE(((op & REQ_OP_MASK) != REQ_OP_SCSI_IN) &&
>> ((op & REQ_OP_MASK) != REQ_OP_SCSI_OUT));
>> +
>> + if (sdev->host->nr_reserved_cmds)
>> + flags |= BLK_MQ_REQ_RESERVED;
>> +
>> rq = blk_mq_alloc_request(sdev->request_queue, op, flags);
>> if (IS_ERR(rq))
>> return NULL;
>
> Can the if-statement be removed such that scsi_get_internal_cmd() fails
> if sdev->host->nr_reserved_cmds == 0? I'm concerned that otherwise it
> will be very hard to determine which requests are internal and which
> ones not from inside a blk_mq_tagset_busy_iter() callback.
>
Original idea was that one could use scsi_get_internal_cmd() even with
nr_reserved_cmds == 0, but you are right that this will probably just
lead to confusion.
Will be modifying it for the next round.
Cheers,
Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
hare@suse.de +49 911 74053 688
SUSE Software Solutions Germany GmbH, 90409 Nürnberg
GF: F. Imendörffer, HRB 36809 (AG Nürnberg)
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 03/18] scsi: add scsi_{get,put}_internal_cmd() helper
2021-05-04 6:12 ` Hannes Reinecke
@ 2021-06-23 10:57 ` John Garry
2021-06-23 13:48 ` Hannes Reinecke
0 siblings, 1 reply; 55+ messages in thread
From: John Garry @ 2021-06-23 10:57 UTC (permalink / raw)
To: Hannes Reinecke, Bart Van Assche, Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, linux-scsi
On 04/05/2021 07:12, Hannes Reinecke wrote:
> On 5/4/21 4:21 AM, Bart Van Assche wrote:
>> On 5/3/21 8:03 AM, Hannes Reinecke wrote:
>>> +/**
>>> + * scsi_get_internal_cmd - allocate an internal SCSI command
>>> + * @sdev: SCSI device from which to allocate the command
>>> + * @op: operation (REQ_OP_SCSI_IN or REQ_OP_SCSI_OUT)
>>> + * @flags: BLK_MQ_REQ_* flags, e.g. BLK_MQ_REQ_NOWAIT.
>>> + *
>>> + * Allocates a SCSI command for internal LLDD use.
>>> + */
>>> +struct scsi_cmnd *scsi_get_internal_cmd(struct scsi_device *sdev,
>>> + unsigned int op, blk_mq_req_flags_t flags)
>>> +{
>>> + struct request *rq;
>>> + struct scsi_cmnd *scmd;
>>> +
>>> + WARN_ON_ONCE(((op & REQ_OP_MASK) != REQ_OP_SCSI_IN) &&
>>> + ((op & REQ_OP_MASK) != REQ_OP_SCSI_OUT));
>>> + rq = blk_mq_alloc_request(sdev->request_queue, op, flags);
>>> + if (IS_ERR(rq))
>>> + return NULL;
>>> + scmd = blk_mq_rq_to_pdu(rq);
>>> + scmd->request = rq;
>>> + scmd->device = sdev;
>>> + return scmd;
>>> +}
>>> +EXPORT_SYMBOL_GPL(scsi_get_internal_cmd);
>>
>> Multiple fields that are initialized by the regular command submission
>> path are not initialized by the above function, e.g. scmd->tag. Has it
>> been considered to call scsi_init_command() instead of adding yet
>> another code path that initializes scmd->request?
>>
> Hmm. No, I don't think it's a good idea.
> Basic idea is that the SCSI request serves as a container for (non-SCSI)
> management commands, _and_ that they are submitted directly from within
> the driver, ie never ever enter ->queue_rq().
> As such we don't need an initialisation vie scsi_init_request(), and it
> would actually be counter-productive as we would be setting up fields
> which we'll never need anyway, and might need to tear down afterwards.
I will note that we also bypass the queue budgeting in
blk_mq_ops.{get,put}_budget. I figure that is not an issue...
BTW, any chance of a new version?
Thanks,
John
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 03/18] scsi: add scsi_{get,put}_internal_cmd() helper
2021-06-23 10:57 ` John Garry
@ 2021-06-23 13:48 ` Hannes Reinecke
2021-06-24 9:55 ` John Garry
0 siblings, 1 reply; 55+ messages in thread
From: Hannes Reinecke @ 2021-06-23 13:48 UTC (permalink / raw)
To: John Garry, Bart Van Assche, Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, linux-scsi
On 6/23/21 12:57 PM, John Garry wrote:
> On 04/05/2021 07:12, Hannes Reinecke wrote:
>> On 5/4/21 4:21 AM, Bart Van Assche wrote:
>>> On 5/3/21 8:03 AM, Hannes Reinecke wrote:
>>>> +/**
>>>> + * scsi_get_internal_cmd - allocate an internal SCSI command
>>>> + * @sdev: SCSI device from which to allocate the command
>>>> + * @op: operation (REQ_OP_SCSI_IN or REQ_OP_SCSI_OUT)
>>>> + * @flags: BLK_MQ_REQ_* flags, e.g. BLK_MQ_REQ_NOWAIT.
>>>> + *
>>>> + * Allocates a SCSI command for internal LLDD use.
>>>> + */
>>>> +struct scsi_cmnd *scsi_get_internal_cmd(struct scsi_device *sdev,
>>>> + unsigned int op, blk_mq_req_flags_t flags)
>>>> +{
>>>> + struct request *rq;
>>>> + struct scsi_cmnd *scmd;
>>>> +
>>>> + WARN_ON_ONCE(((op & REQ_OP_MASK) != REQ_OP_SCSI_IN) &&
>>>> + ((op & REQ_OP_MASK) != REQ_OP_SCSI_OUT));
>>>> + rq = blk_mq_alloc_request(sdev->request_queue, op, flags);
>>>> + if (IS_ERR(rq))
>>>> + return NULL;
>>>> + scmd = blk_mq_rq_to_pdu(rq);
>>>> + scmd->request = rq;
>>>> + scmd->device = sdev;
>>>> + return scmd;
>>>> +}
>>>> +EXPORT_SYMBOL_GPL(scsi_get_internal_cmd);
>>>
>>> Multiple fields that are initialized by the regular command submission
>>> path are not initialized by the above function, e.g. scmd->tag. Has it
>>> been considered to call scsi_init_command() instead of adding yet
>>> another code path that initializes scmd->request?
>>>
>> Hmm. No, I don't think it's a good idea.
>> Basic idea is that the SCSI request serves as a container for
>> (non-SCSI) management commands, _and_ that they are submitted directly
>> from within the driver, ie never ever enter ->queue_rq().
>> As such we don't need an initialisation vie scsi_init_request(), and
>> it would actually be counter-productive as we would be setting up
>> fields which we'll never need anyway, and might need to tear down
>> afterwards.
>
> I will note that we also bypass the queue budgeting in
> blk_mq_ops.{get,put}_budget. I figure that is not an issue...
>
> BTW, any chance of a new version?
>
I have _so_ no idea.
The review from Christoph to patch 07/18 he (apparently) changed his
mind for the current implementation of using scsi_get_host_dev(), citing
an approach I had been implemented for v2 (and which got changed due to
his reviews for v2).
So no I'm not sure if he retracted on his earlier review, or if he just
had forgotten about it.
And before I get clarification from him I can't really move forward, as
both reviews contradict each other.
Christoph?
Cheers,
Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
hare@suse.de +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 03/18] scsi: add scsi_{get,put}_internal_cmd() helper
2021-05-03 15:03 ` [PATCH 03/18] scsi: add scsi_{get,put}_internal_cmd() helper Hannes Reinecke
2021-05-04 2:21 ` Bart Van Assche
2021-05-04 9:53 ` Christoph Hellwig
@ 2021-06-23 16:09 ` Bart Van Assche
2 siblings, 0 replies; 55+ messages in thread
From: Bart Van Assche @ 2021-06-23 16:09 UTC (permalink / raw)
To: Hannes Reinecke, Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, John Garry, linux-scsi
On 5/3/21 8:03 AM, Hannes Reinecke wrote:
> +struct scsi_cmnd *scsi_get_internal_cmd(struct scsi_device *sdev,
> + unsigned int op, blk_mq_req_flags_t flags)
> +{
> + struct request *rq;
> + struct scsi_cmnd *scmd;
> +
> + WARN_ON_ONCE(((op & REQ_OP_MASK) != REQ_OP_SCSI_IN) &&
> + ((op & REQ_OP_MASK) != REQ_OP_SCSI_OUT));
Consider using blk_op_is_scsi() instead of open-coding it.
Thanks,
Bart.
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 03/18] scsi: add scsi_{get,put}_internal_cmd() helper
2021-06-23 13:48 ` Hannes Reinecke
@ 2021-06-24 9:55 ` John Garry
0 siblings, 0 replies; 55+ messages in thread
From: John Garry @ 2021-06-24 9:55 UTC (permalink / raw)
To: Hannes Reinecke, Bart Van Assche, Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, linux-scsi
On 23/06/2021 14:48, Hannes Reinecke wrote:
>>
>> I will note that we also bypass the queue budgeting in
>> blk_mq_ops.{get,put}_budget. I figure that is not an issue...
>>
>> BTW, any chance of a new version?
>>
> I have _so_ no idea.
>
> The review from Christoph to patch 07/18 he (apparently) changed his
> mind for the current implementation of using scsi_get_host_dev(), citing
> an approach I had been implemented for v2 (and which got changed due to
> his reviews for v2).
> So no I'm not sure if he retracted on his earlier review, or if he just
> had forgotten about it.
> And before I get clarification from him I can't really move forward, as
> both reviews contradict each other.
>
> Christoph?
I see.
FWIW, not using scsi_get_host_dev() means that internal command
scsi_cmnd.device cannot be set. But then many fields are not set there.
I suppose that if we delete scsi_get_host_dev() now [as it has no user],
then that will narrow the choice...
BTW, I was just checking the v7, and we have no queue sysfs folder for
that host scsi_device. Regardless of how the queue is created - v2 vs v7
method - I think that we should have that somehow.
Thanks,
John
^ permalink raw reply [flat|nested] 55+ messages in thread
end of thread, other threads:[~2021-06-24 10:01 UTC | newest]
Thread overview: 55+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-03 15:03 [PATCHv8 00/18] scsi: enabled reserved commands for LLDDs Hannes Reinecke
2021-05-03 15:03 ` [PATCH 01/18] fnic: kill 'exclude_id' argument to fnic_cleanup_io() Hannes Reinecke
2021-05-04 2:12 ` Bart Van Assche
2021-05-04 9:49 ` Christoph Hellwig
2021-05-03 15:03 ` [PATCH 02/18] fnic: use scsi_host_busy_iter() to traverse commands Hannes Reinecke
2021-05-04 9:50 ` Christoph Hellwig
2021-05-04 12:53 ` Hannes Reinecke
2021-05-03 15:03 ` [PATCH 03/18] scsi: add scsi_{get,put}_internal_cmd() helper Hannes Reinecke
2021-05-04 2:21 ` Bart Van Assche
2021-05-04 6:12 ` Hannes Reinecke
2021-06-23 10:57 ` John Garry
2021-06-23 13:48 ` Hannes Reinecke
2021-06-24 9:55 ` John Garry
2021-05-04 9:53 ` Christoph Hellwig
2021-05-04 12:54 ` Hannes Reinecke
2021-06-23 16:09 ` Bart Van Assche
2021-05-03 15:03 ` [PATCH 04/18] fnic: use internal commands Hannes Reinecke
2021-05-04 2:25 ` Bart Van Assche
2021-05-04 6:12 ` Hannes Reinecke
2021-05-03 15:03 ` [PATCH 05/18] scsi: use real inquiry data when initialising devices Hannes Reinecke
2021-05-04 2:28 ` Bart Van Assche
2021-05-04 9:55 ` Christoph Hellwig
2021-05-04 12:57 ` Hannes Reinecke
2021-05-03 15:03 ` [PATCH 06/18] scsi: Use dummy inquiry data for the host device Hannes Reinecke
2021-05-04 2:52 ` Bart Van Assche
2021-05-04 6:14 ` Hannes Reinecke
2021-05-03 15:03 ` [PATCH 07/18] scsi: revamp host device handling Hannes Reinecke
2021-05-04 3:06 ` Bart Van Assche
2021-05-04 6:15 ` Hannes Reinecke
2021-05-04 9:59 ` Christoph Hellwig
2021-05-04 13:10 ` Hannes Reinecke
2021-05-03 15:03 ` [PATCH 08/18] snic: use reserved commands Hannes Reinecke
2021-05-04 3:12 ` Bart Van Assche
2021-05-04 6:16 ` Hannes Reinecke
2021-05-03 15:03 ` [PATCH 09/18] snic: use tagset iter for traversing commands Hannes Reinecke
2021-05-04 9:59 ` Christoph Hellwig
2021-05-03 15:03 ` [PATCH 10/18] scsi: implement reserved command handling Hannes Reinecke
2021-05-04 3:20 ` Bart Van Assche
2021-05-04 6:17 ` Hannes Reinecke
2021-05-04 10:55 ` John Garry
2021-05-04 13:12 ` Hannes Reinecke
2021-05-04 16:59 ` Bart Van Assche
2021-05-04 18:09 ` Hannes Reinecke
2021-05-05 0:45 ` Bart Van Assche
2021-05-05 5:56 ` Hannes Reinecke
2021-05-03 15:03 ` [PATCH 11/18] hpsa: move hpsa_hba_inquiry after scsi_add_host() Hannes Reinecke
2021-05-03 15:03 ` [PATCH 12/18] hpsa: use reserved commands Hannes Reinecke
2021-05-03 15:03 ` [PATCH 13/18] hpsa: use scsi_host_busy_iter() to traverse outstanding commands Hannes Reinecke
2021-05-03 15:03 ` [PATCH 14/18] hpsa: drop refcount field from CommandList Hannes Reinecke
2021-05-03 15:03 ` [PATCH 15/18] aacraid: move scsi_add_host() Hannes Reinecke
2021-05-03 15:03 ` [PATCH 16/18] aacraid: store target id in host_scribble Hannes Reinecke
2021-05-04 3:22 ` Bart Van Assche
2021-05-04 6:18 ` Hannes Reinecke
2021-05-03 15:03 ` [PATCH 17/18] aacraid: use scsi_get_internal_cmd() Hannes Reinecke
2021-05-03 15:03 ` [PATCH 18/18] aacraid: use scsi_host_busy_iter() to traverse outstanding commands Hannes Reinecke
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.