[PATCH] info to atm layer about broken connection

[Stanislaw Gruszka]

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:
> 
> http://www.atm.tut.fi/list-archive/linux-atm/msg06111.html
Conclusion of the thread is using vcc_release_async(). 

__
Regards
Staszek Gruszka