After talking with Ed, we have converged. He suggested that the spec
should be
updated to replace the proposed ddi_ssoft_state_init(9F) 'hash_sz'
argument with
an 'n_items' hint-type argument (like ddi_soft_state(9F)) to avoid
exposing details
of a modhash implementation below. This change to a 'hint' facilitates the
implementation switching from a modhash to an avl tree if performance
becomes an issue.
I still need to talk more with Garret. If that does not occur before/at
PSARC
today, we will need to extend the timer.
In either case a new spec will be required.
-Chris
> Hi Ed
>
>
>> hey chris,
>>
>> the current ddi soft state interfaces are "special" in my mind because i
>> thought they were designed to be "fast". (note that i wasn't around for
>> the original design, so my opinion is based off the implmentation.)
>> their implementation uses direct array indexing, avoids locking, and
>> avoids free memory, all seemingly in the name of soft state lookup
>> speed. i guess i'm worried that other people may be making the same
>> assumptions about the performance of the ddi soft state interfaces, and
>> these assumptions don't really carry over to the new interfaces since
>> they are really a thin wrapper around modhash.
>>
>
>
> Yes, performance is an issue for interfaces used in the per-IO code
> path. For the intended use case, the proposed ddi_ssoft_state will not
> be 'directly' involved in the performance path. In the intended use
> case, the performance path 'indirectly' gains access to the
> "target-port" granularity ddi_ssoft_state via a private pointer
> maintained in the finer-grained "target-port,lun" per-scsi_device(9S)
> hba private soft state (PSARC/2008/675). At tran_tgt_init(9E) time of
> a scsi_device(9S), the hba driver is expected to
> ddi_ssoft_state_get(9F) the coarser "target-port" softstate, and
> increment a private "target-port" softstate reference count to account
> for storing a "target-port" softstate pointer in the finer-grained
> per-scsi_device(9S) hba private softstate.
>
>
>
>> hence, i'd rather see something more straitforward, like some
>> version/subset of the modhash interfaces promoted to public, or some new
>> ddi*hash* interfaces.
>>
>
> I want the ability to say "its a softstate, but you feed it strings
> instead of integers". With that simple statement, I believe that anyone
> who has written a solaris driver would "get it". I don't want to loose
> that.
>
> Keep in mind that any part of the device tree that has multiple complex
> addressing levels compressed into a single devinfo node unit-address
> ("@<component-a>,<component-b>,<component-c>") will face this problem -
> which is why I thought the ddi_ssoft_* prefix was warranted. The solution
> to performance path issues will likely involve a private ref_count
> approach similar to our use case above.
>
> If the above explanation is insufficient, to make progress, I am
> willing to change the interface prefix from ddi_ssoft_state_* to
> scsi_taddr_ssoft_state_*, indicating a more restricted use case
> that meets immediate needs.
>
> Thanks
> -Chris
>
from ddi_ssoft_state(9F) proposed man page...
>>> int ddi_ssoft_state_init(ddi_ssoft_state **state_p,
>>> size_t size, size_t hash_sz);
>>>
>>> hash_sz The number of hashed lists which will be allocated;
>>> zero is not allowed. There is a performance
>>> .vs. space tradeoff in the selection of a hash_sz
>>> value: for 'n' soft state structures, on average each
>>> ddi_ssoft_state_get will need to traverse 'n'/hash_sz/2
>>> entries to locate the requested soft state structure.
>>>
>>>
Change to
int ddi_ssoft_state_init(ddi_ssoft_state **state_p,
size_t size, size_t n_items);
...
n_items A hint of the number of items which will be allocated.
