usbatm : modprobe & rmmod

[Duncan Sands]

Hi Roman,

> On Tue, 2005-01-25 at 12:25 +0100, Duncan Sands wrote:
> > what do you want this flexibility for?  As far as I can see cxacru has no
> > need for it.
> 
> Exactly.  Frankly, cxacru even needs less than is provided by usbatm2.
> I just tried to think a bit more generically, e.g. considering the cases
> with multiple interfaces.

by the way there are several different ways multiple interfaces can come into play:
(1) an interface for transmit+receive, an interface for firmware loading, an
interface for line state (like the speedtouch).  This fits fine into usbatm2,
since there is basically a one-to-one correspondance (ATM device) <-> transmit/receive
interface.
(2) one interface for transmission, another for reception (like eagle-usb).
I think this can be made to work fine with usbatm2 also: when bind is called on the
first interface, it grabs the second interface too (usb_driver_claim_interface).
Unbind drops both interfaces (usb_driver_release_interface).  The only problem is
that you need the struct usb_driver, but that's easy to arrange (just stick it in
usbatm_data for instance).
(3) some other way I didn't think of.

As far as I can see, everything is fine (or can easily be made to be fine) with
the usbatm2 setup.  Perhaps Matthieu can comment.

> > > In a few words, the subdriver now registers a struct usbatm_driver with
> > > usbatm_init at probe time, and, once the device is ready for ATM, calls
> > > usbatm_atm_setup to create an ATM device.
> > 
> > What I don't like is that the driver lifecycle is partly managed by the core,
> > partly by the minidriver, with each of them calling back into the other.
> 
> The lifecycle here is managed by the core too (I didn't even had to
> export udsl_{put,get}_instance).  However, the minidriver and the core
> do call into each other, and the correct operation does depend on the
> calling order, i.e. usbatm_init should be called only in minidriver's
> probe, usbatm_atm_setup only in probe or heavy_init, and
> usbatm_disconnect only in disconnect.  Dunno how much of a problem it
> is.

I guess the main problem is that I don't like it :)

> >   In
> > usbatm2, the core controls the whole lifecycle, and simply calls the minidriver
> > at each significant moment (bind, atm_start, atm_stop, unbind.  This makes
> > things extremely regular and the lifecycle simple to understand.
> 
> Right, provided the minidriver fits into this rigid framework.  cxacru
> certainly does, but I've been wondering if Matthieu's eagle-usb does
> too.

Good question.  About the rigidity (which is the flip side of simplicity and
regularity): for me the main question is: does it enhance maintainability/
correctness, or does it encourage hacks to work around it?  If it pushes
minidriver authors towards a clean design, and reduces the chances of writing
wrong code, then it is a good thing.  If it is rigid in the wrong way, making
it impossible to have minidrivers work without hacks to get around the usbatm2
setup, then that is bad.  The problem is that it is hard to judge this kind of
thing in advance.  As more minidrivers get written, it should become clearer.
My feeling is that the usbatm2 design is basically sound.

> > I think synchronous thread shutdown will work fine, and it simplifies
> > thinking about driver and the writing of minidrivers (race conditions).
> 
> Unless I've screwed it up again, the minidriver shouldn't care: all the
> refcounting is handled by the core.  OTOH the minidriver's heavy_init
> can now take its time, without caring much about being quick to cancel.
> And it doesn't look to be more complex than the synchronous shutdown.

I had two things in mind when I made it synchronous: (1) future writers of
minidrivers, maybe with more complicated heavy_init routines than we have,
are less likely to write racy code with a synchronous setup (you could
retort that they are more likely to write code that holds up the khub thread
for a long time in the synchronous setup, by not exiting their heavy_init
thread promptly); (2) with the synchronous setup it is possible to unload
the kernel module at any time, even during the heavy_init (with asynchronous
init, you have to take a module reference which blocks module unloading).
Consider the synchronous version as an experiment.

> > > The updated cxacru seems to behave well under various conditions,
> > > including disconnect, module unloading, etc.
> > 
> > Great!
> 
> Just in case: I meant the old cxacru adjusted to match the updated
> usb_atm, not cxacru2 working with usbatm2 :).

Sure, I understood.

Ciao,

D.