On (09/17/07 09:57), Artem Kachitchkine wrote:
>
>
> http://cr.opensolaris.org/~artem/bru-prophandle/
>
looks good- some minor questions:
ipw2100_impl.h:
need to include <net/if.h> somewhere? seems like IFF_PROMISC is used
in that driver.
mac.c
- mac_setprop():
in the (!init_link) case, we only want to commit the
changes via mac_prop_update if the mc_setprop callback itself
succeeds otherwise we will have divergence between the saved list
and the actual value (for example if input value is invalid). In
the init_link case we want to only do mac_prop_update.
So how about doing this as
if (!init_link) {
mac_open(..);
if (mip->mi_callbacks->mc_setprop(...) != 0) {
if error is not EBUSY
goto bail;
}
mac_close
}
mac_prop_update();
return err;
- what is the expected typical/atypical driver usage? I think
we had discussed possible cases where the driver caches the mph, and
talked about how to deal with cached prop handles by using
refcounts. Is that deferred for a separate wad of changes?
Fortunately for the mph (unlike capabilities and other info on the
mac_impl_t[#]), we don't currently have any mac clients that are
interested in the cached prop handle so one solution is to track the
mph via refcounts and a "dirty" bit- when mac_stop is executed, if
the refcount on the mph is not yet quiesced, we can mark the mph as
dirty, and load a new list at the next mac_start. The dirty mph
should be cleaned up when the last ref on it goes to 0.
[#] then again, are there other benefits to keeping a cached snapshot
of the properties for mac clients? For example, will having a cached
snapshot will allow us to deflect getprop requests without actually
contacting the driver? Maybe there is potential for a fastpath here.
- nit: the XXX at line 2424 should be fixed in the gate by now,
after Samant's putback, so this comment can be removed.
- Not an urgent priority, more an RFE:
would be nice to have an mdb walker for the mac property list.
This changes the interfaces in Section 4 of the design doc.
If you have any edits for that section (maybe its better to leave
specifics out, and ARC the details later for the Brussels-persistence
case), please send them my way.
--Sowmini