[PATCH] info to atm layer about broken connection
stf_xl at wp.pl
Mon Oct 31 09:58:11 EST 2005
Hi again Duncan
> >> ... But it seems to me you don't really want synchronous shutdown
> > > anyway: you just want the connection to be killed.
> > Yes, vcc should be closed asynchronously but vcc_release_async() don't work now,
> > does it ?
> if it doesn't work then that's a reason to fix it, not to work around it.
ATM layer is quite complex, however it seems vcc_release_async()
set ATM_VF_CLOSE flag but no one test it later.
> > AFAIK try_module_get() in usbatm_atm_open() and module_put() in usbatm_atm_close()
> > prevent unload module when there is atm connection, so there shouldn't be
> > any deadlock with rmmod.
> You still haven't explained why you want to prevent the module from being unloaded.
> As I said early on, I want it to be possible to unload the module at anytime. That's
> not set in stone, but in order for it to be changed you need to supply a good reason.
> Right now, the module can be unloaded at any moment; the module unload disconnects
> the device, so the effect of unloading is the same as unplugging the device. I'm
> guessing that the things you don't like the current module unloading behaviour are
> exactly the same things that annoy you when the device is unplugged. Is that right?
Well I rather think that try_module_get(subdriver_module) is standard behavior,
for example atm layer prevent unload usbatm module when there exist vcc
(functions __vcc_connect() and vcc_destroy_socket() in net/atm/common.c).
So usbatm should also prevent unload subdriver module when is needed.
> > My patch send NULL skb only when modem is physically disconnect form usb port.
> It is impossible to tell the difference between a physical disconnect
> and a logical disconnect (= module unload, usbfs ioctl, hotplug restart).
> Your patch sends a NULL skb also when the module is unloaded for example.
I didn't know usb_driver.disconnect can be called in usbfs or hotplug context,
I thought only when module is unloaded or device removed.
If we assume usb_driver.disconnect is called only in such situations,
using try_module_get() prevent call usbatm_usb_disconnect when
vcc list is not empty.
> > I think it is good to synchronously shutdown connection in such situation.
> Why is this better than asynchronous shutdown? Does your patch even do a
> synchronous shutdown? I don't see it waiting for the vcc's to go down.
> Maybe there is some confusion here about the meaning of "synchronous
> shutdown". There are two things here: one is to put the socket in a state
> in which it understands that the connection is closed. This can be done
> by the driver without doing any waiting; this is what I mean by asynchronous
> shutdown. The other thing is the actual destruction of the vcc, which
> doesn't happen (as far as I know) until the userspace application closes the
> socket. So if you don't want USB device disconnection to complete before all
> vccs have been destroyed, then you have to wait for the vccs to go down;
> this is synchronous shutdown. I hope it is now clear what I mean by
> asynchronous vs synchronous shutdown. Which one do you think is best,
> and why? I don't want to do synchronous shutdown unless there is an
> excellent reason to do so.
Ah, I simply understand that push->(vcc, NULL) is sync shutdown
and call vcc_release_async() is async shoutdown.
> > I review push function in pppoatm, clip and br2684 drivers, all of them expect
> > to get NULL skb and it is interpreted in the same manner - link is broken.
> Yes, but who should send it? Check out this thread:
Conclusion of the thread is using vcc_release_async().
More information about the Usbatm