libata: switch to block layer tagging

drivers/ata/libata-scsi.c +7 -2

index 2733b0c..04f7a8d 100644
@@ -742,7 +742,11 @@ static struct ata_queued_cmd *ata_scsi_qc_new(struct ata_device *dev,
 {
 	struct ata_queued_cmd *qc;
 
-	qc = ata_qc_new_init(dev);
+	if (cmd->request->tag != -1)
+		qc = ata_qc_new_init(dev, cmd->request->tag);
+	else
+		qc = ata_qc_new_init(dev, 0);
+
 	if (qc) {
 		qc->scsicmd = cmd;
 		qc->scsidone = done;

@@ -1137,7 +1141,17 @@ static int ata_scsi_dev_config(struct scsi_device *sdev,
 
 		depth = min(sdev->host->can_queue, ata_id_queue_depth(dev->id));
 		depth = min(ATA_MAX_QUEUE - 1, depth);
-		scsi_adjust_queue_depth(sdev, MSG_SIMPLE_TAG, depth);
+
+		/*

libata currently has a pretty dumb ATA_MAX_QUEUE loop for finding a free tag to use. Instead of fixing that up, convert libata to using block layer tagging - gets rid of code in libata, and is also much faster.

Quoted 4 lines · Jens Axboe

libata currently has a pretty dumb ATA_MAX_QUEUE loop for finding a free tag to use. Instead of fixing that up, convert libata to using block layer tagging - gets rid of code in libata, and is also much faster.

...

drivers/ata/libata-scsi.c static int ata_scsi_dev_config(struct scsi_device *sdev, · line 1148

Quoted 10 lines

drivers/ata/libata-scsi.c +5 -1

@@ -1137,7 +1141,17 @@ static int ata_scsi_dev_config(struct scsi_device *sdev,
 
               depth = min(sdev->host->can_queue, ata_id_queue_depth(dev->id));
               depth = min(ATA_MAX_QUEUE - 1, depth);
-               scsi_adjust_queue_depth(sdev, MSG_SIMPLE_TAG, depth);
+
+               /*
+                * If this device is behind a port multiplier, we have
+                * to share the tag map between all devices on that PMP.
+                * Set up the shared tag map here and we get automatic.

Automatic what?

drivers/ata/libata-scsi.c static int ata_scsi_dev_config(struct scsi_device *sdev, · line 1154

Quoted 6 lines

drivers/ata/libata-scsi.c +6 -0

+                */
+               if (dev->link->ap->pmp_link)
+                       scsi_init_shared_tag_map(sdev->host, ATA_MAX_QUEUE - 1);
+
+               scsi_set_tag_type(sdev, MSG_SIMPLE_TAG);
+               scsi_activate_tcq(sdev, depth);

I just read Tejun's reply and it sounds right what he's saying. But can SATA controllers handle NCQ and !NCQ devices on the same port? Can the PMP handle it?

If both can, I don't understand how a mixed config works today.

TBH, this isn't something I'm very worried about since most commercial configs will be homogenous (think HW replacement/support costs).

hth,
grant
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Form the ATA and SATA spec, NCQ is per device. It is possible to assign the same tag on different port, the disks and PMP will not care.
However, today, we assign tag on a port basis [see __ata_qc_from_tag()], therefore only 32 commands can be inflight to all devices behind a given port.

Being able to a do a mapping qc <---> {pmp port, tag} instead of just qc <---> {tag} will provide a performance boost when disks supporting NCQ are connected behind a PMP. Maybe it can be done, by moving qcmd from ata_port to ata_link.

As Tejun said, the patch needs more work to be able to support same tag used on 2 different links behind the same port.
Also, given we are changing the amount of commands we sent to the controller, we would have to change can_queue from ATA_MAX_QUEUE to ATA_MAX_QUEUE*n, where n is the number of ports supported by the controller [max SATA_PMP_MAX_PORTS, but some controller, like SiI3132 only supports 5 devices, and other may only support n = 1]

When done, the patch will require a great amount of testing, given we will exercise the controllers in a brand new way. A white list might be necessary.

Jens, using SCSI tagging for ata commands is a great idea, but it is no small feat...

Quoted 24 lines · Gwendal Grignou

Form the ATA and SATA spec, NCQ is per device. It is possible to assign the same tag on different port, the disks and PMP will not care.
However, today, we assign tag on a port basis [see __ata_qc_from_tag()], therefore only 32 commands can be inflight to all devices behind a given port.

Being able to a do a mapping qc <---> {pmp port, tag} instead of just qc <---> {tag} will provide a performance boost when disks supporting NCQ are connected behind a PMP. Maybe it can be done, by moving qcmd from ata_port to ata_link.

As Tejun said, the patch needs more work to be able to support same tag used on 2 different links behind the same port.
Also, given we are changing the amount of commands we sent to the controller, we would have to change can_queue from ATA_MAX_QUEUE to ATA_MAX_QUEUE*n, where n is the number of ports supported by the controller [max SATA_PMP_MAX_PORTS, but some controller, like SiI3132 only supports 5 devices, and other may only support n = 1]

When done, the patch will require a great amount of testing, given we will exercise the controllers in a brand new way. A white list might be necessary.

Quoted 2 lines

Jens, using SCSI tagging for ata commands is a great idea, but it is no small feat...

So realistically, you want one block queue per PMP port rather than an artificial limits adjustment on the single queue per output phy ... this shouldn't be too hard: it is exactly the way the SAS transport class (and libsas) works today for expander connected SAS devices.

Well... The limiting factor is "ATA command slots per SATA phy", which defines $N active ATA commands per port. If a SATA NCQ device is attached, you may have $N active ATA commands queued. If two SATA NCQ devices are attached to a PMP, which is attached to the SATA controller, the two devices share the limit of $N active ATA commands queued. If 32 devices are attached to a PMP, all 32 devices share the $N command queue limit.

But additionally, as Tejun demonstrated, you might have a mix of NCQ and non-NCQ devices attached to the PMP.

Thus, it is a case of nested limits:

- $N maximum active commands per port; typically N==32
- $M maximum active commands per device; typically N==1 or N==32

Quoted 8 lines · Jeff Garzik

Well... The limiting factor is "ATA command slots per SATA phy", which
defines $N active ATA commands per port. If a SATA NCQ device is attached, you may have $N active ATA commands queued. If two SATA NCQ devices are attached to a PMP, which is attached to the SATA controller, the two devices share the limit of $N active ATA commands queued. If 32 devices are attached to a PMP, all 32 devices share the $N command queue limit.

Quoted 7 lines

But additionally, as Tejun demonstrated, you might have a mix of NCQ and non-NCQ devices attached to the PMP.

Thus, it is a case of nested limits:

- $N maximum active commands per port; typically N==32
- $M maximum active commands per device; typically N==1 or N==32

Note that this applies /somewhat/ to mvsas as well: mvsas has up to 16 "ATA register sets" that the OS driver may assign to various ATA devices on a single phy.

This simulates the "command slot" concept on dedicated SATA+PMP+NCQ hardware like AHCI or sata_sil24.

If you attach more than 16 ATA devices to a PMP, which is theoretically possible, then mvsas MUST fail discovery, because it cannot address that many ATA devices.

But mvsas's different architecture means that it does not have similar "command slot" limits.

Jeff

We can not have more than 16 devices per ata_port: a PMP can only have 15 device ports [Sata 2.6- 16.2], and we can not connect a PMP to another one.

Jeff, you're right, I made some mistakes:
- Reading SiI3132 doc again, it only supports 32 command per port, so the tags must be share among all the drives behind the same port. But other chipset does support up to 128 commands per port.
- Jens' patch is working when all disks support NCQ. Only when we have a mix of drives with or without NCQ we have a problem.

Quoted 6 lines · Gwendal Grignou

Jeff, you're right, I made some mistakes:
- Reading SiI3132 doc again, it only supports 32 command per port, so the tags must be share among all the drives behind the same port. But other chipset does support up to 128 commands per port.
- Jens' patch is working when all disks support NCQ. Only when we have a mix of drives with or without NCQ we have a problem.

..

Along those lines, the newer Marvell chipsets support up to 128 commands per host port.

I seem to recall that the Pacific Digital Qstor chip can manage an insane number of commands -- something like 32 per device per PM port [eg. (15 * 32) in total per host port].

Seems to be a widespread kind of thing for non-legacy chipsets. Someday we really ought to beef up libata to allow host-chipset queuing of non-NCQ commands, too.

Form the ATA and SATA spec, NCQ is per device. It is possible to assign the same tag on different port, the disks and PMP will not care.
However, today, we assign tag on a port basis [see __ata_qc_from_tag()], therefore only 32 commands can be inflight to all devices behind a given port.

Being able to a do a mapping qc <---> {pmp port, tag} instead of just qc <---> {tag} will provide a performance boost when disks supporting NCQ are connected behind a PMP. Maybe it can be done, by moving qcmd from ata_port to ata_link.

As Tejun said, the patch needs more work to be able to support same tag used on 2 different links behind the same port.
Also, given we are changing the amount of commands we sent to the controller, we would have to change can_queue from ATA_MAX_QUEUE to ATA_MAX_QUEUE*n, where n is the number of ports supported by the controller [max SATA_PMP_MAX_PORTS, but some controller, like SiI3132 only supports 5 devices, and other may only support n = 1]

When done, the patch will require a great amount of testing, given we will exercise the controllers in a brand new way. A white list might be necessary.

Quoted 4 lines · Jens Axboe

libata currently has a pretty dumb ATA_MAX_QUEUE loop for finding a free tag to use. Instead of fixing that up, convert libata to using block layer tagging - gets rid of code in libata, and is also much faster.

...

drivers/ata/libata-scsi.c static int ata_scsi_dev_config(struct scsi_device *sdev, · line 1148

Quoted 10 lines

drivers/ata/libata-scsi.c +5 -1

@@ -1137,7 +1141,17 @@ static int ata_scsi_dev_config(struct scsi_device *sdev,
 
               depth = min(sdev->host->can_queue, ata_id_queue_depth(dev->id));
               depth = min(ATA_MAX_QUEUE - 1, depth);
-               scsi_adjust_queue_depth(sdev, MSG_SIMPLE_TAG, depth);
+
+               /*
+                * If this device is behind a port multiplier, we have
+                * to share the tag map between all devices on that PMP.
+                * Set up the shared tag map here and we get automatic.

Automatic what?

drivers/ata/libata-scsi.c static int ata_scsi_dev_config(struct scsi_device *sdev, · line 1154

Quoted 6 lines

drivers/ata/libata-scsi.c +6 -0

+                */
+               if (dev->link->ap->pmp_link)
+                       scsi_init_shared_tag_map(sdev->host, ATA_MAX_QUEUE - 1);
+
+               scsi_set_tag_type(sdev, MSG_SIMPLE_TAG);
+               scsi_activate_tcq(sdev, depth);

I just read Tejun's reply and it sounds right what he's saying. But can SATA controllers handle NCQ and !NCQ devices on the same port? Can the PMP handle it?

If both can, I don't understand how a mixed config works today.

libata currently has a pretty dumb ATA_MAX_QUEUE loop for finding a free tag to use. Instead of fixing that up, convert libata to using block layer tagging - gets rid of code in libata, and is also much faster.

Signed-off-by Jens Axboe <[email protected]>

---
 drivers/ata/libata-core.c |   65 ++++----------------------------------------
 drivers/ata/libata-scsi.c |   23 ++++++++++++++-
 drivers/ata/libata.h      |   19 ++++++++++++-
 include/linux/libata.h    |    1 -
 4 files changed, 44 insertions(+), 64 deletions(-)

Quoted 5 lines · Jens Axboe

libata currently has a pretty dumb ATA_MAX_QUEUE loop for finding a free tag to use. Instead of fixing that up, convert libata to using block layer tagging - gets rid of code in libata, and is also much faster.

Quoted 8 lines

Signed-off-by Jens Axboe <[email protected]>

---
 drivers/ata/libata-core.c |   65
++++----------------------------------------
 drivers/ata/libata-scsi.c |   23 ++++++++++++++-
 drivers/ata/libata.h      |   19 ++++++++++++-
 include/linux/libata.h    |    1 -
 4 files changed, 44 insertions(+), 64 deletions(-)

Patch looks good to me, with regards (a) simplex, (b) master/slave, (c) non-queued and (d) NCQ. I'm not confident about the PMP case, but if Tejun is happy, I guess I can go ahead and queue these...

We need this in libata to ensure that we don't race between internal tag usage and the block layer usage.

there won't be any race around it (the old-EH path is broken if invoked with commands in flight anyway, so doesn't matter). Also, as

Brian King did a new-EH conversion for ipr, some time ago. Maybe that work could be picked up, extended to libsas, and permit removal of all the old-EH code remaining in libata.

We need this in libata to ensure that we don't race between internal tag usage and the block layer usage.

Signed-off-by Jens Axboe <[email protected]>

---
 block/blk-tag.c           |   99 ++++++++++++++++++++++++++++++++------------- drivers/ata/libata-core.c |   29 +++++++++---- include/linux/blkdev.h    |    3 +

 3 files changed, 95 insertions(+), 36 deletions(-)

block/blk-tag.c +1 -0

index e9a7501..208468b 100644
@@ -149,6 +149,7 @@ static struct blk_queue_tag *__blk_queue_init_tags(struct request_queue *q,
                goto fail;
 
        atomic_set(&tags->refcnt, 1);
+       init_waitqueue_head(&tags->waitq);
        return tags;

 fail:
       kfree(tags);

block/blk-tag.c +13 -0

@@ -264,6 +265,65 @@ int blk_queue_resize_tags(struct request_queue *q, int new_depth)
  }
  EXPORT_SYMBOL(blk_queue_resize_tags);
 
+void blk_queue_acquire_tag(struct request_queue *q, int tag)
+{
+       struct blk_queue_tag *bqt;
+
+       if (!blk_queue_tagged(q) || !q->queue_tags)
+               return;
+
+       bqt = q->queue_tags;
+       do {
+               DEFINE_WAIT(wait);
+
+               if (!test_and_set_bit_lock(tag, bqt->tag_map))
+                       break;

block/blk-tag.c +3 -0

+
+void blk_queue_release_tag(struct request_queue *q, int tag)
+{

Hi,

This patchset switches libata to block layer tagging. It gets rid of yet another O(N) command loop.

 block/blk-tag.c           |   99 +++++++++++++++++++++++++++-----------
 drivers/ata/libata-core.c |   94 ++++++++++--------------------------
 drivers/ata/libata-scsi.c |   23 ++++++++
 drivers/ata/libata.h      |   19 ++++++-
 include/linux/blkdev.h    |    3 +
 include/linux/libata.h    |    1 
 6 files changed, 139 insertions(+), 100 deletions(-)

Quoted 5 lines · Jens Axboe

Hi,

This patchset switches libata to block layer tagging. It gets rid of yet another O(N) command loop.

Quoted 7 lines

 block/blk-tag.c           |   99 +++++++++++++++++++++++++++-----------
 drivers/ata/libata-core.c |   94 ++++++++++--------------------------
 drivers/ata/libata-scsi.c |   23 ++++++++
 drivers/ata/libata.h      |   19 ++++++-
 include/linux/blkdev.h    |    3 +

include/linux/libata.h | 1 6 files changed, 139 insertions(+),
100 deletions(-)

Will take a look post-sleep... mainly it's a question of possibly

interfering with SCSI's use of block layer tagging. Though for the moment I will simply assume you verified that ;)