Chris Horne wrote:
> Hi Garrett
>
>
>> I have the strong feeling that this case is trying to conflate a general
>> purpose hash table with soft state storage. While drivers could
>> potentially use a hash table for soft state storage, it seems that
>> creating a limited purpose set of routines which are really just
>> accessors into a hash table structure might not be the best approach.
>>
>> Most previous usages of "soft state" consider one soft state structure
>> per device-instance (dip), whereas it appears that this project
>> considers the needs of a nexus driver to store state for each of
>> potentially multiple child devices. This seems more like a general data
>> structure problem to me.
>>
>> What is wrong with the modhash routines as they are today? Can't
>> drivers just use those directly?
>>
>
>
> The 'modhash.h' interfaces are not ARCed, they are not documented
> (AFAIK), they do not present an API familiar to driver writers, and
> their generality brings with it complexity.
>
> The ddi_ssoft_state* implementation is build on top of 'modhash.h'.
>
> The eventual promotion of the ddi_ssoft_state* interfaces is tied to
> SCSAv3 enhancements, where a DDI compliant mechanism to support
> soft_state for complex target port unit addresses is needed.
>
> The proposed ddi_ssoft_state* interfaces have the same API profile as
> the current ddi_soft_state* API, making them (in my opinion) more
> appropriate for inclusion as part of the DDI: they minimize the volume
> of solaris-specific API that a driver writer needs to understand by
> building on a well-established profile.
>
But if what is needed is generic hash table facilities, then wouldn't it
be more appropriate to uplevel the modhash routines or create more
general purpose wrappers.
I'm a big fan of reducing duplication in the DDI. While your goals seem
to be inline with that, I'm not sure that this project achieves it as
well as just providing the generic hash facility would.
FWIW, a lot of drivers these days don't even bother with the
ddi_soft_state stuff. I find them personally rather clumsy, and opt to
use either my own stuff, or more commonly, if I can just use the private
state pointer in the dev_info, I just use that.
>
>
>> Put another way, how does this case improve life for drivers beyond just
>> using modhash directly?
>>
>> I'm also not sure I understand the point of ddi_get_isoft_state. Is the
>> "typed" adjective here a reference to the fact that the first argument
>> (the master state pointer) is typed to something other than void *? (Do
>> we really believe that there significant cases here where the type
>> safety this would afford justifies the cost of introducing another
>> interface? I confess that I'm very skeptical of that.)
>>
>
>
> I added the ddi_isoft_state* interfaces so that the same level of type
> checking was available for both ddi_ssoft_state* and the indexed case.
>
> If a consensus objection persists, I can drop ddi_isoft_state* from the
> proposal.
>
It seems like a small benefit, and comes at the cost of "seemingly
arbitrary" difference between the legacy API and new API. (Possibly
creating additional burden for folks working to support drivers on
multiple releases of Solaris... of course, we *never* backport drivers,
right? :-)
-- Garrett
