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

[Bill Davidsen]

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@xxxxxxx>
  CTO TMR Associates, Inc
  Doing interesting things with small computers since 1979
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/