mbox series

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

Message ID 20200527141400.58087-1-hare@suse.de (mailing list archive)
Headers show
Series scsi: use xarray for devices and targets | expand

Message

Hannes Reinecke May 27, 2020, 2:13 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:
- 'channel' and 'id' 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 change 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 is compile-tested only, to give you an impression of the
overall idea and to get the discussion rolling.

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                | 129 ++++++++++++++++++++++++++++---------
 drivers/scsi/scsi_lib.c            |   8 +--
 drivers/scsi/scsi_scan.c           |  66 +++++++++----------
 drivers/scsi/scsi_sysfs.c          |  39 +++++++----
 drivers/target/target_core_pscsi.c |   8 +--
 include/scsi/scsi_device.h         |  21 +++---
 include/scsi/scsi_host.h           |   5 +-
 8 files changed, 175 insertions(+), 104 deletions(-)

Comments

Bart Van Assche May 27, 2020, 4:36 p.m. UTC | #1
On 2020-05-27 07:13, Hannes Reinecke wrote:
> 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:
> - 'channel' and 'id' 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 change 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 is compile-tested only, to give you an impression of the
> overall idea and to get the discussion rolling.

Hi Hannes,

My understanding of the xarray concept is that it provides two
advantages over using linked lists:
- Faster lookups.
- Requires less memory.

Will we benefit from any of these advantages in the SCSI code? Hadn't
James Bottomley already brought up that lookup by (channel, target, lun)
only happens from some LLDs and from the procfs code?

Are there any use cases where the number of SCSI devices is large enough
to benefit from the memory reduction?

Thanks,

Bart.
Hannes Reinecke May 27, 2020, 4:59 p.m. UTC | #2
On 5/27/20 6:36 PM, Bart Van Assche wrote:
> On 2020-05-27 07:13, Hannes Reinecke wrote:
>> 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:
>> - 'channel' and 'id' 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 change 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 is compile-tested only, to give you an impression of the
>> overall idea and to get the discussion rolling.
> 
> Hi Hannes,
> 
> My understanding of the xarray concept is that it provides two
> advantages over using linked lists:
> - Faster lookups.
> - Requires less memory.
> 
> Will we benefit from any of these advantages in the SCSI code? Hadn't
> James Bottomley already brought up that lookup by (channel, target, lun)
> only happens from some LLDs and from the procfs code?
> 
It's not only lookup, it's iteration in general.
Which affects scanning and device removal; especially the latter is 
_very_ error prone (just look at scsi_target_reap etc), so any reduction 
in complexity is a good thing in general methinks.

> Are there any use cases where the number of SCSI devices is large enough
> to benefit from the memory reduction?
> 
I would assume that we're seeing benefits as soon as we're in the range 
of tens to hundreds of devices; then list lookup will be eating up more 
time and space as xarrays.

And the big benefit of using xarrays is that we will be alerted if an 
element with the same indices is being added; we've already had issues 
in the past here which are notoriously difficult to track down.

Cheers,

Hannes
Douglas Gilbert May 28, 2020, 3:59 a.m. UTC | #3
On 2020-05-27 12:36 p.m., Bart Van Assche wrote:
> On 2020-05-27 07:13, Hannes Reinecke wrote:
>> 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:
>> - 'channel' and 'id' 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 change 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 is compile-tested only, to give you an impression of the
>> overall idea and to get the discussion rolling.
> 
> Hi Hannes,
> 
> My understanding of the xarray concept is that it provides two
> advantages over using linked lists:
> - Faster lookups.
> - Requires less memory.

Bart,
You might add these:
   - sane deletion semantics
   - inbuilt locking
   - inherently safer iterations, especially if marks are used

Matthew can probably add more.

> Will we benefit from any of these advantages in the SCSI code? Hadn't
> James Bottomley already brought up that lookup by (channel, target, lun)
> only happens from some LLDs and from the procfs code?

The way that the SCSI object tree hangs together with doubly linked
lists may have been a coherent design 20 years ago when JB wrote it,
but it has been white-anted big time since then.

The current state of that code is hard to defend. I have between 10 and 20
more examples of patently stupid things the current code does. See my
exchange with JB concerning the starget iterator over its sdevs that decided
to check on _every_ sdev in that host. That is done in several places.

The redundant sdevs in a shost collection is probably the most glaring
current design flaw.

> Are there any use cases where the number of SCSI devices is large enough
> to benefit from the memory reduction?

I don't believe that overall memory usage is a problem. Fitting the sdev_s
of hot devices in a smaller number of cache lines would help. That is
where Ming Lei was looking that kicked off this exercise that has
morphed into using xarray.

Doug Gilbert