mbox series

[PATCHSET,0/9] libata: anable full 32 tag queue depth

Message ID 1526064671-7528-1-git-send-email-axboe@kernel.dk
Headers show
Series libata: anable full 32 tag queue depth | expand

Message

Jens Axboe May 11, 2018, 6:51 p.m. UTC
Ever since we implemented NCQ for libata, the max queue depth on
the device side has been limited to 31. This was done to retain one
command for error handling. But we can utilize the full depth with
a bit of fixing.

The motivation for this is twofold:

1) New devices do amazingly well with extracting parallelism from
   a deeper queue depth, much more so than earlier. I recently ran
   some tests on two identical drives, with the only difference
   being SAS vs SATA interface. The former runs at QD=254, while
   the latter is limited to 31. The SAS drive continues to pull
   out performance from randomized IO at higher queue depths,
   ending up being as much as 30% faster than the SATA drive even
   though they are physically identical.

2) We have IOPS limited applications on rotating storage. A 3%
   increase in performance is nothing to sneeze at.

Most of this series is prep work to get us to the point where
we can enable the full 32 queue depth. The last patch enables
it for AHCI. Other drivers can easily enable it as well, just
modify the can_queue depth passed in. I didn't want to make it
the default, as with 32 commands running, SActive register read
will be 0xffffffff, which some drivers might interpret as the
controller having fallen off the bus. Opting in is easy once that's
verified.

 drivers/ata/acard-ahci.c     |    4 +--
 drivers/ata/ahci.h           |    2 -
 drivers/ata/libahci.c        |    8 +++---
 drivers/ata/libata-core.c    |   53 +++++++++++++++++--------------------------
 drivers/ata/libata-eh.c      |   10 +++++---
 drivers/ata/libata-scsi.c    |   16 ++++++------
 drivers/ata/sata_dwc_460ex.c |   14 +++++------
 drivers/ata/sata_fsl.c       |    8 +++---
 drivers/ata/sata_mv.c        |   26 ++++++++++-----------
 drivers/ata/sata_nv.c        |   36 ++++++++++++++---------------
 drivers/ata/sata_sil24.c     |    6 ++--
 include/linux/libata.h       |   22 ++++++++---------
 12 files changed, 100 insertions(+), 105 deletions(-)

Comments

Tejun Heo May 11, 2018, 8:12 p.m. UTC | #1
Hello,

On Fri, May 11, 2018 at 12:51:02PM -0600, Jens Axboe wrote:
> Ever since we implemented NCQ for libata, the max queue depth on
> the device side has been limited to 31. This was done to retain one
> command for error handling. But we can utilize the full depth with
> a bit of fixing.

This is great.  Applied 1-9 to libata/for-4.18.

Thanks.
Jens Axboe May 11, 2018, 8:49 p.m. UTC | #2
On 5/11/18 2:12 PM, Tejun Heo wrote:
> Hello,
> 
> On Fri, May 11, 2018 at 12:51:02PM -0600, Jens Axboe wrote:
>> Ever since we implemented NCQ for libata, the max queue depth on
>> the device side has been limited to 31. This was done to retain one
>> command for error handling. But we can utilize the full depth with
>> a bit of fixing.
> 
> This is great.  Applied 1-9 to libata/for-4.18.

Awesome, thanks! One piece of fallout, can you add the patch below
as well?

From: Jens Axboe <axboe@kernel.dk>
Subject: [PATCH] sata_fsl: use the right type for tag bitshift

Since ATA_TAG_INTERNAL is now > 31 bits, we need to extend the
type to ULL to cover 32/64-bit cases.

Signed-off-by: Jens Axboe <axboe@kernel.dk>

diff --git a/drivers/ata/sata_fsl.c b/drivers/ata/sata_fsl.c
index 1b22d5c339d7..b8d9cfc60374 100644
--- a/drivers/ata/sata_fsl.c
+++ b/drivers/ata/sata_fsl.c
@@ -1293,7 +1293,7 @@ static void sata_fsl_host_intr(struct ata_port *ap)
 		ata_qc_complete_multiple(ap, ap->qc_active ^ done_mask);
 		return;
 
-	} else if ((ap->qc_active & (1 << ATA_TAG_INTERNAL))) {
+	} else if ((ap->qc_active & (1ULL << ATA_TAG_INTERNAL))) {
 		iowrite32(1, hcr_base + CC);
 		qc = ata_qc_from_tag(ap, ATA_TAG_INTERNAL);
Tejun Heo May 14, 2018, 3:15 p.m. UTC | #3
On Fri, May 11, 2018 at 02:49:25PM -0600, Jens Axboe wrote:
> On 5/11/18 2:12 PM, Tejun Heo wrote:
> > Hello,
> > 
> > On Fri, May 11, 2018 at 12:51:02PM -0600, Jens Axboe wrote:
> >> Ever since we implemented NCQ for libata, the max queue depth on
> >> the device side has been limited to 31. This was done to retain one
> >> command for error handling. But we can utilize the full depth with
> >> a bit of fixing.
> > 
> > This is great.  Applied 1-9 to libata/for-4.18.
> 
> Awesome, thanks! One piece of fallout, can you add the patch below
> as well?
> 
> From: Jens Axboe <axboe@kernel.dk>
> Subject: [PATCH] sata_fsl: use the right type for tag bitshift
> 
> Since ATA_TAG_INTERNAL is now > 31 bits, we need to extend the
> type to ULL to cover 32/64-bit cases.
> 
> Signed-off-by: Jens Axboe <axboe@kernel.dk>

Applied to libata/for-4.18.

Thanks.