From: viro@parcelfarce.linux.theplanet.co.uk Add and use the new parport mutex. --- 25-akpm/drivers/parport/share.c | 129 +++++++++++----------------------------- 1 files changed, 36 insertions(+), 93 deletions(-) diff -puN drivers/parport/share.c~PP1-parport_locking-RC1 drivers/parport/share.c --- 25/drivers/parport/share.c~PP1-parport_locking-RC1 Wed Jan 14 13:50:48 2004 +++ 25-akpm/drivers/parport/share.c Wed Jan 14 13:50:48 2004 @@ -49,7 +49,8 @@ static LIST_HEAD(all_ports); static spinlock_t full_list_lock = SPIN_LOCK_UNLOCKED; static struct parport_driver *driver_chain = NULL; -static spinlock_t driverlist_lock = SPIN_LOCK_UNLOCKED; + +static DECLARE_MUTEX(registration_lock); /* What you can do to a port that's gone away.. */ static void dead_write_lines (struct parport *p, unsigned char b){} @@ -102,55 +103,19 @@ static struct parport_operations dead_op /* Call attach(port) for each registered driver. */ static void attach_driver_chain(struct parport *port) { + /* caller has exclusive registration_lock */ struct parport_driver *drv; - void (**attach) (struct parport *); - int count = 0, i; - - /* This is complicated because attach() must be able to block, - * but we can't let it do that while we're holding a - * spinlock. */ - - spin_lock (&driverlist_lock); for (drv = driver_chain; drv; drv = drv->next) - count++; - spin_unlock (&driverlist_lock); - - /* Drivers can unregister here; that's okay. If they register - * they'll be given an attach during parport_register_driver, - * so that's okay too. The only worry is that someone might - * get given an attach twice if they registered just before - * this function gets called. */ - - /* Hmm, this could be fixed with a generation number.. - * FIXME */ - - attach = kmalloc (sizeof (void(*)(struct parport *)) * count, - GFP_KERNEL); - if (!attach) { - printk (KERN_WARNING "parport: not enough memory to attach\n"); - return; - } - - spin_lock (&driverlist_lock); - for (i = 0, drv = driver_chain; drv && i < count; drv = drv->next) - attach[i++] = drv->attach; - spin_unlock (&driverlist_lock); - - for (count = 0; count < i; count++) - (*attach[count]) (port); - - kfree (attach); + drv->attach(port); } /* Call detach(port) for each registered driver. */ static void detach_driver_chain(struct parport *port) { + /* caller has exclusive registration_lock */ struct parport_driver *drv; - - spin_lock (&driverlist_lock); for (drv = driver_chain; drv; drv = drv->next) drv->detach (port); - spin_unlock (&driverlist_lock); } /* Ask kmod for some lowlevel drivers. */ @@ -178,7 +143,7 @@ static void get_lowlevel_driver (void) * pointer it must call parport_get_port() to do so. Calling * parport_register_device() on that port will do this for you. * - * The driver's detach() function may not block. The port that + * The driver's detach() function may block. The port that * detach() is given will be valid for the duration of the * callback, but if the driver wants to take a copy of the * pointer it must call parport_get_port() to do so. @@ -189,8 +154,6 @@ static void get_lowlevel_driver (void) int parport_register_driver (struct parport_driver *drv) { struct parport *port; - struct parport **ports; - int count = 0, i; if (!portlist) get_lowlevel_driver (); @@ -203,31 +166,12 @@ int parport_register_driver (struct parp * it. But we need to hold a spinlock to iterate over the * list of ports.. */ - spin_lock (&parportlist_lock); + down(®istration_lock); for (port = portlist; port; port = port->next) - count++; - spin_unlock (&parportlist_lock); - - ports = kmalloc (sizeof (struct parport *) * count, GFP_KERNEL); - if (!ports) - printk (KERN_WARNING "parport: not enough memory to attach\n"); - else { - spin_lock (&parportlist_lock); - for (i = 0, port = portlist; port && i < count; - port = port->next) - ports[i++] = port; - spin_unlock (&parportlist_lock); - - for (count = 0; count < i; count++) - drv->attach (ports[count]); - - kfree (ports); - } - - spin_lock (&driverlist_lock); + drv->attach(port); drv->next = driver_chain; driver_chain = drv; - spin_unlock (&driverlist_lock); + up(®istration_lock); return 0; } @@ -245,44 +189,38 @@ int parport_register_driver (struct parp * be called, and for each port that attach() was called for, the * detach() routine will have been called. * - * If the caller's attach() function can block, it is their - * responsibility to make sure to wait for it to exit before - * unloading. - * - * All the driver's detach() calls are guaranteed to have + * All the driver's attach() and detach() calls are guaranteed to have * finished by the time this function returns. - * - * The driver's detach() call is not allowed to block. **/ void parport_unregister_driver (struct parport_driver *arg) { - struct parport_driver *drv = driver_chain, *olddrv = NULL; + struct parport_driver *drv, *olddrv = NULL; + down(®istration_lock); + drv = driver_chain; while (drv) { if (drv == arg) { struct parport *port; - spin_lock (&driverlist_lock); if (olddrv) olddrv->next = drv->next; else driver_chain = drv->next; - spin_unlock (&driverlist_lock); /* Call the driver's detach routine for each * port to clean up any resources that the * attach routine acquired. */ - spin_lock (&parportlist_lock); for (port = portlist; port; port = port->next) drv->detach (port); - spin_unlock (&parportlist_lock); + up(®istration_lock); return; } olddrv = drv; drv = drv->next; } + up(®istration_lock); } static void free_port (struct parport *port) @@ -476,22 +414,6 @@ struct parport *parport_register_port(un void parport_announce_port (struct parport *port) { - /* We are locked against anyone else performing alterations, but - * because of parport_enumerate people can still _read_ the list - * while we are changing it; so be careful.. - * - * It's okay to have portlist_tail a little bit out of sync - * since it's only used for changing the list, not for reading - * from it. - */ - - spin_lock_irq(&parportlist_lock); - if (portlist_tail) - portlist_tail->next = port; - portlist_tail = port; - if (!portlist) - portlist = port; - spin_unlock_irq(&parportlist_lock); #ifdef CONFIG_PARPORT_1284 /* Analyse the IEEE1284.3 topology of the port. */ if (parport_daisy_init (port) == 0) { @@ -509,8 +431,27 @@ void parport_announce_port (struct parpo } #endif + down(®istration_lock); + /* We are locked against anyone else performing alterations, but + * because of parport_enumerate people can still _read_ the list + * while we are changing it; so be careful.. + * + * It's okay to have portlist_tail a little bit out of sync + * since it's only used for changing the list, not for reading + * from it. + */ + + spin_lock_irq(&parportlist_lock); + if (portlist_tail) + portlist_tail->next = port; + portlist_tail = port; + if (!portlist) + portlist = port; + spin_unlock_irq(&parportlist_lock); + /* Let drivers know that a new port has arrived. */ attach_driver_chain (port); + up(®istration_lock); } /** @@ -536,6 +477,7 @@ void parport_unregister_port(struct parp { struct parport *p; + down(®istration_lock); port->ops = &dead_ops; /* Spread the word. */ @@ -565,6 +507,7 @@ void parport_unregister_port(struct parp "%s not found in port list!\n", port->name); } spin_unlock(&parportlist_lock); + up(®istration_lock); /* Yes, parport_enumerate _is_ unsafe. Don't use it. */ parport_put_port (port); _