This is a follow on to Part I in the following:
https://lore.kernel.org/linux-scsi/[email protected]/T/#ta
This mostly focuses on libata changes to queue internal commands as
requests.
This is less complete than Part I series, due to:
- not tested on SATA PMP
- not support for ipr.c, which does not
support ata_port_operations.error_handler
- Not tested enough - for example, there are prob lots of issues lurking
in libata qc iter functions now that ata_port.qcmd[] is deleted
John Garry (7):
ata: libata-scsi: Add ata_scsi_queue_internal()
ata: libata-scsi: Add ata_internal_queuecommand()
ata: libata: Make space for ATA queue command in scmd payload
ata: libata: Add ata_internal_timeout()
ata: libata: Queue ATA internal commands as requests
scsi: mvsas: Remove internal tag handling
scsi: hisi_sas: Remove internal tag handling for reserved commands
drivers/ata/libata-core.c | 141 ++++++++++++++-----------
drivers/ata/libata-eh.c | 11 +-
drivers/ata/libata-sata.c | 5 +-
drivers/ata/libata-scsi.c | 76 ++++++++++++-
drivers/ata/libata.h | 3 +-
drivers/scsi/aic94xx/aic94xx_init.c | 2 +
drivers/scsi/hisi_sas/hisi_sas.h | 3 -
drivers/scsi/hisi_sas/hisi_sas_main.c | 82 +++-----------
drivers/scsi/hisi_sas/hisi_sas_v1_hw.c | 2 +
drivers/scsi/hisi_sas/hisi_sas_v2_hw.c | 2 +
drivers/scsi/hisi_sas/hisi_sas_v3_hw.c | 11 +-
drivers/scsi/isci/init.c | 2 +
drivers/scsi/libsas/sas_scsi_host.c | 20 +++-
drivers/scsi/mvsas/mv_init.c | 13 +--
drivers/scsi/mvsas/mv_sas.c | 55 +---------
drivers/scsi/mvsas/mv_sas.h | 1 -
drivers/scsi/pm8001/pm8001_init.c | 2 +
include/linux/libata.h | 64 ++++++++++-
include/scsi/libsas.h | 8 +-
19 files changed, 281 insertions(+), 222 deletions(-)
--
2.35.3
Add a function to handle queued ATA internal SCSI cmnds - does much the
same as ata_exec_internal_sg() does (which will be fixed up later to
actually queue internal cmnds through this function).
Signed-off-by: John Garry <[email protected]>
---
drivers/ata/libata-sata.c | 3 +++
drivers/ata/libata-scsi.c | 43 +++++++++++++++++++++++++++++++++++++++
drivers/ata/libata.h | 3 ++-
include/linux/libata.h | 6 ++++++
4 files changed, 54 insertions(+), 1 deletion(-)
diff --git a/drivers/ata/libata-sata.c b/drivers/ata/libata-sata.c
index b6806d41a8c5..e8b828c56542 100644
--- a/drivers/ata/libata-sata.c
+++ b/drivers/ata/libata-sata.c
@@ -1258,6 +1258,9 @@ int ata_sas_queuecmd(struct scsi_cmnd *cmd, struct ata_port *ap)
{
int rc = 0;
+ if (blk_mq_is_reserved_rq(scsi_cmd_to_rq(cmd)))
+ return ata_scsi_queue_internal(cmd, ap->link.device);
+
if (likely(ata_dev_enabled(ap->link.device)))
rc = __ata_scsi_queuecmd(cmd, ap->link.device);
else {
diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 476e0ef4bd29..30d7c90b0c35 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -3965,6 +3965,49 @@ static inline ata_xlat_func_t ata_get_xlat_func(struct ata_device *dev, u8 cmd)
return NULL;
}
+unsigned int ata_scsi_queue_internal(struct scsi_cmnd *scmd,
+ struct ata_device *dev)
+{
+ struct ata_link *link = dev->link;
+ struct ata_port *ap = link->ap;
+ struct ata_queued_cmd *qc;
+
+ /* no internal command while frozen */
+ if (ap->pflags & ATA_PFLAG_FROZEN)
+ goto did_err;
+
+ /* initialize internal qc */
+ qc = __ata_qc_from_tag(ap, ATA_TAG_INTERNAL);
+ link->preempted_tag = link->active_tag;
+ link->preempted_sactive = link->sactive;
+ ap->preempted_qc_active = ap->qc_active;
+ ap->preempted_nr_active_links = ap->nr_active_links;
+ link->active_tag = ATA_TAG_POISON;
+ link->sactive = 0;
+ ap->qc_active = 0;
+ ap->nr_active_links = 0;
+
+ if (qc->dma_dir != DMA_NONE) {
+ int n_elem;
+
+ n_elem = 1;
+ qc->n_elem = n_elem;
+ qc->sg = scsi_sglist(scmd);
+ qc->nbytes = qc->sg->length;
+ ata_sg_init(qc, qc->sg, n_elem);
+ }
+
+ scmd->submitter = SUBMITTED_BY_BLOCK_LAYER;
+
+ ata_qc_issue(qc);
+
+ return 0;
+did_err:
+ scmd->result = (DID_ERROR << 16);
+ scsi_done(scmd);
+ return 0;
+}
+
int __ata_scsi_queuecmd(struct scsi_cmnd *scmd, struct ata_device *dev)
{
u8 scsi_op = scmd->cmnd[0];
diff --git a/drivers/ata/libata.h b/drivers/ata/libata.h
index 0c2df1e60768..15cd1cd618b8 100644
--- a/drivers/ata/libata.h
+++ b/drivers/ata/libata.h
@@ -82,7 +82,6 @@ extern int ata_port_probe(struct ata_port *ap);
extern void __ata_port_probe(struct ata_port *ap);
extern unsigned int ata_read_log_page(struct ata_device *dev, u8 log,
u8 page, void *buf, unsigned int sectors);
-
#define to_ata_port(d) container_of(d, struct ata_port, tdev)
/* libata-acpi.c */
@@ -130,6 +129,8 @@ extern int ata_scsi_user_scan(struct Scsi_Host *shost, unsigned int channel,
void ata_scsi_sdev_config(struct scsi_device *sdev);
int ata_scsi_dev_config(struct scsi_device *sdev, struct ata_device *dev);
int __ata_scsi_queuecmd(struct scsi_cmnd *scmd, struct ata_device *dev);
+unsigned int ata_scsi_queue_internal(struct scsi_cmnd *scmd,
+ struct ata_device *dev);
/* libata-eh.c */
extern unsigned int ata_internal_cmd_timeout(struct ata_device *dev, u8 cmd);
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 827d5838cd23..8938b584520f 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -764,7 +764,9 @@ struct ata_link {
struct device tdev;
unsigned int active_tag; /* active tag on this link */
+ unsigned int preempted_tag;
u32 sactive; /* active NCQ commands */
+ u32 preempted_sactive;
unsigned int flags; /* ATA_LFLAG_xxx */
@@ -857,6 +859,10 @@ struct ata_port {
#ifdef CONFIG_ATA_ACPI
struct ata_acpi_gtm __acpi_init_gtm; /* use ata_acpi_init_gtm() */
#endif
+
+ u64 preempted_qc_active;
+ int preempted_nr_active_links;
+
/* owned by EH */
u8 sector_buf[ATA_SECT_SIZE] ____cacheline_aligned;
};
--
2.35.3
Now that any sas_task which we're sent has a request associated, we can
use the request tag for slot IPTT.
However, since v2 HW has its own slot IPTT allocation scheme due to badly
broken HW, continue to use it.
Signed-off-by: John Garry <[email protected]>
---
drivers/scsi/hisi_sas/hisi_sas.h | 3 -
drivers/scsi/hisi_sas/hisi_sas_main.c | 82 +++++---------------------
drivers/scsi/hisi_sas/hisi_sas_v3_hw.c | 9 +--
3 files changed, 17 insertions(+), 77 deletions(-)
diff --git a/drivers/scsi/hisi_sas/hisi_sas.h b/drivers/scsi/hisi_sas/hisi_sas.h
index 6f8a52a1b808..8cd238f75066 100644
--- a/drivers/scsi/hisi_sas/hisi_sas.h
+++ b/drivers/scsi/hisi_sas/hisi_sas.h
@@ -39,9 +39,6 @@
#define HISI_SAS_PM_BIT 2
#define HISI_SAS_HW_FAULT_BIT 3
#define HISI_SAS_MAX_COMMANDS (HISI_SAS_QUEUE_SLOTS)
-#define HISI_SAS_RESERVED_IPTT 96
-#define HISI_SAS_UNRESERVED_IPTT \
- (HISI_SAS_MAX_COMMANDS - HISI_SAS_RESERVED_IPTT)
#define HISI_SAS_IOST_ITCT_CACHE_NUM 64
#define HISI_SAS_IOST_ITCT_CACHE_DW_SZ 10
diff --git a/drivers/scsi/hisi_sas/hisi_sas_main.c b/drivers/scsi/hisi_sas/hisi_sas_main.c
index 65475775c844..7f784cdacf9f 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_main.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_main.c
@@ -161,49 +161,13 @@ static void hisi_sas_slot_index_clear(struct hisi_hba *hisi_hba, int slot_idx)
static void hisi_sas_slot_index_free(struct hisi_hba *hisi_hba, int slot_idx)
{
- if (hisi_hba->hw->slot_index_alloc ||
- slot_idx >= HISI_SAS_UNRESERVED_IPTT) {
+ if (hisi_hba->hw->slot_index_alloc) {
spin_lock(&hisi_hba->lock);
hisi_sas_slot_index_clear(hisi_hba, slot_idx);
spin_unlock(&hisi_hba->lock);
}
}
-static void hisi_sas_slot_index_set(struct hisi_hba *hisi_hba, int slot_idx)
-{
- void *bitmap = hisi_hba->slot_index_tags;
-
- __set_bit(slot_idx, bitmap);
-}
-
-static int hisi_sas_slot_index_alloc(struct hisi_hba *hisi_hba,
- struct request *rq)
-{
- int index;
- void *bitmap = hisi_hba->slot_index_tags;
-
- if (rq)
- return rq->tag + HISI_SAS_RESERVED_IPTT;
-
- spin_lock(&hisi_hba->lock);
- index = find_next_zero_bit(bitmap, HISI_SAS_RESERVED_IPTT,
- hisi_hba->last_slot_index + 1);
- if (index >= HISI_SAS_RESERVED_IPTT) {
- index = find_next_zero_bit(bitmap,
- HISI_SAS_RESERVED_IPTT,
- 0);
- if (index >= HISI_SAS_RESERVED_IPTT) {
- spin_unlock(&hisi_hba->lock);
- return -SAS_QUEUE_FULL;
- }
- }
- hisi_sas_slot_index_set(hisi_hba, index);
- hisi_hba->last_slot_index = index;
- spin_unlock(&hisi_hba->lock);
-
- return index;
-}
-
void hisi_sas_slot_task_free(struct hisi_hba *hisi_hba, struct sas_task *task,
struct hisi_sas_slot *slot)
{
@@ -465,8 +429,10 @@ static int hisi_sas_queue_command(struct sas_task *task, gfp_t gfp_flags)
struct hisi_sas_port *port;
struct hisi_hba *hisi_hba;
struct hisi_sas_slot *slot;
+ unsigned int dq_index;
struct request *rq;
struct device *dev;
+ u32 blk_tag;
int rc;
if (!sas_port) {
@@ -486,20 +452,9 @@ static int hisi_sas_queue_command(struct sas_task *task, gfp_t gfp_flags)
hisi_hba = dev_to_hisi_hba(device);
dev = hisi_hba->dev;
rq = sas_task_find_rq(task);
- if (rq) {
- unsigned int dq_index;
- u32 blk_tag;
-
- blk_tag = blk_mq_unique_tag(rq);
- dq_index = blk_mq_unique_tag_to_hwq(blk_tag);
- dq = &hisi_hba->dq[dq_index];
- } else {
- struct Scsi_Host *shost = hisi_hba->shost;
- struct blk_mq_queue_map *qmap = &shost->tag_set.map[HCTX_TYPE_DEFAULT];
- int queue = qmap->mq_map[raw_smp_processor_id()];
-
- dq = &hisi_hba->dq[queue];
- }
+ blk_tag = blk_mq_unique_tag(rq);
+ dq_index = blk_mq_unique_tag_to_hwq(blk_tag);
+ dq = &hisi_hba->dq[dq_index];
switch (task->task_proto) {
case SAS_PROTOCOL_SSP:
@@ -563,13 +518,13 @@ static int hisi_sas_queue_command(struct sas_task *task, gfp_t gfp_flags)
goto err_out_dma_unmap;
}
- if (!internal_abort && hisi_hba->hw->slot_index_alloc)
+ if (hisi_hba->hw->slot_index_alloc) {
rc = hisi_hba->hw->slot_index_alloc(hisi_hba, device);
- else
- rc = hisi_sas_slot_index_alloc(hisi_hba, rq);
-
- if (rc < 0)
- goto err_out_dif_dma_unmap;
+ if (rc < 0)
+ goto err_out_dif_dma_unmap;
+ } else {
+ rc = rq->tag;
+ }
slot = &hisi_hba->slot_info[rc];
slot->n_elem = n_elem;
@@ -2434,17 +2389,8 @@ int hisi_sas_probe(struct platform_device *pdev,
shost->max_lun = ~0;
shost->max_channel = 1;
shost->max_cmd_len = 16;
- if (hisi_hba->hw->slot_index_alloc) {
- shost->can_queue = HISI_SAS_MAX_COMMANDS;
- shost->cmd_per_lun = HISI_SAS_MAX_COMMANDS;
- } else {
- /*
- * Intentionally use HISI_SAS_UNRESERVED_IPTT for .can_queue until
- * every sas_task we're sent has a request associated.
- */
- shost->can_queue = HISI_SAS_UNRESERVED_IPTT;
- shost->cmd_per_lun = HISI_SAS_UNRESERVED_IPTT;
- }
+ shost->can_queue = HISI_SAS_MAX_COMMANDS;
+ shost->cmd_per_lun = HISI_SAS_MAX_COMMANDS;
sha->sas_ha_name = DRV_NAME;
sha->dev = hisi_hba->dev;
diff --git a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
index 4caf07306b24..c7963ae8ad50 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
@@ -4862,12 +4862,9 @@ hisi_sas_v3_probe(struct pci_dev *pdev, const struct pci_device_id *id)
shost->max_lun = ~0;
shost->max_channel = 1;
shost->max_cmd_len = 16;
- /*
- * Intentionally use HISI_SAS_UNRESERVED_IPTT for .can_queue until
- * every sas_task we're sent has a request associated.
- */
- shost->can_queue = HISI_SAS_UNRESERVED_IPTT;
- shost->cmd_per_lun = HISI_SAS_UNRESERVED_IPTT;
+
+ shost->can_queue = HISI_SAS_MAX_COMMANDS;
+ shost->cmd_per_lun = HISI_SAS_MAX_COMMANDS;
sha->sas_ha_name = DRV_NAME;
sha->dev = dev;
--
2.35.3
Now that any sas_task which we're sent has a request associated, delete
internal tag management.
Signed-off-by: John Garry <[email protected]>
---
drivers/scsi/mvsas/mv_init.c | 11 --------
drivers/scsi/mvsas/mv_sas.c | 55 ++----------------------------------
drivers/scsi/mvsas/mv_sas.h | 1 -
3 files changed, 2 insertions(+), 65 deletions(-)
diff --git a/drivers/scsi/mvsas/mv_init.c b/drivers/scsi/mvsas/mv_init.c
index e1b6cc196cef..9cb8d5b315db 100644
--- a/drivers/scsi/mvsas/mv_init.c
+++ b/drivers/scsi/mvsas/mv_init.c
@@ -147,7 +147,6 @@ static void mvs_free(struct mvs_info *mvi)
scsi_host_put(mvi->shost);
list_for_each_entry(mwq, &mvi->wq_list, entry)
cancel_delayed_work(&mwq->work_q);
- kfree(mvi->rsvd_tags);
kfree(mvi);
}
@@ -371,10 +370,6 @@ static struct mvs_info *mvs_pci_alloc(struct pci_dev *pdev,
mvi->sas = sha;
mvi->shost = shost;
- mvi->rsvd_tags = bitmap_zalloc(MVS_RSVD_SLOTS, GFP_KERNEL);
- if (!mvi->rsvd_tags)
- goto err_out;
-
if (MVS_CHIP_DISP->chip_ioremap(mvi))
goto err_out;
if (!mvs_alloc(mvi, shost))
@@ -473,12 +468,6 @@ static void mvs_post_sas_ha_init(struct Scsi_Host *shost,
else
can_queue = MVS_CHIP_SLOT_SZ;
- /*
- * Carve out MVS_RSVD_SLOTS slots internally until every sas_task we're sent
- * has a request associated.
- */
- can_queue -= MVS_RSVD_SLOTS;
-
shost->sg_tablesize = min_t(u16, SG_ALL, MVS_MAX_SG);
shost->can_queue = can_queue;
mvi->shost->cmd_per_lun = MVS_QUEUE_SIZE;
diff --git a/drivers/scsi/mvsas/mv_sas.c b/drivers/scsi/mvsas/mv_sas.c
index 9978c424214c..3c92fc48680d 100644
--- a/drivers/scsi/mvsas/mv_sas.c
+++ b/drivers/scsi/mvsas/mv_sas.c
@@ -20,40 +20,6 @@ static int mvs_find_tag(struct mvs_info *mvi, struct sas_task *task, u32 *tag)
return 0;
}
-static void mvs_tag_clear(struct mvs_info *mvi, u32 tag)
-{
- void *bitmap = mvi->rsvd_tags;
- clear_bit(tag, bitmap);
-}
-
-static void mvs_tag_free(struct mvs_info *mvi, u32 tag)
-{
- if (tag >= MVS_RSVD_SLOTS)
- return;
-
- mvs_tag_clear(mvi, tag);
-}
-
-static void mvs_tag_set(struct mvs_info *mvi, unsigned int tag)
-{
- void *bitmap = mvi->rsvd_tags;
- set_bit(tag, bitmap);
-}
-
-static int mvs_tag_alloc(struct mvs_info *mvi, u32 *tag_out)
-{
- unsigned int index, tag;
- void *bitmap = mvi->rsvd_tags;
-
- index = find_first_zero_bit(bitmap, MVS_RSVD_SLOTS);
- tag = index;
- if (tag >= MVS_RSVD_SLOTS)
- return -SAS_QUEUE_FULL;
- mvs_tag_set(mvi, tag);
- *tag_out = tag;
- return 0;
-}
-
static struct mvs_info *mvs_find_dev_mvi(struct domain_device *dev)
{
unsigned long i = 0, j = 0, hi = 0;
@@ -765,13 +731,7 @@ static int mvs_task_prep(struct sas_task *task, struct mvs_info *mvi, int is_tmf
}
rq = sas_task_find_rq(task);
- if (rq) {
- tag = rq->tag + MVS_RSVD_SLOTS;
- } else {
- rc = mvs_tag_alloc(mvi, &tag);
- if (rc)
- goto err_out;
- }
+ tag = rq->tag;
slot = &mvi->slot_info[tag];
@@ -782,7 +742,7 @@ static int mvs_task_prep(struct sas_task *task, struct mvs_info *mvi, int is_tmf
slot->buf = dma_pool_zalloc(mvi->dma_pool, GFP_ATOMIC, &slot->buf_dma);
if (!slot->buf) {
rc = -ENOMEM;
- goto err_out_tag;
+ goto err_out;
}
tei.task = task;
@@ -826,8 +786,6 @@ static int mvs_task_prep(struct sas_task *task, struct mvs_info *mvi, int is_tmf
err_out_slot_buf:
dma_pool_free(mvi->dma_pool, slot->buf, slot->buf_dma);
-err_out_tag:
- mvs_tag_free(mvi, tag);
err_out:
dev_printk(KERN_ERR, mvi->dev, "mvsas prep failed[%d]!\n", rc);
@@ -863,12 +821,6 @@ int mvs_queue_command(struct sas_task *task, gfp_t gfp_flags)
return rc;
}
-static void mvs_slot_free(struct mvs_info *mvi, u32 rx_desc)
-{
- u32 slot_idx = rx_desc & RXQ_SLOT_MASK;
- mvs_tag_free(mvi, slot_idx);
-}
-
static void mvs_slot_task_free(struct mvs_info *mvi, struct sas_task *task,
struct mvs_slot_info *slot, u32 slot_idx)
{
@@ -906,7 +858,6 @@ static void mvs_slot_task_free(struct mvs_info *mvi, struct sas_task *task,
slot->task = NULL;
slot->port = NULL;
slot->slot_tag = 0xFFFFFFFF;
- mvs_slot_free(mvi, slot_idx);
}
static void mvs_update_wideport(struct mvs_info *mvi, int phy_no)
@@ -1913,8 +1864,6 @@ int mvs_int_rx(struct mvs_info *mvi, bool self_clear)
} else if (rx_desc & RXQ_ERR) {
if (!(rx_desc & RXQ_DONE))
mvs_slot_complete(mvi, rx_desc, 0);
- } else if (rx_desc & RXQ_SLOT_RESET) {
- mvs_slot_free(mvi, rx_desc);
}
}
diff --git a/drivers/scsi/mvsas/mv_sas.h b/drivers/scsi/mvsas/mv_sas.h
index 68df771e2975..bc3b5711fc07 100644
--- a/drivers/scsi/mvsas/mv_sas.h
+++ b/drivers/scsi/mvsas/mv_sas.h
@@ -370,7 +370,6 @@ struct mvs_info {
u32 chip_id;
const struct mvs_chip_info *chip;
- unsigned long *rsvd_tags;
/* further per-slot information */
struct mvs_phy phy[MVS_MAX_PHYS];
struct mvs_port port[MVS_MAX_PHYS];
--
2.35.3
Add a internal command timeout handler. Also hook into libsas timeout
handler.
Signed-off-by: John Garry <[email protected]>
---
drivers/ata/libata-scsi.c | 10 ++++++++++
drivers/scsi/libsas/sas_scsi_host.c | 5 +++++
include/linux/libata.h | 2 ++
3 files changed, 17 insertions(+)
diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 18c60addf943..cbf266c9d4c2 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -1138,6 +1138,16 @@ int ata_internal_queuecommand(struct Scsi_Host *shost, struct scsi_cmnd *scmd)
}
EXPORT_SYMBOL_GPL(ata_internal_queuecommand);
+enum blk_eh_timer_return ata_internal_timeout(struct scsi_cmnd *scmd)
+{
+ struct request *rq = blk_mq_rq_from_pdu(scmd);
+ struct completion *wait = rq->end_io_data;
+
+ complete(wait);
+ return BLK_EH_DONE;
+}
+EXPORT_SYMBOL_GPL(ata_internal_timeout);
+
/**
* ata_scsi_slave_config - Set SCSI device attributes
* @sdev: SCSI device to examine
diff --git a/drivers/scsi/libsas/sas_scsi_host.c b/drivers/scsi/libsas/sas_scsi_host.c
index 4fdd84868ac2..9d2122e65fba 100644
--- a/drivers/scsi/libsas/sas_scsi_host.c
+++ b/drivers/scsi/libsas/sas_scsi_host.c
@@ -921,10 +921,15 @@ void sas_task_complete_internal(struct sas_task *task)
enum blk_eh_timer_return sas_internal_timeout(struct scsi_cmnd *scmd)
{
+ struct domain_device *dev = cmd_to_domain_dev(scmd);
struct sas_task *task = TO_SAS_TASK(scmd);
bool is_completed = true;
unsigned long flags;
+ /* Handle libata internal command */
+ if (dev_is_sata(dev) && !task->slow_task)
+ return ata_internal_timeout(scmd);
+
spin_lock_irqsave(&task->task_state_lock, flags);
if (!(task->task_state_flags & SAS_TASK_STATE_DONE)) {
task->task_state_flags |= SAS_TASK_STATE_ABORTED;
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 1a03c7fbb4e6..d5a15d1a5c4d 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -1143,6 +1143,7 @@ extern void ata_scsi_unlock_native_capacity(struct scsi_device *sdev);
extern int ata_scsi_slave_config(struct scsi_device *sdev);
extern int ata_internal_queuecommand(struct Scsi_Host *shost,
struct scsi_cmnd *scmd);
+extern enum blk_eh_timer_return ata_internal_timeout(struct scsi_cmnd *scmd);
extern int ata_init_cmd_priv(struct Scsi_Host *shost, struct scsi_cmnd *cmd);
extern void ata_scsi_slave_destroy(struct scsi_device *sdev);
extern int ata_scsi_change_queue_depth(struct scsi_device *sdev,
@@ -1397,6 +1398,7 @@ extern const struct attribute_group *ata_common_sdev_groups[];
.max_sectors = ATA_MAX_SECTORS_LBA48,\
.reserved_queuecommand = ata_internal_queuecommand,\
.cmd_size = sizeof(struct ata_queued_cmd),\
+ .reserved_timedout = ata_internal_timeout,\
.init_cmd_priv = ata_init_cmd_priv
#define ATA_SUBBASE_SHT(drv_name) \
--
2.35.3
On 10/25/22 19:32, John Garry wrote:
> Add a function to handle queued ATA internal SCSI cmnds - does much the
> same as ata_exec_internal_sg() does (which will be fixed up later to
> actually queue internal cmnds through this function).
>
> Signed-off-by: John Garry <[email protected]>
> ---
> drivers/ata/libata-sata.c | 3 +++
> drivers/ata/libata-scsi.c | 43 +++++++++++++++++++++++++++++++++++++++
> drivers/ata/libata.h | 3 ++-
> include/linux/libata.h | 6 ++++++
> 4 files changed, 54 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/ata/libata-sata.c b/drivers/ata/libata-sata.c
> index b6806d41a8c5..e8b828c56542 100644
> --- a/drivers/ata/libata-sata.c
> +++ b/drivers/ata/libata-sata.c
> @@ -1258,6 +1258,9 @@ int ata_sas_queuecmd(struct scsi_cmnd *cmd, struct ata_port *ap)
> {
> int rc = 0;
>
> + if (blk_mq_is_reserved_rq(scsi_cmd_to_rq(cmd)))
> + return ata_scsi_queue_internal(cmd, ap->link.device);
> +
> if (likely(ata_dev_enabled(ap->link.device)))
> rc = __ata_scsi_queuecmd(cmd, ap->link.device);
> else {
> diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
> index 476e0ef4bd29..30d7c90b0c35 100644
> --- a/drivers/ata/libata-scsi.c
> +++ b/drivers/ata/libata-scsi.c
> @@ -3965,6 +3965,49 @@ static inline ata_xlat_func_t ata_get_xlat_func(struct ata_device *dev, u8 cmd)
> return NULL;
> }
>
> +unsigned int ata_scsi_queue_internal(struct scsi_cmnd *scmd,
> + struct ata_device *dev)
> +{
> + struct ata_link *link = dev->link;
> + struct ata_port *ap = link->ap;
> + struct ata_queued_cmd *qc;
> +
> + /* no internal command while frozen */
> + if (ap->pflags & ATA_PFLAG_FROZEN)
> + goto did_err;
> +
> + /* initialize internal qc */
> + qc = __ata_qc_from_tag(ap, ATA_TAG_INTERNAL);
> + link->preempted_tag = link->active_tag;
> + link->preempted_sactive = link->sactive;
> + ap->preempted_qc_active = ap->qc_active;
> + ap->preempted_nr_active_links = ap->nr_active_links;
> + link->active_tag = ATA_TAG_POISON;
> + link->sactive = 0;
> + ap->qc_active = 0;
> + ap->nr_active_links = 0;
> +
> + if (qc->dma_dir != DMA_NONE) {
> + int n_elem;
> +
> + n_elem = 1;
> + qc->n_elem = n_elem;
> + qc->sg = scsi_sglist(scmd);
> + qc->nbytes = qc->sg->length;
> + ata_sg_init(qc, qc->sg, n_elem);
> + }
> +
> + scmd->submitter = SUBMITTED_BY_BLOCK_LAYER;
> +
> + ata_qc_issue(qc);
Arg, no ! This potentially mixes NCQ and non-NCQ commands, which is
forbidden by ATA spec. You need to use something like:
if (ap->ops->qc_defer) {
if ((rc = ap->ops->qc_defer(qc)))
goto defer;
}
ata_qc_issue(qc);
which is done in __ata_scsi_queuecmd() -> ata_scsi_translate()
Unless you guarantee that ata_scsi_queue_internal() is always called
from libata EH context ?
> +
> + return 0;
> +did_err:
> + scmd->result = (DID_ERROR << 16);
> + scsi_done(scmd);
> + return 0;
> +}
> +
> int __ata_scsi_queuecmd(struct scsi_cmnd *scmd, struct ata_device *dev)
> {
> u8 scsi_op = scmd->cmnd[0];
> diff --git a/drivers/ata/libata.h b/drivers/ata/libata.h
> index 0c2df1e60768..15cd1cd618b8 100644
> --- a/drivers/ata/libata.h
> +++ b/drivers/ata/libata.h
> @@ -82,7 +82,6 @@ extern int ata_port_probe(struct ata_port *ap);
> extern void __ata_port_probe(struct ata_port *ap);
> extern unsigned int ata_read_log_page(struct ata_device *dev, u8 log,
> u8 page, void *buf, unsigned int sectors);
> -
> #define to_ata_port(d) container_of(d, struct ata_port, tdev)
>
> /* libata-acpi.c */
> @@ -130,6 +129,8 @@ extern int ata_scsi_user_scan(struct Scsi_Host *shost, unsigned int channel,
> void ata_scsi_sdev_config(struct scsi_device *sdev);
> int ata_scsi_dev_config(struct scsi_device *sdev, struct ata_device *dev);
> int __ata_scsi_queuecmd(struct scsi_cmnd *scmd, struct ata_device *dev);
> +unsigned int ata_scsi_queue_internal(struct scsi_cmnd *scmd,
> + struct ata_device *dev);
>
> /* libata-eh.c */
> extern unsigned int ata_internal_cmd_timeout(struct ata_device *dev, u8 cmd);
> diff --git a/include/linux/libata.h b/include/linux/libata.h
> index 827d5838cd23..8938b584520f 100644
> --- a/include/linux/libata.h
> +++ b/include/linux/libata.h
> @@ -764,7 +764,9 @@ struct ata_link {
>
> struct device tdev;
> unsigned int active_tag; /* active tag on this link */
> + unsigned int preempted_tag;
> u32 sactive; /* active NCQ commands */
> + u32 preempted_sactive;
>
> unsigned int flags; /* ATA_LFLAG_xxx */
>
> @@ -857,6 +859,10 @@ struct ata_port {
> #ifdef CONFIG_ATA_ACPI
> struct ata_acpi_gtm __acpi_init_gtm; /* use ata_acpi_init_gtm() */
> #endif
> +
> + u64 preempted_qc_active;
> + int preempted_nr_active_links;
> +
> /* owned by EH */
> u8 sector_buf[ATA_SECT_SIZE] ____cacheline_aligned;
> };
--
Damien Le Moal
Western Digital Research
On 27/10/2022 02:42, Damien Le Moal wrote:
> On 10/25/22 19:32, John Garry wrote:
>> Add a function to handle queued ATA internal SCSI cmnds - does much the
>> same as ata_exec_internal_sg() does (which will be fixed up later to
>> actually queue internal cmnds through this function).
>>
>> Signed-off-by: John Garry <[email protected]>
>> ---
>> drivers/ata/libata-sata.c | 3 +++
>> drivers/ata/libata-scsi.c | 43 +++++++++++++++++++++++++++++++++++++++
>> drivers/ata/libata.h | 3 ++-
>> include/linux/libata.h | 6 ++++++
>> 4 files changed, 54 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/ata/libata-sata.c b/drivers/ata/libata-sata.c
>> index b6806d41a8c5..e8b828c56542 100644
>> --- a/drivers/ata/libata-sata.c
>> +++ b/drivers/ata/libata-sata.c
>> @@ -1258,6 +1258,9 @@ int ata_sas_queuecmd(struct scsi_cmnd *cmd, struct ata_port *ap)
>> {
>> int rc = 0;
>>
>> + if (blk_mq_is_reserved_rq(scsi_cmd_to_rq(cmd)))
>> + return ata_scsi_queue_internal(cmd, ap->link.device);
>> +
>> if (likely(ata_dev_enabled(ap->link.device)))
>> rc = __ata_scsi_queuecmd(cmd, ap->link.device);
>> else {
>> diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
>> index 476e0ef4bd29..30d7c90b0c35 100644
>> --- a/drivers/ata/libata-scsi.c
>> +++ b/drivers/ata/libata-scsi.c
>> @@ -3965,6 +3965,49 @@ static inline ata_xlat_func_t ata_get_xlat_func(struct ata_device *dev, u8 cmd)
>> return NULL;
>> }
>>
>> +unsigned int ata_scsi_queue_internal(struct scsi_cmnd *scmd,
>> + struct ata_device *dev)
>> +{
>> + struct ata_link *link = dev->link;
>> + struct ata_port *ap = link->ap;
>> + struct ata_queued_cmd *qc;
>> +
>> + /* no internal command while frozen */
>> + if (ap->pflags & ATA_PFLAG_FROZEN)
>> + goto did_err;
>> +
>> + /* initialize internal qc */
>> + qc = __ata_qc_from_tag(ap, ATA_TAG_INTERNAL);
>> + link->preempted_tag = link->active_tag;
>> + link->preempted_sactive = link->sactive;
>> + ap->preempted_qc_active = ap->qc_active;
>> + ap->preempted_nr_active_links = ap->nr_active_links;
>> + link->active_tag = ATA_TAG_POISON;
>> + link->sactive = 0;
>> + ap->qc_active = 0;
>> + ap->nr_active_links = 0;
>> +
>> + if (qc->dma_dir != DMA_NONE) {
>> + int n_elem;
>> +
>> + n_elem = 1;
>> + qc->n_elem = n_elem;
>> + qc->sg = scsi_sglist(scmd);
>> + qc->nbytes = qc->sg->length;
>> + ata_sg_init(qc, qc->sg, n_elem);
>> + }
>> +
>> + scmd->submitter = SUBMITTED_BY_BLOCK_LAYER;
>> +
>> + ata_qc_issue(qc);
>
> Arg, no ! This potentially mixes NCQ and non-NCQ commands, which is
> forbidden by ATA spec. You need to use something like:
>
> if (ap->ops->qc_defer) {
> if ((rc = ap->ops->qc_defer(qc)))
> goto defer;
> }
>
> ata_qc_issue(qc);
>
> which is done in __ata_scsi_queuecmd() -> ata_scsi_translate()
>
> Unless you guarantee that ata_scsi_queue_internal() is always called
> from libata EH context ?
This will be called synchronously called from ata_exec_internal_sg(), so
the same rules on NCQ vs non-NCQ would apply here. As I see,
ata_exec_internal_sg() assumes non-NCQ mode and is not multi-thread safe.
Thanks,
John
>
>> +
>> + return 0;
>> +did_err:
>> + scmd->result = (DID_ERROR << 16);
>> + scsi_done(scmd);
>> + return 0;
>> +}
>> +
>> int __ata_scsi_queuecmd(struct scsi_cmnd *scmd, struct ata_device *dev)
>> {
>> u8 scsi_op = scmd->cmnd[0];
>> diff --git a/drivers/ata/libata.h b/drivers/ata/libata.h
>> index 0c2df1e60768..15cd1cd618b8 100644
>> --- a/drivers/ata/libata.h
>> +++ b/drivers/ata/libata.h
>> @@ -82,7 +82,6 @@ extern int ata_port_probe(struct ata_port *ap);
>> extern void __ata_port_probe(struct ata_port *ap);
>> extern unsigned int ata_read_log_page(struct ata_device *dev, u8 log,
>> u8 page, void *buf, unsigned int sectors);
>> -
>> #define to_ata_port(d) container_of(d, struct ata_port, tdev)
>>
>> /* libata-acpi.c */
>> @@ -130,6 +129,8 @@ extern int ata_scsi_user_scan(struct Scsi_Host *shost, unsigned int channel,
>> void ata_scsi_sdev_config(struct scsi_device *sdev);
>> int ata_scsi_dev_config(struct scsi_device *sdev, struct ata_device *dev);
>> int __ata_scsi_queuecmd(struct scsi_cmnd *scmd, struct ata_device *dev);
>> +unsigned int ata_scsi_queue_internal(struct scsi_cmnd *scmd,
>> + struct ata_device *dev);
>>
>> /* libata-eh.c */
>> extern unsigned int ata_internal_cmd_timeout(struct ata_device *dev, u8 cmd);
>> diff --git a/include/linux/libata.h b/include/linux/libata.h
>> index 827d5838cd23..8938b584520f 100644
>> --- a/include/linux/libata.h
>> +++ b/include/linux/libata.h
>> @@ -764,7 +764,9 @@ struct ata_link {
>>
>> struct device tdev;
>> unsigned int active_tag; /* active tag on this link */
>> + unsigned int preempted_tag;
>> u32 sactive; /* active NCQ commands */
>> + u32 preempted_sactive;
>>
>> unsigned int flags; /* ATA_LFLAG_xxx */
>>
>> @@ -857,6 +859,10 @@ struct ata_port {
>> #ifdef CONFIG_ATA_ACPI
>> struct ata_acpi_gtm __acpi_init_gtm; /* use ata_acpi_init_gtm() */
>> #endif
>> +
>> + u64 preempted_qc_active;
>> + int preempted_nr_active_links;
>> +
>> /* owned by EH */
>> u8 sector_buf[ATA_SECT_SIZE] ____cacheline_aligned;
>> };
>
On 10/27/22 19:45, John Garry wrote:
> On 27/10/2022 02:42, Damien Le Moal wrote:
>> On 10/25/22 19:32, John Garry wrote:
>>> Add a function to handle queued ATA internal SCSI cmnds - does much the
>>> same as ata_exec_internal_sg() does (which will be fixed up later to
>>> actually queue internal cmnds through this function).
>>>
>>> Signed-off-by: John Garry <[email protected]>
>>> ---
>>> drivers/ata/libata-sata.c | 3 +++
>>> drivers/ata/libata-scsi.c | 43 +++++++++++++++++++++++++++++++++++++++
>>> drivers/ata/libata.h | 3 ++-
>>> include/linux/libata.h | 6 ++++++
>>> 4 files changed, 54 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/ata/libata-sata.c b/drivers/ata/libata-sata.c
>>> index b6806d41a8c5..e8b828c56542 100644
>>> --- a/drivers/ata/libata-sata.c
>>> +++ b/drivers/ata/libata-sata.c
>>> @@ -1258,6 +1258,9 @@ int ata_sas_queuecmd(struct scsi_cmnd *cmd, struct ata_port *ap)
>>> {
>>> int rc = 0;
>>>
>>> + if (blk_mq_is_reserved_rq(scsi_cmd_to_rq(cmd)))
>>> + return ata_scsi_queue_internal(cmd, ap->link.device);
>>> +
>>> if (likely(ata_dev_enabled(ap->link.device)))
>>> rc = __ata_scsi_queuecmd(cmd, ap->link.device);
>>> else {
>>> diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
>>> index 476e0ef4bd29..30d7c90b0c35 100644
>>> --- a/drivers/ata/libata-scsi.c
>>> +++ b/drivers/ata/libata-scsi.c
>>> @@ -3965,6 +3965,49 @@ static inline ata_xlat_func_t ata_get_xlat_func(struct ata_device *dev, u8 cmd)
>>> return NULL;
>>> }
>>>
>>> +unsigned int ata_scsi_queue_internal(struct scsi_cmnd *scmd,
>>> + struct ata_device *dev)
>>> +{
>>> + struct ata_link *link = dev->link;
>>> + struct ata_port *ap = link->ap;
>>> + struct ata_queued_cmd *qc;
>>> +
>>> + /* no internal command while frozen */
>>> + if (ap->pflags & ATA_PFLAG_FROZEN)
>>> + goto did_err;
>>> +
>>> + /* initialize internal qc */
>>> + qc = __ata_qc_from_tag(ap, ATA_TAG_INTERNAL);
>>> + link->preempted_tag = link->active_tag;
>>> + link->preempted_sactive = link->sactive;
>>> + ap->preempted_qc_active = ap->qc_active;
>>> + ap->preempted_nr_active_links = ap->nr_active_links;
>>> + link->active_tag = ATA_TAG_POISON;
>>> + link->sactive = 0;
>>> + ap->qc_active = 0;
>>> + ap->nr_active_links = 0;
>>> +
>>> + if (qc->dma_dir != DMA_NONE) {
>>> + int n_elem;
>>> +
>>> + n_elem = 1;
>>> + qc->n_elem = n_elem;
>>> + qc->sg = scsi_sglist(scmd);
>>> + qc->nbytes = qc->sg->length;
>>> + ata_sg_init(qc, qc->sg, n_elem);
>>> + }
>>> +
>>> + scmd->submitter = SUBMITTED_BY_BLOCK_LAYER;
>>> +
>>> + ata_qc_issue(qc);
>>
>> Arg, no ! This potentially mixes NCQ and non-NCQ commands, which is
>> forbidden by ATA spec. You need to use something like:
>>
>> if (ap->ops->qc_defer) {
>> if ((rc = ap->ops->qc_defer(qc)))
>> goto defer;
>> }
>>
>> ata_qc_issue(qc);
>>
>> which is done in __ata_scsi_queuecmd() -> ata_scsi_translate()
>>
>> Unless you guarantee that ata_scsi_queue_internal() is always called
>> from libata EH context ?
>
> This will be called synchronously called from ata_exec_internal_sg(), so
> the same rules on NCQ vs non-NCQ would apply here. As I see,
> ata_exec_internal_sg() assumes non-NCQ mode and is not multi-thread safe.
Yep. No thread safety needed as we are always guaranteed to be in EH with
the queue quiesced when this is executed. No other commands can come in at
the same time.
>
> Thanks,
> John
>
>>
>>> +
>>> + return 0;
>>> +did_err:
>>> + scmd->result = (DID_ERROR << 16);
>>> + scsi_done(scmd);
>>> + return 0;
>>> +}
>>> +
>>> int __ata_scsi_queuecmd(struct scsi_cmnd *scmd, struct ata_device *dev)
>>> {
>>> u8 scsi_op = scmd->cmnd[0];
>>> diff --git a/drivers/ata/libata.h b/drivers/ata/libata.h
>>> index 0c2df1e60768..15cd1cd618b8 100644
>>> --- a/drivers/ata/libata.h
>>> +++ b/drivers/ata/libata.h
>>> @@ -82,7 +82,6 @@ extern int ata_port_probe(struct ata_port *ap);
>>> extern void __ata_port_probe(struct ata_port *ap);
>>> extern unsigned int ata_read_log_page(struct ata_device *dev, u8 log,
>>> u8 page, void *buf, unsigned int sectors);
>>> -
>>> #define to_ata_port(d) container_of(d, struct ata_port, tdev)
>>>
>>> /* libata-acpi.c */
>>> @@ -130,6 +129,8 @@ extern int ata_scsi_user_scan(struct Scsi_Host *shost, unsigned int channel,
>>> void ata_scsi_sdev_config(struct scsi_device *sdev);
>>> int ata_scsi_dev_config(struct scsi_device *sdev, struct ata_device *dev);
>>> int __ata_scsi_queuecmd(struct scsi_cmnd *scmd, struct ata_device *dev);
>>> +unsigned int ata_scsi_queue_internal(struct scsi_cmnd *scmd,
>>> + struct ata_device *dev);
>>>
>>> /* libata-eh.c */
>>> extern unsigned int ata_internal_cmd_timeout(struct ata_device *dev, u8 cmd);
>>> diff --git a/include/linux/libata.h b/include/linux/libata.h
>>> index 827d5838cd23..8938b584520f 100644
>>> --- a/include/linux/libata.h
>>> +++ b/include/linux/libata.h
>>> @@ -764,7 +764,9 @@ struct ata_link {
>>>
>>> struct device tdev;
>>> unsigned int active_tag; /* active tag on this link */
>>> + unsigned int preempted_tag;
>>> u32 sactive; /* active NCQ commands */
>>> + u32 preempted_sactive;
>>>
>>> unsigned int flags; /* ATA_LFLAG_xxx */
>>>
>>> @@ -857,6 +859,10 @@ struct ata_port {
>>> #ifdef CONFIG_ATA_ACPI
>>> struct ata_acpi_gtm __acpi_init_gtm; /* use ata_acpi_init_gtm() */
>>> #endif
>>> +
>>> + u64 preempted_qc_active;
>>> + int preempted_nr_active_links;
>>> +
>>> /* owned by EH */
>>> u8 sector_buf[ATA_SECT_SIZE] ____cacheline_aligned;
>>> };
>>
>
--
Damien Le Moal
Western Digital Research