Edward Pilatowicz wrote: > On Tue, Jul 07, 2009 at 06:00:35PM -0700, Garrett D'Amore wrote: > >> Edward Pilatowicz wrote: >> >>> On Mon, Jul 06, 2009 at 03:09:34PM -0700, Garrett D'Amore wrote: >>> >>> >>>> Edward Pilatowicz wrote: >>>> >>>> >>>>> hey garrett, >>>>> some quick questions. >>>>> >>>>> - will this fall back behaviour be supported for CLONE_DEV based minor >>>>> nodes? (i assume not.) >>>>> >>>>> >>>>> >>>> I think it would be. The only thing special about clone opens is that >>>> some minor number (typically 0 for NIC devices) is "special" and may not >>>> be associated with a physical instance. There isn't anything I can think >>>> of that would preclude this behavior. >>>> >>>> Granted, I would really, really like to see the need for CLONE_DEV >>>> devices go away. (I think this is largely a DLPI style 2 artifact. >>>> Everyone should DLPI style 1.) >>>> >>>> >>>> >>> really? i think that supporting this fall back behaviour for CLONE_DEV >>> based minor nodes is a would introduce unnecessary complexity. >>> >>> putting aside the issue that we'd all like to see CLONE_DEV go away, the >>> clone driver was never designed to work with non-streams devices. you'd >>> probably have to modify it pretty extensively to support this fall back >>> to cb_ops mode. either that or you'd have to add more clone dev device >>> specific knowledge to spec_open(). >>> >>> >> Why? Wouldn't a device driver just return ENOSTR just like any other? >> >> In order to be most useful, this feature has to be available to NIC >> devices, which make use of CLONE_DEV for DLPI style 2 (*cough*). I don't >> see why any interaction with the clone device driver is at all required. >> >> > > because during a CLONE_DEV open, spec_open() invokes the clone drivers > streams open entry point. that in turn invokes the target drivers > streams open entry point. if that returns ENOSTR, then the clone driver > will pass that result back to spec_open(). so then with your changes > spec_open() will attempt to invoke the clone drivers cb_ops open entry > point, which doesn't exist. >
That's actually OK. Because you'd never try a CLONE_DEV open unless you wanted STREAMS semantics. My point is that a device case can export a minor node that supports that CLONE_DEV minor node, and a minor node that doesn't. (In fact, all GLDv3 drivers do this... look at /dev/iprb vs. /dev/iprb0 (or pick your NIC driver) for an example. I just want to be able to add other kinds of minor nodes. > hence my comments that to support this you'll need to change the clone > driver (to create a cb_ops open entry point for it) or you'll need to > bake more clone driver specific knowledge into spec_open(). > I don't need to do either. I need not support a CLONE_DEV falling back to cb_ops. That would be nonsensical, IMO. Supporting mixing and matching of CLONE_DEV, STREAMS, and regular minor nodes in the same device instance *is* a goal, and that works even without changes to the clone dev or special knowledge in specfs. > >>> also, CLONE_DEV open semantics wrt dev_t - dip bindings are very >>> different from normal device opens. opens of CLONE_DEV minor nodes >>> result in a dev_t that isn't bound to any dip. all non CLONE_DEV opens >>> are required to be bound to a dip before the open can proceed. if you >>> try to support fall back for these nodes it will be impossible to >>> establish this binding before invoking the cb_ops open routine because >>> you don't know what the cloned dev_t will be. so essentially you'd be >>> introducing a new window where a driver open routine could be invoke on >>> a minor node that is not actually bound to any device instance. >>> >>> >> Yes, but the window is already there. Its a defect of CLONE_DEV. Either >> way you need a minor node to be out there, either via the clone device >> or via a "real" node. If a GLDv3 NIC driver creates a clone dev, then >> the minor node will still be available. >> >> Let me be clearer here: it will not be possible for the clone device >> driver to yield a reference to a non-STREAMS device node. But if a >> driver using the new semantics wants to export a CLONE_DEV node, there >> isn't a problem. It can also export regular nodes. No problem. >> > > ok. so it sounds like we're actually on the same page. the fallback > behavior is NOT supported for CLONE_DEV based minor nodes. all > CLONE_DEV based minor nodes MUST return a stream. (but the driver can > feel free to create other non CLONE_DEV minor nodes which can function > as non-stream nodes.) > *RIGHT*. Okay, yes, we're on the same page then. :-) > >>>>> - instead of having the streams open entry point return ENOSTR to fall >>>>> back to cb_ops, did you consider adding a new flag to >>>>> ddi_create_minor_node() so that a driver could specify the desired >>>>> access semantics of a given minor node? >>>>> >>>>> >>>>> >>>> I didn't think of that... I originally considered a dev_ops flag to >>>> trigger the dynamic behavior, but decided I didn't need one. >>>> >>>> I'd have to contemplate whether the minor node flag would even be >>>> workable... it probably is. What would the benefit be, though? >>>> >>>> >>>> >>> the benefit would be that you wouldn't have to open a device to know how >>> it's going to behave. with the current semantics, until you open the >>> device you don't know how it should be accessed. this could be used by >>> consumers to help decide how they want to access a device. it could >>> also be used by the framework to verify that a device is behaving as >>> expected. (think more ASSERTs.) >>> >>> >> I see a lot of new code, and very little benefit gained. For nodes that >> export both STREAMS and character nodes, it would save an extra open() >> call. But apart from that, there is no tangible benefit. >> >> To see my sample implementation (which I've now tested with a modified >> version of my audio framework), see >> >> http://cr.opensolaris.org/~gdamore/schism/ >> >> Its only 18 lines of change, and yet it *works*. KISS. >> >> > > yeah. i looked briefly at that. i noticed you didn't make any updates > to the ldi. i expect you'll need to. you'll probably want to include > test cases for accessing one of your fallback device nodes via the ldi. > Good point... I'll have a look see. I'd have thought ldi would go thru specfs. Does it not? - Garrett > ed >
