[PATCH] drivers/base/driver.c : driver_unregister

[PATCH] drivers/base/driver.c : driver_unregister

[Arne Caspari]

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

Hi all,

I think the meaning of this patch is obvious: In driver_unregister, the 
bus_remove_driver function call was called outside the driver unload 
semaphore which should obviously protect it.

 /Arne


[-- Attachment #2: driver_unregister.patch --]
[-- Type: text/x-patch, Size: 476 bytes --]

*** linux-2.6.9/drivers/base/driver.c	Mon Oct 18 23:55:06 2004
--- kernel-source-2.6.9/drivers/base/driver.c	Sat Dec 11 10:59:59 2004
***************
*** 106,113 ****
  
  void driver_unregister(struct device_driver * drv)
  {
- 	bus_remove_driver(drv);
  	down(&drv->unload_sem);
  	up(&drv->unload_sem);
  }
  
--- 106,113 ----
  
  void driver_unregister(struct device_driver * drv)
  {
  	down(&drv->unload_sem);
+ 	bus_remove_driver(drv);
  	up(&drv->unload_sem);
  }
  

Re: [PATCH] drivers/base/driver.c : driver_unregister

[Russell King]

On Sat, Dec 11, 2004 at 07:54:32PM +0100, Arne Caspari wrote:
> I think the meaning of this patch is obvious: In driver_unregister, the 
> bus_remove_driver function call was called outside the driver unload 
> semaphore which should obviously protect it.

No.  The semaphore is there to ensure that the function does not
return until the driver structure has a use count of zero.  If you
tested your patch, you'd find that your change would deadlock on
the locked semaphore.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 PCMCIA      - http://pcmcia.arm.linux.org.uk/
                 2.6 Serial core

Re: [PATCH] drivers/base/driver.c : driver_unregister

[Arne Caspari]

Russell King wrote:

>On Sat, Dec 11, 2004 at 07:54:32PM +0100, Arne Caspari wrote:
>  
>
>>I think the meaning of this patch is obvious: In driver_unregister, the 
>>bus_remove_driver function call was called outside the driver unload 
>>semaphore which should obviously protect it.
>>    
>>
>
>No.  The semaphore is there to ensure that the function does not
>return until the driver structure has a use count of zero.  If you
>tested your patch, you'd find that your change would deadlock on
>the locked semaphore.
>  
>

Russell,

Ah, now I understand that thing.  Reading the comments again, I should 
have seen the reason for this earlier.

I am sorry I can not test that patch since unloading of the modules I am 
currently testing blocks anyway. This makes it very hard to test the 
patch :-( and currently this was the reason why I was going to this.

Sorry if I caused any inconvenience.

 /Arne

Re: [PATCH] drivers/base/driver.c : driver_unregister

[Arne Caspari]

Arne Caspari wrote:
> Russell King wrote:
>> No.  The semaphore is there to ensure that the function does not
>> return until the driver structure has a use count of zero.  If you
>> tested your patch, you'd find that your change would deadlock on
>> the locked semaphore.
 >
> I am sorry I can not test that patch since unloading of the modules I am 
> currently testing blocks anyway. This makes it very hard to test the 
> patch :-( and currently this was the reason why I was going to this.

I reverted the code to the original 2.6.9 and unloading of IEEE1394 
modules like 'eth1394' does just that: It deadlocks on this semaphore.

At least this is a good excuse why I was not able to test my patch ;-) 
The behaviour just remained the same as before...

Btw. I am developing/debugging on a machine without serial/parallel 
ports. Is there a way to connect a kernel mode debugger to this. I am 
used to windows development and there the debugger works on a IEEE1394 
connection. Does anybody have hints to improve development on such a 
machine?


Thanks,

	/Arne

Re: [PATCH] drivers/base/driver.c : driver_unregister

[Matt Mackall]

On Mon, Dec 13, 2004 at 08:21:10AM +0100, Arne Caspari wrote:
> Arne Caspari wrote:
> >Russell King wrote:
> >>No.  The semaphore is there to ensure that the function does not
> >>return until the driver structure has a use count of zero.  If you
> >>tested your patch, you'd find that your change would deadlock on
> >>the locked semaphore.
> >
> >I am sorry I can not test that patch since unloading of the modules I am 
> >currently testing blocks anyway. This makes it very hard to test the 
> >patch :-( and currently this was the reason why I was going to this.
> 
> I reverted the code to the original 2.6.9 and unloading of IEEE1394 
> modules like 'eth1394' does just that: It deadlocks on this semaphore.
> 
> At least this is a good excuse why I was not able to test my patch ;-) 
> The behaviour just remained the same as before...
> 
> Btw. I am developing/debugging on a machine without serial/parallel 
> ports. Is there a way to connect a kernel mode debugger to this. I am 
> used to windows development and there the debugger works on a IEEE1394 
> connection. Does anybody have hints to improve development on such a 
> machine?

Netconsole, and/or kgdb-over-ethernet? You'll need a -mm or -tiny
kernel for the latter, or just pull the patches individually.

-- 
Mathematics is the supreme nostalgia of our time.