[PATCHv3,0/4] scsi: use xarray for devices and targets

Message

Hannes Reinecke May 28, 2020, 4:36 p.m. UTC

Hi all,

based on the ideas from Doug Gilbert here's now my take on using
xarrays for devices and targets.
It revolves around two ideas:

 - The scsi target 'channel' and 'id' numbers are never ever used
   to the full 32 bit range; channels are well below 10, and no
   driver is using more than 16 bits for the id. So we can reduce
   the type of 'channel' and 'id' to 16 bits, and use the 32 bit
   value 'channel << 16 | id' as the index into the target xarray.
 - Most SCSI LUNs are below 256 (to ensure compability with older
   systems). So there we can use the LUN number as the index into
   the xarray; for larger LUN numbers we'll allocate a separate
   index.

With these changes we can implement an efficient lookup mechanism,
devolving into direct lookup for most cases.
And iteration should be as efficient as the current, list-based,
approach.

This patchset now survives basic testing, hence I've removed
the 'RFC' tag from the initial patchset.

Changes to v1:
- Fixup __scsi_iterate_devices()
- Include reviews from Johannes
- Minor fixes
- Include comments from Doug

Changes to v2:
- Reshuffle hunks as per suggestion from Johannes

Hannes Reinecke (4):
  scsi: convert target lookup to xarray
  target_core_pscsi: use __scsi_device_lookup()
  scsi: move target device list to xarray
  scsi: remove direct device lookup per host

 drivers/scsi/hosts.c               |   3 +-
 drivers/scsi/scsi.c                | 131 ++++++++++++++++++++++++++++---------
 drivers/scsi/scsi_lib.c            |   9 ++-
 drivers/scsi/scsi_scan.c           |  66 ++++++++-----------
 drivers/scsi/scsi_sysfs.c          |  42 ++++++++----
 drivers/target/target_core_pscsi.c |   8 +--
 include/scsi/scsi_device.h         |  21 +++---
 include/scsi/scsi_host.h           |   5 +-
 8 files changed, 179 insertions(+), 106 deletions(-)

Comments

Douglas Gilbert May 29, 2020, 4:21 a.m. UTC | #1

Hannes,
I believe scsi_alloc_target() is broken in the found_target case.
For starters starget is created, built and _not_ put in the xarray,
hence it is leaked?  Seems to me that the code shouldn't go
as far as it does before it detects the found_target case.

What I'm seeing in testing (script attached) after applying my
patches outlined in earlier posts (second attachment) is that
LUN 0 is missing on all targets within a host apart from
target_id==0 . For example:

# modprobe scsi_debug ptype=0x9 no_uld=1 max_luns=2 num_tgts=2
# lsscsi -gs
[0:0:0:0]    comms   Linux    scsi_debug       0189  -  /dev/sg0        -
[0:0:0:1]    comms   Linux    scsi_debug       0189  -  /dev/sg1        -
[0:0:1:1]    comms   Linux    scsi_debug       0189  -  /dev/sg2        -
[N:0:1:1]    disk    INTEL SSDPEKKF256G7L__1     /dev/nvme0n1  -      256GB

# sg_luns /dev/sg2
Lun list length = 16 which imples 2 lun entries
Report luns [select_report=0x0]:
     0000000000000000
     0001000000000000

So where did [0:0:1:0] go? The response to REPORT LUNS says it should
be there.

I thought about jumping into scsi_alloc_target() but it is horribly
complicated, so I'll let you deal with it :-)

Doug Gilbert