[PATCH 1/3] usbatm: Allow sub-drivers to handle calls to atm_proc_read.

[PATCH 1/3] usbatm: Allow sub-drivers to handle calls to atm_proc_read.

[Simon Arlott]

usbatm only outputs basic information via the per-device /proc/net/atm/ file, this patch allows the device specific USB ATM drivers to replace the atm_proc_read function with their own.

Signed-off-by: Simon Arlott <simon@fire.lp0.eu>

---
 drivers/usb/atm/usbatm.c |    3 +++
 drivers/usb/atm/usbatm.h |    3 +++
 2 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/drivers/usb/atm/usbatm.c b/drivers/usb/atm/usbatm.c
index ec63b0e..d91ed11 100644
--- a/drivers/usb/atm/usbatm.c
+++ b/drivers/usb/atm/usbatm.c
@@ -761,6 +761,9 @@ static int usbatm_atm_proc_read(struct a
 		return -ENODEV;
 	}
 
+	if (instance->driver->proc_read != NULL)
+		return instance->driver->proc_read(instance, atm_dev, pos, page);
+
 	if (!left--)
 		return sprintf(page, "%s\n", instance->description);
 
diff --git a/drivers/usb/atm/usbatm.h b/drivers/usb/atm/usbatm.h
index ff8551e..d3c0ee4 100644
--- a/drivers/usb/atm/usbatm.h
+++ b/drivers/usb/atm/usbatm.h
@@ -121,6 +121,9 @@ struct usbatm_driver {
 	/* cleanup ATM device ... can sleep, but can't fail */
 	void (*atm_stop) (struct usbatm_data *, struct atm_dev *);
 
+	/* called when the proc file is read */
+	int (*proc_read) (struct usbatm_data *, struct atm_dev *, loff_t * pos, char *page);
+
         int bulk_in;	/* bulk rx endpoint */
         int isoc_in;	/* isochronous rx endpoint */
         int bulk_out;	/* bulk tx endpoint */
-- 
1.4.3.1


-- 
Simon Arlott (subscribed to lkml, don't CC)

Re: [PATCH 1/3] usbatm: Allow sub-drivers to handle calls to atm_proc_read.

[Duncan Sands]

> usbatm only outputs basic information via the per-device /proc/net/atm/ file,
> this patch allows the device specific USB ATM drivers to replace the
> atm_proc_read function with their own.  

I'm still meditating on this.  The reason I didn't do this originally is
because of potential problems with modem disconnection and/or module
unloading (the cxacru module can be unloaded at any time - it's the usbatm
module that can't be unloaded when a connection is open - so you've got to
be careful that no-one can call into cxacru after or while it's being
destroyed).  I think it will be OK as long as usbatm calls unbind after
shutting down the ATM layer (since otherwise your read method could be
called after you've freed your cxacru private data) which is not the case
right now, but should be easy to arrange.  Horrible things may happen
if proc_read can still be running after atm_dev_deregister has returned,
but, if so, horrible things can already happen right now.  I don't understand
why this is impossible; maybe it is possible.  The worst that will happen
(given that none of the proc_read methods sleeps) is that freed memory will
be accessed and the contents spat out in the proc file (if proc_read sleeps,
that could result in trying to run code inside a destroyed module).

Ciao,

Duncan.

Re: [PATCH 1/3] usbatm: Allow sub-drivers to handle calls to atm_proc_read.

[Simon Arlott]

[-- Attachment #1: Type: text/plain, Size: 1534 bytes --]

On 31/01/07 14:48, Duncan Sands wrote:
>> usbatm only outputs basic information via the per-device /proc/net/atm/ file,
>> this patch allows the device specific USB ATM drivers to replace the
>> atm_proc_read function with their own.  
> 
> I'm still meditating on this.  The reason I didn't do this originally is
> because of potential problems with modem disconnection and/or module
> unloading (the cxacru module can be unloaded at any time - it's the usbatm
> module that can't be unloaded when a connection is open - so you've got to
> be careful that no-one can call into cxacru after or while it's being
> destroyed).  I think it will be OK as long as usbatm calls unbind after
> shutting down the ATM layer (since otherwise your read method could be
> called after you've freed your cxacru private data) which is not the case
> right now, but should be easy to arrange.  Horrible things may happen
> if proc_read can still be running after atm_dev_deregister has returned,
> but, if so, horrible things can already happen right now.  I don't understand
> why this is impossible; maybe it is possible.  The worst that will happen
> (given that none of the proc_read methods sleeps) is that freed memory will
> be accessed and the contents spat out in the proc file (if proc_read sleeps,
> that could result in trying to run code inside a destroyed module).
> 
> Ciao,
> 
> Duncan.

Couldn't the cxacru instance pointer to the proc_read function be set to NULL before unloading?

-- 
Simon Arlott


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 829 bytes --]

Re: [PATCH 1/3] usbatm: Allow sub-drivers to handle calls to atm_proc_read.

[Duncan Sands]

> Couldn't the cxacru instance pointer to the proc_read function be set to NULL before unloading?

The problem is reads that started (on some other CPU) before you started shutting things down
(eg: but setting this to null or whatever other method you like) and only finish after you have
finished shutting things down.  Or rather, never finish at all because the code they are executing
has been deleted from the kernel, causing an Oops.  This is not our problem: we can't do anything
about it: remove_proc_entry needs to be fixed so that it waits for all readers/writers of the proc
file to finish before returning, IMO.

Best wishes,

Duncan.