From: viro@parcelfarce.linux.theplanet.co.uk

Current tree has all allocated ports on portlist.  However,
most of the portlist users assume that we only have announced ports
there and break badly if they happen to see the list after port driver
has created a port (parport_register_port()) but before it finishes
the setup (and calls parport_announce_port()).

The only place that wants to see all allocated port is
parport_register_port() itself and only to pick the first unused
port number.

We add a new list (all_ports) and put ports there when allocated;
that list is kept ordered by port->number.  Ports are placed on portlist
only by parport_announce_port().

Gobs of shite in parport_register_port() removed, pile of races
closed...

We use a new mutex to protect all additions/removals of drivers and
ports.  That cures a lot of insanity:

* driver removals can't hit us in the middle of attach_driver_chain().  Old
  code simply dies on that.

* port removals can't hit us in the middle of driver registration.  Again,
  old code dies on that.

* driver ->detach() is allowed to block now.

* we are guaranteed that by the time when parport_unregister_driver()
  returns, all ->detach() calls are finished.  Old code did _not_ guarantee
  that (read: was inherently racy since rmmod of driver could race with port
  removal and get driver->detach(port) called after the module was gone).

* we are guaranteed that driver->attach(port) won't be called more than
  once.  With the old code that was a matter of luck.

* removed piles and piles of braindead code.

A bunch of parport_enumerate() users was duplicating parport_find_...()
without proper locking.  Replaced with use of appropriate helpers, races
closed.

parport_gsc.c turned into proper parisc driver; instead of scanning the list
of ports upon rmmod in search of ones that had been created by us, we do
cleanup where it belongs - in parisc driver ->remove().

bw-qcam.c turned into proper parport driver.  Instead of (racy) scanning the
list of ports we use ->attach() and ->detach().

daisy.c used to access the topology list with no locking whatsoever. 
Protected by spinlock, fixed numerous bugs in handling of the single-linked
list (I'm not kidding - just take a look at the old parport_daisy_fini() and
weep; it is supposed to go through a simple list and remove all entries that
satisfy a condition).


---

 25-akpm/drivers/parport/share.c |   75 ++++++++++++++++++----------------------
 25-akpm/include/linux/parport.h |    2 +
 2 files changed, 37 insertions(+), 40 deletions(-)

diff -puN drivers/parport/share.c~PP0-full_list-RC1 drivers/parport/share.c
--- 25/drivers/parport/share.c~PP0-full_list-RC1	Wed Jan 14 13:18:03 2004
+++ 25-akpm/drivers/parport/share.c	Wed Jan 14 13:50:44 2004
@@ -44,6 +44,10 @@ int parport_default_spintime =  DEFAULT_
 static struct parport *portlist = NULL, *portlist_tail = NULL;
 static spinlock_t parportlist_lock = SPIN_LOCK_UNLOCKED;
 
+/* list of all allocated ports, sorted by ->number */
+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;
 
@@ -284,6 +288,9 @@ void parport_unregister_driver (struct p
 static void free_port (struct parport *port)
 {
 	int d;
+	spin_lock(&full_list_lock);
+	list_del(&port->full_list);
+	spin_unlock(&full_list_lock);
 	for (d = 0; d < 5; d++) {
 		if (port->probe_info[d].class_name)
 			kfree (port->probe_info[d].class_name);
@@ -386,8 +393,9 @@ struct parport *parport_enumerate(void)
 struct parport *parport_register_port(unsigned long base, int irq, int dma,
 				      struct parport_operations *ops)
 {
+	struct list_head *l;
 	struct parport *tmp;
-	int portnum;
+	int num;
 	int device;
 	char *name;
 
@@ -418,6 +426,7 @@ struct parport *parport_register_port(un
 	init_MUTEX_LOCKED (&tmp->ieee1284.irq); /* actually a semaphore at 0 */
 	tmp->spintime = parport_default_spintime;
 	atomic_set (&tmp->ref_count, 1);
+	INIT_LIST_HEAD(&tmp->full_list);
 
 	name = kmalloc(15, GFP_KERNEL);
 	if (!name) {
@@ -427,53 +436,22 @@ struct parport *parport_register_port(un
 	}
 	/* Search for the lowest free parport number. */
 
-	spin_lock_irq (&parportlist_lock);
-	for (portnum = 0; ; portnum++) {
-		struct parport *itr = portlist;
-		while (itr) {
-			if (itr->number == portnum)
-				/* No good, already used. */
-				break;
-			else
-				itr = itr->next;
-		}
-
-		if (itr == NULL)
-			/* Got to the end of the list. */
+	spin_lock(&full_list_lock);
+	for (l = all_ports.next, num = 0; l != &all_ports; l = l->next, num++) {
+		struct parport *p = list_entry(l, struct parport, full_list);
+		if (p->number != num)
 			break;
 	}
+	tmp->portnum = tmp->number = num;
+	list_add_tail(&tmp->full_list, l);
+	spin_unlock(&full_list_lock);
 
 	/*
 	 * Now that the portnum is known finish doing the Init.
 	 */
-	tmp->portnum = tmp->number = portnum;
-	sprintf(name, "parport%d", portnum);
+	sprintf(name, "parport%d", tmp->portnum = tmp->number);
 	tmp->name = name;
 
-	
-	/*
-	 * Chain the entry to our list.
-	 *
-	 * This function must not run from an irq handler so we don' t need
-	 * to clear irq on the local CPU. -arca
-	 */
-
-	/* 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.
-	 */
-
-	if (portlist_tail)
-		portlist_tail->next = tmp;
-	portlist_tail = tmp;
-	if (!portlist)
-		portlist = tmp;
-	spin_unlock_irq(&parportlist_lock);
-
 	for (device = 0; device < 5; device++)
 		/* assume the worst */
 		tmp->probe_info[device].class = PARPORT_CLASS_LEGACY;
@@ -497,6 +475,23 @@ 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) {
diff -puN include/linux/parport.h~PP0-full_list-RC1 include/linux/parport.h
--- 25/include/linux/parport.h~PP0-full_list-RC1	Wed Jan 14 13:18:03 2004
+++ 25-akpm/include/linux/parport.h	Wed Jan 14 13:18:03 2004
@@ -311,6 +311,8 @@ struct parport {
 
 	int spintime;
 	atomic_t ref_count;
+
+	struct list_head full_list;
 };
 
 #define DEFAULT_SPIN_TIME 500 /* us */

_