Re: scsi_alloc_target: parent of the target (need not be a scsi host)

[Douglas Gilbert <dgilbert@interlog.com>]

On 2020-05-10 2:52 p.m., James Bottomley wrote:
> On Sun, 2020-05-10 at 14:32 -0400, Douglas Gilbert wrote:
>> This gem is in scsi_scan.c in the documentation of that function's
>> first argument. "need not be a scsi host" should read "it damn well
>> better be a scsi host" otherwise that function will crash and burn!
> 
> It shouldn't: several transport classes, like SAS and FC have
> intermediate devices between the host and the target and they all work
> just fine using the non-host parent.  Since you don't give the error
> this is just guesswork, but the host has to be somewhere in the parent
> chain otherwise dev_to_shost(parent) will return NULL ... is that your
> problem?

May be that "need not be a scsi host" should be expanded to something
that suggests dev_to_shost(first_arg) can resolve to a non-NULL pointer.

Are there restrictions on those intermediate object(s)? Can there be more
than one? If so, can those intermediate objects only form a linear chain
or could it be more complex, an object subtree for example?

>> I'm trying to work out why the function: starget_for_each_device() in
>> scsi.c does _not_ use that collection right in front of it (i.e.
>> scsi_target::devices). Instead, it step up to the host level, and
>> iterates over all devices (LUs) on that host and only calls the given
>> function for those devices that match the channel and target numbers.
>> That is bizarrely wasteful if scsi_target::devices could be iterated
>> over instead.
>>
>> Anyone know why this is?
> 
> Best guess would be it wasn't converted over when the target list was
> introduced.

Okay, I'll change it and see what breaks.


If you are not familiar with "mark"s in xarrays, they are binary flags
hidden inside the pointers that xarray holds to form a container. With
these flags (two available per xarray) one can make iterations much more
efficient with xa_for_each_marked() { }. The win is that there is no
need to dereference the pointers of collection members that are _not_
marked. After playing with these in my rework of the sg driver, I
concluded that the best thing to mark was object states. Unfortunately
there are only 2 marks *** per xarray but scsi_device, for example, has
9 states in scsi_device_state. Would you like to hazard a guess of
which two (or two groups), if any, are iterated over often enough to
make marking worthwhile?

Those two marks could be stretched to four, sort of, as scsi_device
objects are members of two xarrays in my xarray rework: all sdev objects
in a starget, and all sdev objects in a shost.

Doug Gilbert


*** two flags is only _two_ sub-list possibilities per xarray. So, for
     example, you can't iterate like this (directly):
         xa_for_each_marked(mark1_set && mark2_clear) { }.