Re: [PATCH] fix bad locking in drivers/base/driver.c

[Bill Davidsen <davidsen@tmr.com>]

Mike Waychison wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> Greg KH wrote:
> 
>>On Mon, Jan 24, 2005 at 07:25:19PM +0100, Jirka Kosina wrote:
>>
>>
>>>Hi,
>>>
>>>there has been (for quite some time) a bug in function driver_unregister() 
>>>- the lock/unlock sequence is protecting nothing and the actual 
>>>bus_remove_driver() is called outside critical section.
>>>
>>>Please apply.
>>
>>
>>No, please read the comment in the code about why this is the way it is.
>>The code is correct as is.
>>
> 
> 
> Why don't we clean this up as in the proposed attached patch (against
> 2.6.10).  Compile-tested only.

Let's clean up the spelling as well
> 
> - --
> Mike Waychison
> Sun Microsystems, Inc.
> 1 (650) 352-5299 voice
> 1 (416) 202-8336 voice
> 

> Index: linux-2.6.10/drivers/base/driver.c
> ===================================================================
> --- linux-2.6.10.orig/drivers/base/driver.c	2004-12-24 16:35:25.000000000 -0500
> +++ linux-2.6.10/drivers/base/driver.c	2005-01-25 02:16:31.000000000 -0500
> @@ -79,14 +79,14 @@ void put_driver(struct device_driver * d
>   *	since most of the things we have to do deal with the bus
>   *	structures.
>   *
> - *	The one interesting aspect is that we initialize @drv->unload_sem
> - *	to a locked state here. It will be unlocked when the driver
> - *	reference count reaches 0.
> + *	The one interesting aspect is that we setup @drv->unloaded
> + *	as a completion that gets complete when the driver reference 
> + *	count reaches 0.
>   */
>  int driver_register(struct device_driver * drv)
>  {
>  	INIT_LIST_HEAD(&drv->devices);
> -	init_MUTEX_LOCKED(&drv->unload_sem);
> +	init_completion(&drv->unloaded);
>  	return bus_add_driver(drv);
>  }
>  
> @@ -97,7 +97,7 @@ int driver_register(struct device_driver
>   *
>   *	Again, we pass off most of the work to the bus-level call.
>   *
> - *	Though, once that is done, we attempt to take @drv->unload_sem.
> + *	Though, once that is done, we wait until @drv->unloaded is copmleted.
------------------------------------------------------------------>completed
>   *	This will block until the driver refcount reaches 0, and it is
>   *	released. Only modular drivers will call this function, and we
>   *	have to guarantee that it won't complete, letting the driver

-- 
bill davidsen <davidsen@tmr.com>
   CTO TMR Associates, Inc
   Doing interesting things with small computers since 1979