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. 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(). >> 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.) >> >>>> - 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. ed
