Re: [PATCH RFC v3 2/7] ata: libata-scsi: Add ata_internal_queuecommand()

From: Hannes Reinecke
Date: Thu Oct 27 2022 - 09:03:07 EST


On 10/27/22 11:56, John Garry wrote:
On 27/10/2022 02:45, Damien Le Moal wrote:
On 10/25/22 19:32, John Garry wrote:
Add callback to queue reserved commands - call it "internal" as this is
what libata uses.

Also add it to the base ATA SHT.

Signed-off-by: John Garry <john.garry@xxxxxxxxxx>
---
  drivers/ata/libata-scsi.c | 14 ++++++++++++++
  include/linux/libata.h    |  5 ++++-
  2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 30d7c90b0c35..0d6f37d80137 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -1118,6 +1118,20 @@ int ata_scsi_dev_config(struct scsi_device *sdev, struct ata_device *dev)
      return 0;
  }
+int ata_internal_queuecommand(struct Scsi_Host *shost, struct scsi_cmnd *scmd)
+{
+    struct ata_port *ap;
+    int res;
+
+    ap = ata_shost_to_port(shost);

You can have this initialization together with the ap declaration.

+    spin_lock_irq(ap->lock);
+    res = ata_sas_queuecmd(scmd, ap);
+    spin_unlock_irq(ap->lock);
+
+    return res;
+}
+EXPORT_SYMBOL_GPL(ata_internal_queuecommand);

I am officially lost here. Do not see why this function is needed...

The general idea in this series is to send ATA internal commands as requests. And this function is used as the SCSI midlayer to queue reserved commands. See how it is plugged into __ATA_BASE_SHT, below.

So we have this overall flow:

ata_exec_internal_sg():
 -> alloc request
 -> blk_execute_rq_nowait()
     ... -> scsi_queue_rq()
        -> sht->reserved_queuecommd()
            -> ata_internal_queuecommand()

And then we have ata_internal_queuecommand() -> ata_sas_queuecmd() -> ata_scsi_queue_internal() -> ata_qc_issue().

Hope it makes sense.

Thanks,
John


+
  /**
   *    ata_scsi_slave_config - Set SCSI device attributes
   *    @sdev: SCSI device to examine
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 8938b584520f..f09c5dca16ce 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -1141,6 +1141,8 @@ extern int ata_std_bios_param(struct scsi_device *sdev,
                    sector_t capacity, int geom[]);
  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 void ata_scsi_slave_destroy(struct scsi_device *sdev);
  extern int ata_scsi_change_queue_depth(struct scsi_device *sdev,
                         int queue_depth);
@@ -1391,7 +1393,8 @@ extern const struct attribute_group *ata_common_sdev_groups[];
      .slave_destroy        = ata_scsi_slave_destroy,    \
      .bios_param        = ata_std_bios_param,        \
      .unlock_native_capacity    = ata_scsi_unlock_native_capacity,\
-    .max_sectors        = ATA_MAX_SECTORS_LBA48
+    .max_sectors        = ATA_MAX_SECTORS_LBA48,\
+    .reserved_queuecommand = ata_internal_queuecommand
  #define ATA_SUBBASE_SHT(drv_name)                \
      __ATA_BASE_SHT(drv_name),                \



But that means we can't use it before the SCSI host is initialized; some HBAs require to send commands before the host can be initialized properly.

Cheers,

Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
hare@xxxxxxx +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Ivo Totev, Andrew
Myers, Andrew McDonald, Martje Boudien Moerman