[0/2] scsi/libata: A potential tagging fix and improvement

Message

John Garry March 15, 2022, 10:39 a.m. UTC

Two loosely related patches are included:

- Fix for scsi_realloc_sdev_budget_map(). I noticed that the budget token
  for scsi commands was way in excess of the device queue depth, so I
  think we need to fix the sbitmap depth. I need to test this more.

- libata change to use scsi command budget token for qc tag for SAS host.
  I marked this as RFC as for SAS hosts I don't see anything which
  guarantees that the budget size is <= 32 always.
  For libsas hosts we resize the device depth to 32 in the slave configure
  callback, but this seems an unreliable approach since not all hosts may
  call this.
  In addition, I am worried that even if we resize the device depth
  properly in the slave config callback, we may still try to alloc qc tag
  prior to this - in lun scan, for example.
  So we need a way to guarantee that the device queue depth is <= 32
  always, which I would be open to suggestions for.

John Garry (2):
  scsi: core: Fix sbitmap depth in scsi_realloc_sdev_budget_map()
  libata: Use scsi cmnd budget token for qc tag for SAS host

 drivers/ata/libata-core.c |  5 +++--
 drivers/ata/libata-sata.c | 21 ++++-----------------
 drivers/ata/libata-scsi.c |  2 +-
 drivers/ata/libata.h      |  4 ++--
 drivers/scsi/scsi_scan.c  |  5 +++++
 include/linux/libata.h    |  1 -
 6 files changed, 15 insertions(+), 23 deletions(-)

Comments

Damien Le Moal March 16, 2022, 3:25 a.m. UTC | #1

On 3/15/22 19:39, John Garry wrote:
> Two loosely related patches are included:
> 
> - Fix for scsi_realloc_sdev_budget_map(). I noticed that the budget token
>   for scsi commands was way in excess of the device queue depth, so I
>   think we need to fix the sbitmap depth. I need to test this more.
> 
> - libata change to use scsi command budget token for qc tag for SAS host.
>   I marked this as RFC as for SAS hosts I don't see anything which
>   guarantees that the budget size is <= 32 always.
>   For libsas hosts we resize the device depth to 32 in the slave configure
>   callback, but this seems an unreliable approach since not all hosts may
>   call this.
>   In addition, I am worried that even if we resize the device depth
>   properly in the slave config callback, we may still try to alloc qc tag
>   prior to this - in lun scan, for example.
>   So we need a way to guarantee that the device queue depth is <= 32
>   always, which I would be open to suggestions for.
> 
> John Garry (2):
>   scsi: core: Fix sbitmap depth in scsi_realloc_sdev_budget_map()
>   libata: Use scsi cmnd budget token for qc tag for SAS host
> 
>  drivers/ata/libata-core.c |  5 +++--
>  drivers/ata/libata-sata.c | 21 ++++-----------------
>  drivers/ata/libata-scsi.c |  2 +-
>  drivers/ata/libata.h      |  4 ++--
>  drivers/scsi/scsi_scan.c  |  5 +++++
>  include/linux/libata.h    |  1 -
>  6 files changed, 15 insertions(+), 23 deletions(-)
> 

I tested this and it is working fine for me. This actually solves the QD
not changing problem I had detected with the pm80xx driver.
Now, doing this:

# cat /sys/block/sde/device/queue_depth
32
# echo 16 > /sys/block/sde/device/queue_depth
# cat /sys/block/sde/device/queue_depth
16

is working as expected.

See my comments on patch 2 for getting final ack and tested tags :)

John Garry March 16, 2022, 8:15 a.m. UTC | #2

On 16/03/2022 03:25, Damien Le Moal wrote:s

Hi Damien,

> I tested this and it is working fine for me. This actually solves the QD
> not changing problem I had detected with the pm80xx driver.
> Now, doing this:
> 
> # cat /sys/block/sde/device/queue_depth
> 32
> # echo 16 > /sys/block/sde/device/queue_depth
> # cat /sys/block/sde/device/queue_depth
> 16
> 

Having this working is down to the first patch. So I will resend that 
patch today separately so that we may look to have it included in 5.18, 
even though we're so late in the cycle...

> is working as expected.
> 
> See my comments on patch 2 for getting final ack and tested tags:)

OK, Thanks,
John