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

John Garry posted 2 patches 4 years, 3 months ago
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(-)
[PATCH 0/2] scsi/libata: A potential tagging fix and improvement
Posted by John Garry 4 years, 3 months ago
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(-)

-- 
2.26.2
Re: [PATCH 0/2] scsi/libata: A potential tagging fix and improvement
Posted by Damien Le Moal 4 years, 3 months ago
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 :)

-- 
Damien Le Moal
Western Digital Research
Re: [PATCH 0/2] scsi/libata: A potential tagging fix and improvement
Posted by John Garry 4 years, 3 months ago
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