irda update 3/6:

o [FEATURE] New hashbin locking scheme for irqueue
o [FEATURE] Get rid of old broken hashbin locking schemes
o [FEATURE] Lock hasbins while enumerating it in various places
o [CORRECT] Remove all remaining "save_flags(flags);cli();"
o [CORRECT] Fix two "return with spinlock" found by Stanford checker

include/net/irda/irlmp.h

@@ -183,7 +183,6 @@ struct irlmp_cb {
 	hashbin_t *services;
 
 	hashbin_t *cachelog;	/* Current discovery log */
-	spinlock_t log_lock;	/* discovery log spinlock */
 
 	int running;
 
@@ -221,7 +220,7 @@ void irlmp_disconnect_indication(struct lsap_cb *self, LM_REASON reason,
 				 struct sk_buff *userdata);
 int  irlmp_disconnect_request(struct lsap_cb *, struct sk_buff *userdata);
 
-void irlmp_discovery_confirm(hashbin_t *discovery_log, DISCOVERY_MODE);
+void irlmp_discovery_confirm(hashbin_t *discovery_log, DISCOVERY_MODE mode);
 void irlmp_discovery_request(int nslots);
 struct irda_device_info *irlmp_get_discoveries(int *pn, __u16 mask, int nslots);
 void irlmp_do_expiry(void);
@@ -258,8 +257,6 @@ extern int sysctl_discovery;
 extern int sysctl_lap_keepalive_time;	/* in ms, default is LM_IDLE_TIMEOUT */
 extern struct irlmp_cb *irlmp;
 
-static inline hashbin_t *irlmp_get_cachelog(void) { return irlmp->cachelog; }
-
 /* Check if LAP queue is full.
  * Used by IrTTP for low control, see comments in irlap.h - Jean II */
 static inline int irlmp_lap_tx_queue_full(struct lsap_cb *self)

include/net/irda/irqueue.h

@@ -36,12 +36,12 @@
 #define NAME_SIZE      32
 
 /*
- * Hash types
+ * Hash types (some flags can be xored)
+ * See comments in irqueue.c for which one to use...
  */
-#define HB_NOLOCK      0
-#define HB_GLOBAL      1
-#define HB_LOCAL       2
-#define HB_SORTED      4
+#define HB_NOLOCK	0	/* No concurent access prevention */
+#define HB_LOCK		1	/* Prevent concurent write with global lock */
+#define HB_SORTED	4	/* Not yet supported */
 
 /*
  * Hash defines
@@ -57,11 +57,6 @@
 
 typedef void (*FREE_FUNC)(void *arg);
 
-/*
- * Hashbin
- */
-#define GET_HASHBIN(x) ( x & HASHBIN_MASK )
-
 struct irda_queue {
 	struct irda_queue *q_next;
 	struct irda_queue *q_prev;
@@ -75,8 +70,9 @@ typedef struct hashbin_t {
 	__u32      magic;
 	int        hb_type;
 	int        hb_size;
-	spinlock_t hb_mutex[HASHBIN_SIZE] IRDA_ALIGN;
-	irda_queue_t   *hb_queue[HASHBIN_SIZE] IRDA_ALIGN;
+	spinlock_t hb_spinlock;		/* HB_LOCK - Can be used by the user */
+
+	irda_queue_t* hb_queue[HASHBIN_SIZE] IRDA_ALIGN;
 
 	irda_queue_t* hb_current;
 } hashbin_t;
@@ -86,17 +82,16 @@ int      hashbin_delete(hashbin_t* hashbin, FREE_FUNC func);
 int      hashbin_clear(hashbin_t* hashbin, FREE_FUNC free_func);
 void     hashbin_insert(hashbin_t* hashbin, irda_queue_t* entry, long hashv, 
 			char* name);
-void*    hashbin_find(hashbin_t* hashbin, long hashv, char* name);
 void*    hashbin_remove(hashbin_t* hashbin, long hashv, char* name);
 void*    hashbin_remove_first(hashbin_t *hashbin);
 void*	 hashbin_remove_this( hashbin_t* hashbin, irda_queue_t* entry);
+void*    hashbin_find(hashbin_t* hashbin, long hashv, char* name);
+void*    hashbin_lock_find(hashbin_t* hashbin, long hashv, char* name);
+void*    hashbin_find_next(hashbin_t* hashbin, long hashv, char* name,
+			   void ** pnext);
 irda_queue_t *hashbin_get_first(hashbin_t *hashbin);
 irda_queue_t *hashbin_get_next(hashbin_t *hashbin);
 
-void enqueue_last(irda_queue_t **queue, irda_queue_t* element);
-void enqueue_first(irda_queue_t **queue, irda_queue_t* element);
-irda_queue_t *dequeue_first(irda_queue_t **queue);
-
 #define HASHBIN_GET_SIZE(hashbin) hashbin->hb_size
 
 #endif

net/irda/discovery.c

@@ -61,7 +61,7 @@ void irlmp_add_discovery(hashbin_t *cachelog, discovery_t *new)
 	/* Set time of first discovery if node is new (see below) */
 	new->first_timestamp = new->timestamp;
 
-	spin_lock_irqsave(&irlmp->log_lock, flags);
+	spin_lock_irqsave(&cachelog->hb_spinlock, flags);
 
 	/* 
 	 * Remove all discoveries of devices that has previously been 
@@ -95,13 +95,13 @@ void irlmp_add_discovery(hashbin_t *cachelog, discovery_t *new)
 	/* Insert the new and updated version */
 	hashbin_insert(cachelog, (irda_queue_t *) new, new->daddr, NULL);
 
-	spin_unlock_irqrestore(&irlmp->log_lock, flags);
+	spin_unlock_irqrestore(&cachelog->hb_spinlock, flags);
 }
 
 /*
  * Function irlmp_add_discovery_log (cachelog, log)
  *
- *    Merge a disovery log into the cachlog.
+ *    Merge a disovery log into the cachelog.
  *
  */
 void irlmp_add_discovery_log(hashbin_t *cachelog, hashbin_t *log)
@@ -115,11 +115,17 @@ void irlmp_add_discovery_log(hashbin_t *cachelog, hashbin_t *log)
 	 *  discovery, so restart discovery again with just the half timeout
 	 *  of the normal one.
 	 */
+	/* Well... It means that there was nobody out there - Jean II */
 	if (log == NULL) {
 		/* irlmp_start_discovery_timer(irlmp, 150); */
 		return;
 	}
 
+	/*
+	 * Locking : we are the only owner of this discovery log, so
+	 * no need to lock it.
+	 * We just need to lock the global log in irlmp_add_discovery().
+	 */
 	discovery = (discovery_t *) hashbin_remove_first(log);
 	while (discovery != NULL) {
 		irlmp_add_discovery(cachelog, discovery);
@@ -146,7 +152,7 @@ void irlmp_expire_discoveries(hashbin_t *log, __u32 saddr, int force)
 
 	IRDA_DEBUG(4, __FUNCTION__ "()\n");
 
-	spin_lock_irqsave(&irlmp->log_lock, flags);
+	spin_lock_irqsave(&log->hb_spinlock, flags);
 
 	discovery = (discovery_t *) hashbin_get_first(log);
 	while (discovery != NULL) {
@@ -169,7 +175,7 @@ void irlmp_expire_discoveries(hashbin_t *log, __u32 saddr, int force)
 		}
 	}
 
-	spin_unlock_irqrestore(&irlmp->log_lock, flags);
+	spin_unlock_irqrestore(&log->hb_spinlock, flags);
 }
 
 /*
@@ -230,13 +236,13 @@ struct irda_device_info *irlmp_copy_discoveries(hashbin_t *log, int *pn, __u16 m
 		return NULL;
 
 	/* Save spin lock - spinlock should be discovery specific */
-	spin_lock_irqsave(&irlmp->log_lock, flags);
+	spin_lock_irqsave(&log->hb_spinlock, flags);
 
 	/* Create the client specific buffer */
 	n = HASHBIN_GET_SIZE(log);
 	buffer = kmalloc(n * sizeof(struct irda_device_info), GFP_ATOMIC);
 	if (buffer == NULL) {
-		spin_unlock_irqrestore(&irlmp->log_lock, flags);
+		spin_unlock_irqrestore(&log->hb_spinlock, flags);
 		return NULL;
 	}
 
@@ -257,7 +263,7 @@ struct irda_device_info *irlmp_copy_discoveries(hashbin_t *log, int *pn, __u16 m
 		discovery = (discovery_t *) hashbin_get_next(log);
 	}
 
-	spin_unlock_irqrestore(&irlmp->log_lock, flags);
+	spin_unlock_irqrestore(&log->hb_spinlock, flags);
 
 	/* Get the actual number of device in the buffer and return */
 	*pn = i;
@@ -276,7 +282,7 @@ __u32 irlmp_find_device(hashbin_t *cachelog, char *name, __u32 *saddr)
 	unsigned long flags;
 	discovery_t *d;
 
-	spin_lock_irqsave(&irlmp->log_lock, flags);
+	spin_lock_irqsave(&cachelog->hb_spinlock, flags);
 
 	/* Look at all discoveries for that link */
 	d = (discovery_t *) hashbin_get_first(cachelog);
@@ -288,13 +294,13 @@ __u32 irlmp_find_device(hashbin_t *cachelog, char *name, __u32 *saddr)
 		if (strcmp(name, d->nickname) == 0) {
 			*saddr = d->saddr;
 			
-			spin_unlock_irqrestore(&irlmp->log_lock, flags);
+			spin_unlock_irqrestore(&cachelog->hb_spinlock, flags);
 			return d->daddr;
 		}
 		d = (discovery_t *) hashbin_get_next(cachelog);
 	}
 
-	spin_unlock_irqrestore(&irlmp->log_lock, flags);
+	spin_unlock_irqrestore(&cachelog->hb_spinlock, flags);
 
 	return 0;
 }
@@ -310,7 +316,7 @@ int discovery_proc_read(char *buf, char **start, off_t offset, int length,
 {
 	discovery_t *discovery;
 	unsigned long flags;
-	hashbin_t *cachelog = irlmp_get_cachelog();
+	hashbin_t *cachelog = irlmp->cachelog;
 	int		len = 0;
 
 	if (!irlmp)
@@ -318,7 +324,7 @@ int discovery_proc_read(char *buf, char **start, off_t offset, int length,
 
 	len = sprintf(buf, "IrLMP: Discovery log:\n\n");	
 	
-	spin_lock_irqsave(&irlmp->log_lock, flags);
+	spin_lock_irqsave(&cachelog->hb_spinlock, flags);
 
 	discovery = (discovery_t *) hashbin_get_first(cachelog);
 	while (( discovery != NULL) && (len < length)) {
@@ -362,7 +368,7 @@ int discovery_proc_read(char *buf, char **start, off_t offset, int length,
 		
 		discovery = (discovery_t *) hashbin_get_next(cachelog);
 	}
-	spin_unlock_irqrestore(&irlmp->log_lock, flags);
+	spin_unlock_irqrestore(&cachelog->hb_spinlock, flags);
 
 	return len;
 }

net/irda/irda_device.c

@@ -91,13 +91,13 @@ int irda_device_proc_read(char *buf, char **start, off_t offset, int len,
 
 int __init irda_device_init( void)
 {
-	dongles = hashbin_new(HB_GLOBAL);
+	dongles = hashbin_new(HB_LOCK);
 	if (dongles == NULL) {
 		printk(KERN_WARNING "IrDA: Can't allocate dongles hashbin!\n");
 		return -ENOMEM;
 	}
 
-	tasks = hashbin_new(HB_GLOBAL);
+	tasks = hashbin_new(HB_LOCK);
 	if (tasks == NULL) {
 		printk(KERN_WARNING "IrDA: Can't allocate tasks hashbin!\n");
 		return -ENOMEM;
@@ -438,7 +438,7 @@ dongle_t *irda_device_dongle_init(struct net_device *dev, int type)
 	}
 #endif
 
-	if (!(reg = hashbin_find(dongles, type, NULL))) {
+	if (!(reg = hashbin_lock_find(dongles, type, NULL))) {
 		ERROR("IrDA: Unable to find requested dongle\n");
 		return NULL;
 	}
@@ -477,7 +477,7 @@ int irda_device_dongle_cleanup(dongle_t *dongle)
 int irda_device_register_dongle(struct dongle_reg *new)
 {
 	/* Check if this dongle has been registred before */
-	if (hashbin_find(dongles, new->type, NULL)) {
+	if (hashbin_lock_find(dongles, new->type, NULL)) {
 		MESSAGE("%s: Dongle already registered\n", __FUNCTION__);
                 return 0;
         }

net/irda/iriap.c

@@ -91,11 +91,12 @@ int __init iriap_init(void)
 	__u16 hints;
 
 	/* Allocate master array */
-	iriap = hashbin_new(HB_LOCAL);
+	iriap = hashbin_new(HB_LOCK);
 	if (!iriap)
 		return -ENOMEM;
 
-	objects = hashbin_new(HB_LOCAL);
+	/* Object repository - defined in irias_object.c */
+	objects = hashbin_new(HB_LOCK);
 	if (!objects) {
 		WARNING("%s: Can't allocate objects hashbin!\n", __FUNCTION__);
 		return -ENOMEM;
@@ -973,13 +974,12 @@ int irias_proc_read(char *buf, char **start, off_t offset, int len)
 
 	ASSERT( objects != NULL, return 0;);
 
-	save_flags( flags);
-	cli();
-
 	len = 0;
 
 	len += sprintf(buf+len, "LM-IAS Objects:\n");
 
+	spin_lock_irqsave(&objects->hb_spinlock, flags);
+
 	/* List all objects */
 	obj = (struct ias_object *) hashbin_get_first(objects);
 	while ( obj != NULL) {
@@ -989,6 +989,11 @@ int irias_proc_read(char *buf, char **start, off_t offset, int len)
 		len += sprintf(buf+len, "id=%d", obj->id);
 		len += sprintf(buf+len, "\n");
 
+		/* Careful for priority inversions here !
+		 * All other uses of attrib spinlock are independant of
+		 * the object spinlock, so we are safe. Jean II */
+		spin_lock(&obj->attribs->hb_spinlock);
+
 		/* List all attributes for this object */
 		attrib = (struct ias_attrib *)
 			hashbin_get_first(obj->attribs);
@@ -1025,9 +1030,11 @@ int irias_proc_read(char *buf, char **start, off_t offset, int len)
 			attrib = (struct ias_attrib *)
 				hashbin_get_next(obj->attribs);
 		}
+		spin_unlock(&obj->attribs->hb_spinlock);
+
 	        obj = (struct ias_object *) hashbin_get_next(objects);
 	}
-	restore_flags(flags);
+	spin_unlock_irqrestore(&objects->hb_spinlock, flags);
 
 	return len;
 }

net/irda/irias_object.c

@@ -93,7 +93,10 @@ struct ias_object *irias_new_object( char *name, int id)
 	obj->name = strndup(name, IAS_MAX_CLASSNAME);
 	obj->id = id;
 
-	obj->attribs = hashbin_new(HB_LOCAL);
+	/* Locking notes : the attrib spinlock has lower precendence
+	 * than the objects spinlock. Never grap the objects spinlock
+	 * while holding any attrib spinlock (risk of deadlock). Jean II */
+	obj->attribs = hashbin_new(HB_LOCK);
 
 	return obj;
 }
@@ -211,7 +214,8 @@ struct ias_object *irias_find_object(char *name)
 {
 	ASSERT(name != NULL, return NULL;);
 
-	return hashbin_find(objects, 0, name);
+	/* Unsafe (locking), object might change */
+	return hashbin_lock_find(objects, 0, name);
 }
 
 /*
@@ -228,10 +232,11 @@ struct ias_attrib *irias_find_attrib(struct ias_object *obj, char *name)
 	ASSERT(obj->magic == IAS_OBJECT_MAGIC, return NULL;);
 	ASSERT(name != NULL, return NULL;);
 
-	attrib = hashbin_find(obj->attribs, 0, name);
+	attrib = hashbin_lock_find(obj->attribs, 0, name);
 	if (attrib == NULL)
 		return NULL;
 
+	/* Unsafe (locking), attrib might change */
 	return attrib;
 }
 
@@ -267,26 +272,32 @@ int irias_object_change_attribute(char *obj_name, char *attrib_name,
 {
 	struct ias_object *obj;
 	struct ias_attrib *attrib;
+	unsigned long flags;
 
 	/* Find object */
-	obj = hashbin_find(objects, 0, obj_name);
+	obj = hashbin_lock_find(objects, 0, obj_name);
 	if (obj == NULL) {
 		WARNING("%s: Unable to find object: %s\n", __FUNCTION__,
 			obj_name);
 		return -1;
 	}
 
+	/* Slightly unsafe (obj might get removed under us) */
+	spin_lock_irqsave(&obj->attribs->hb_spinlock, flags);
+
 	/* Find attribute */
 	attrib = hashbin_find(obj->attribs, 0, attrib_name);
 	if (attrib == NULL) {
 		WARNING("%s: Unable to find attribute: %s\n", __FUNCTION__,
 			attrib_name);
+		spin_unlock_irqrestore(&obj->attribs->hb_spinlock, flags);
 		return -1;
 	}
 
 	if ( attrib->value->type != new_value->type) {
 		IRDA_DEBUG( 0, __FUNCTION__
 		       "(), changing value type not allowed!\n");
+		spin_unlock_irqrestore(&obj->attribs->hb_spinlock, flags);
 		return -1;
 	}
 
@@ -297,6 +308,7 @@ int irias_object_change_attribute(char *obj_name, char *attrib_name,
 	attrib->value = new_value;
 
 	/* Success */
+	spin_unlock_irqrestore(&obj->attribs->hb_spinlock, flags);
 	return 0;
 }
 

net/irda/irlap.c

@@ -80,7 +80,7 @@ int irlap_proc_read(char *, char **, off_t, int);
 int __init irlap_init(void)
 {
 	/* Allocate master array */
-	irlap = hashbin_new(HB_LOCAL);
+	irlap = hashbin_new(HB_LOCK);
 	if (irlap == NULL) {
 	        ERROR("%s: can't allocate irlap hashbin!\n", __FUNCTION__);
 		return -ENOMEM;
@@ -146,7 +146,7 @@ struct irlap_cb *irlap_open(struct net_device *dev, struct qos_info *qos,
 	do {
 		get_random_bytes(&self->saddr, sizeof(self->saddr));
 	} while ((self->saddr == 0x0) || (self->saddr == BROADCAST) ||
-		 (hashbin_find(irlap, self->saddr, NULL)) );
+		 (hashbin_lock_find(irlap, self->saddr, NULL)) );
 	/* Copy to the driver */
 	memcpy(dev->dev_addr, &self->saddr, 4);
 
@@ -530,7 +530,8 @@ void irlap_discovery_request(struct irlap_cb *self, discovery_t *discovery)
 		self->discovery_log = NULL;
 	}
 
-	self->discovery_log= hashbin_new(HB_LOCAL);
+	/* All operations will occur at predictable time, no need to lock */
+	self->discovery_log= hashbin_new(HB_NOLOCK);
 
 	info.S = discovery->nslots; /* Number of slots */
 	info.s = 0; /* Current slot */
@@ -1092,15 +1093,14 @@ int irlap_proc_read(char *buf, char **start, off_t offset, int len)
 	unsigned long flags;
 	int i = 0;
 
-	save_flags(flags);
-	cli();
+	spin_lock_irqsave(&irlap->hb_spinlock, flags);
 
 	len = 0;
 
 	self = (struct irlap_cb *) hashbin_get_first(irlap);
 	while (self != NULL) {
-		ASSERT(self != NULL, return -ENODEV;);
-		ASSERT(self->magic == LAP_MAGIC, return -EBADR;);
+		ASSERT(self != NULL, break;);
+		ASSERT(self->magic == LAP_MAGIC, break;);
 
 		len += sprintf(buf+len, "irlap%d ", i++);
 		len += sprintf(buf+len, "state: %s\n",
@@ -1172,7 +1172,7 @@ int irlap_proc_read(char *buf, char **start, off_t offset, int len)
 
 		self = (struct irlap_cb *) hashbin_get_next(irlap);
 	}
-	restore_flags(flags);
+	spin_unlock_irqrestore(&irlap->hb_spinlock, flags);
 
 	return len;
 }

net/irda/irlmp_event.c

@@ -207,6 +207,43 @@ void irlmp_idle_timer_expired(void *data)
 	irlmp_do_lap_event(self, LM_LAP_IDLE_TIMEOUT, NULL);
 }
 
+/*
+ * Send an event on all LSAPs attached to this LAP.
+ */
+static inline void
+irlmp_do_all_lsap_event(hashbin_t *	lsap_hashbin,
+			IRLMP_EVENT	event)
+{
+	struct lsap_cb *lsap;
+	struct lsap_cb *lsap_next;
+
+	/* Note : this function use the new hashbin_find_next()
+	 * function, instead of the old hashbin_get_next().
+	 * This make sure that we are always pointing one lsap
+	 * ahead, so that if the current lsap is removed as the
+	 * result of sending the event, we don't care.
+	 * Also, as we store the context ourselves, if an enumeration
+	 * of the same lsap hashbin happens as the result of sending the
+	 * event, we don't care.
+	 * The only problem is if the next lsap is removed. In that case,
+	 * hashbin_find_next() will return NULL and we will abort the
+	 * enumeration. - Jean II */
+
+	/* Also : we don't accept any skb in input. We can *NOT* pass
+	 * the same skb to multiple clients safely, we would need to
+	 * skb_clone() it. - Jean II */
+
+	lsap = (struct lsap_cb *) hashbin_get_first(lsap_hashbin);
+
+	while (NULL != hashbin_find_next(lsap_hashbin,
+					 (long) lsap,
+					 NULL,
+					 (void *) &lsap_next) ) {
+		irlmp_do_lsap_event(lsap, event, NULL);
+		lsap = lsap_next;
+	}
+}
+
 /*********************************************************************
  *
  *    LAP connection control states
@@ -274,9 +311,6 @@ static void irlmp_state_standby(struct lap_cb *self, IRLMP_EVENT event,
 static void irlmp_state_u_connect(struct lap_cb *self, IRLMP_EVENT event,
 				  struct sk_buff *skb)
 {
-	struct lsap_cb *lsap;
-	struct lsap_cb *lsap_current;
-
 	IRDA_DEBUG(2, __FUNCTION__ "(), event=%s\n", irlmp_event[event]);
 
 	switch (event) {
@@ -290,11 +324,9 @@ static void irlmp_state_u_connect(struct lap_cb *self, IRLMP_EVENT event,
 		/* Just accept connection TODO, this should be fixed */
 		irlap_connect_response(self->irlap, skb);
 
-		lsap = (struct lsap_cb *) hashbin_get_first(self->lsaps);
-		while (lsap != NULL) {
-			irlmp_do_lsap_event(lsap, LM_LAP_CONNECT_CONFIRM, NULL);
-			lsap = (struct lsap_cb*) hashbin_get_next(self->lsaps);
-		}
+		/* Tell LSAPs that they can start sending data */
+		irlmp_do_all_lsap_event(self->lsaps, LM_LAP_CONNECT_CONFIRM);
+
 		/* Note : by the time we get there (LAP retries and co),
 		 * the lsaps may already have gone. This avoid getting stuck
 		 * forever in LAP_ACTIVE state - Jean II */
@@ -310,11 +342,9 @@ static void irlmp_state_u_connect(struct lap_cb *self, IRLMP_EVENT event,
 		/* For all lsap_ce E Associated do LS_Connect_confirm */
 		irlmp_next_lap_state(self, LAP_ACTIVE);
 
-		lsap = (struct lsap_cb *) hashbin_get_first(self->lsaps);
-		while (lsap != NULL) {
-			irlmp_do_lsap_event(lsap, LM_LAP_CONNECT_CONFIRM, NULL);
-			lsap = (struct lsap_cb*) hashbin_get_next(self->lsaps);
-		}
+		/* Tell LSAPs that they can start sending data */
+		irlmp_do_all_lsap_event(self->lsaps, LM_LAP_CONNECT_CONFIRM);
+
 		/* Note : by the time we get there (LAP retries and co),
 		 * the lsaps may already have gone. This avoid getting stuck
 		 * forever in LAP_ACTIVE state - Jean II */
@@ -328,18 +358,8 @@ static void irlmp_state_u_connect(struct lap_cb *self, IRLMP_EVENT event,
 		irlmp_next_lap_state(self, LAP_STANDBY);
 
 		/* Send disconnect event to all LSAPs using this link */
-		lsap = (struct lsap_cb *) hashbin_get_first( self->lsaps);
-		while (lsap != NULL ) {
-			ASSERT(lsap->magic == LMP_LSAP_MAGIC, return;);
-
-			lsap_current = lsap;
-
-			/* Be sure to stay one item ahead */
-			lsap = (struct lsap_cb *) hashbin_get_next(self->lsaps);
-			irlmp_do_lsap_event(lsap_current,
-					    LM_LAP_DISCONNECT_INDICATION,
-					    NULL);
-		}
+		irlmp_do_all_lsap_event(self->lsaps,
+					LM_LAP_DISCONNECT_INDICATION);
 		break;
 	case LM_LAP_DISCONNECT_REQUEST:
 		IRDA_DEBUG(4, __FUNCTION__ "(), LM_LAP_DISCONNECT_REQUEST\n");
@@ -368,9 +388,6 @@ static void irlmp_state_u_connect(struct lap_cb *self, IRLMP_EVENT event,
 static void irlmp_state_active(struct lap_cb *self, IRLMP_EVENT event,
 			       struct sk_buff *skb)
 {
-	struct lsap_cb *lsap;
-	struct lsap_cb *lsap_current;
-
 	IRDA_DEBUG(4, __FUNCTION__ "()\n");
 
 	switch (event) {
@@ -383,22 +400,11 @@ static void irlmp_state_active(struct lap_cb *self, IRLMP_EVENT event,
 		 *  notify all LSAPs using this LAP, but that should be safe to
 		 *  do anyway.
 		 */
-		lsap = (struct lsap_cb *) hashbin_get_first(self->lsaps);
-		while (lsap != NULL) {
-			irlmp_do_lsap_event(lsap, LM_LAP_CONNECT_CONFIRM, NULL);
-			lsap = (struct lsap_cb*) hashbin_get_next(self->lsaps);
-		}
+		irlmp_do_all_lsap_event(self->lsaps, LM_LAP_CONNECT_CONFIRM);
 
 		/* Needed by connect indication */
-		lsap = (struct lsap_cb *) hashbin_get_first(irlmp->unconnected_lsaps);
-		while (lsap != NULL) {
-			lsap_current = lsap;
-
-			/* Be sure to stay one item ahead */
-			lsap = (struct lsap_cb*) hashbin_get_next(irlmp->unconnected_lsaps);
-			irlmp_do_lsap_event(lsap_current,
-					    LM_LAP_CONNECT_CONFIRM, NULL);
-		}
+		irlmp_do_all_lsap_event(irlmp->unconnected_lsaps,
+					LM_LAP_CONNECT_CONFIRM);
 		/* Keep state */
 		break;
 	case LM_LAP_DISCONNECT_REQUEST:
@@ -447,18 +453,8 @@ static void irlmp_state_active(struct lap_cb *self, IRLMP_EVENT event,
 		/*
 		 *  Inform all connected LSAP's using this link
 		 */
-		lsap = (struct lsap_cb *) hashbin_get_first(self->lsaps);
-		while (lsap != NULL ) {
-			ASSERT(lsap->magic == LMP_LSAP_MAGIC, return;);
-
-			lsap_current = lsap;
-
-			/* Be sure to stay one item ahead */
-			lsap = (struct lsap_cb *) hashbin_get_next(self->lsaps);
-			irlmp_do_lsap_event(lsap_current,
-					    LM_LAP_DISCONNECT_INDICATION,
-					    NULL);
-		}
+		irlmp_do_all_lsap_event(self->lsaps,
+					LM_LAP_DISCONNECT_INDICATION);
 
 		/* Force an expiry of the discovery log.
 		 * Now that the LAP is free, the system may attempt to

net/irda/irlmp_frame.c

@@ -210,6 +210,7 @@ void irlmp_link_unitdata_indication(struct lap_cb *self, struct sk_buff *skb)
 	__u8   dlsap_sel;   /* Destination LSAP address */
 	__u8   pid;         /* Protocol identifier */
 	__u8   *fp;
+	unsigned long flags;
 	
 	IRDA_DEBUG(4, __FUNCTION__ "()\n");
 
@@ -242,6 +243,8 @@ void irlmp_link_unitdata_indication(struct lap_cb *self, struct sk_buff *skb)
 		return;
 	}
 	
+	/* Search the connectionless LSAP */
+	spin_lock_irqsave(&irlmp->unconnected_lsaps->hb_spinlock, flags);
 	lsap = (struct lsap_cb *) hashbin_get_first(irlmp->unconnected_lsaps);
 	while (lsap != NULL) {
 		/*
@@ -255,6 +258,8 @@ void irlmp_link_unitdata_indication(struct lap_cb *self, struct sk_buff *skb)
 		}
 		lsap = (struct lsap_cb *) hashbin_get_next(irlmp->unconnected_lsaps);
 	}
+	spin_unlock_irqrestore(&irlmp->unconnected_lsaps->hb_spinlock, flags);
+
 	if (lsap)
 		irlmp_connless_data_indication(lsap, skb);
 	else {
@@ -374,6 +379,7 @@ void irlmp_link_discovery_indication(struct lap_cb *self,
 	ASSERT(self != NULL, return;);
 	ASSERT(self->magic == LMP_LAP_MAGIC, return;);
 
+	/* Add to main log, cleanup */
 	irlmp_add_discovery(irlmp->cachelog, discovery);
 	
 	/* Just handle it the same way as a discovery confirm,
@@ -396,6 +402,7 @@ void irlmp_link_discovery_confirm(struct lap_cb *self, hashbin_t *log)
 	ASSERT(self != NULL, return;);
 	ASSERT(self->magic == LMP_LAP_MAGIC, return;);
 	
+	/* Add to main log, cleanup */
 	irlmp_add_discovery_log(irlmp->cachelog, log);
 
 	/* Propagate event to various LSAPs registered for it.
@@ -411,6 +418,8 @@ void irlmp_link_discovery_confirm(struct lap_cb *self, hashbin_t *log)
 static inline void irlmp_update_cache(struct lap_cb *lap,
 				      struct lsap_cb *lsap)
 {
+	/* Prevent concurent read to get garbage */
+	lap->cache.valid = FALSE;
 	/* Update cache entry */
 	lap->cache.dlsap_sel = lsap->dlsap_sel;
 	lap->cache.slsap_sel = lsap->slsap_sel;
@@ -441,6 +450,7 @@ static struct lsap_cb *irlmp_find_lsap(struct lap_cb *self, __u8 dlsap_sel,
 				       hashbin_t *queue) 
 {
 	struct lsap_cb *lsap;
+	unsigned long flags;
 	
 	/* 
 	 *  Optimize for the common case. We assume that the last frame
@@ -455,6 +465,9 @@ static struct lsap_cb *irlmp_find_lsap(struct lap_cb *self, __u8 dlsap_sel,
 		return (self->cache.lsap);
 	}
 #endif
+
+	spin_lock_irqsave(&queue->hb_spinlock, flags);
+
 	lsap = (struct lsap_cb *) hashbin_get_first(queue);
 	while (lsap != NULL) {
 		/* 
@@ -465,29 +478,27 @@ static struct lsap_cb *irlmp_find_lsap(struct lap_cb *self, __u8 dlsap_sel,
 		 */
 		if ((status == CONNECT_CMD) && 
 		    (lsap->slsap_sel == slsap_sel) &&      
-		    (lsap->dlsap_sel == LSAP_ANY)) 
-		{
+		    (lsap->dlsap_sel == LSAP_ANY)) {
+			/* This is where the dest lsap sel is set on incomming
+			 * lsaps */
 			lsap->dlsap_sel = dlsap_sel;
-			
-#ifdef CONFIG_IRDA_CACHE_LAST_LSAP
-			irlmp_update_cache(self, lsap);
-#endif
-			return lsap;
+			break;
 		}
 		/*
 		 *  Check if source LSAP and dest LSAP selectors match.
 		 */
 		if ((lsap->slsap_sel == slsap_sel) && 
 		    (lsap->dlsap_sel == dlsap_sel)) 
-		{
+			break;
+
+		lsap = (struct lsap_cb *) hashbin_get_next(queue);
+	}
 #ifdef CONFIG_IRDA_CACHE_LAST_LSAP
+	if(lsap)
 		irlmp_update_cache(self, lsap);
 #endif
-			return lsap;
-		}
-		lsap = (struct lsap_cb *) hashbin_get_next(queue);
-	}
+	spin_unlock_irqrestore(&queue->hb_spinlock, flags);
 
-	/* Sorry not found! */
-	return NULL;
+	/* Return what we've found or NULL */
+	return lsap;
 }

net/irda/irsyms.c

@@ -132,12 +132,14 @@ EXPORT_SYMBOL(irlmp_dup);
 EXPORT_SYMBOL(lmp_reasons);
 
 /* Queue */
-EXPORT_SYMBOL(hashbin_find);
 EXPORT_SYMBOL(hashbin_new);
 EXPORT_SYMBOL(hashbin_insert);
 EXPORT_SYMBOL(hashbin_delete);
 EXPORT_SYMBOL(hashbin_remove);
 EXPORT_SYMBOL(hashbin_remove_this);
+EXPORT_SYMBOL(hashbin_find);
+EXPORT_SYMBOL(hashbin_lock_find);
+EXPORT_SYMBOL(hashbin_find_next);
 EXPORT_SYMBOL(hashbin_get_next);
 EXPORT_SYMBOL(hashbin_get_first);
 

net/irda/irttp.c

@@ -91,7 +91,7 @@ int __init irttp_init(void)
 
 	irttp->magic = TTP_MAGIC;
 
-	irttp->tsaps = hashbin_new(HB_LOCAL);
+	irttp->tsaps = hashbin_new(HB_LOCK);
 	if (!irttp->tsaps) {
 		ERROR("%s: can't allocate IrTTP hashbin!\n", __FUNCTION__);
 		return -ENOMEM;
@@ -1365,30 +1365,43 @@ int irttp_connect_response(struct tsap_cb *self, __u32 max_sdu_size,
 struct tsap_cb *irttp_dup(struct tsap_cb *orig, void *instance)
 {
 	struct tsap_cb *new;
+	unsigned long flags;
 
 	IRDA_DEBUG(1, __FUNCTION__ "()\n");
 
-	if (!hashbin_find(irttp->tsaps, (long) orig, NULL)) {
+	/* Protect our access to the old tsap instance */
+	spin_lock_irqsave(&irttp->tsaps->hb_spinlock, flags);
+
+	/* Find the old instance */
+	if (!hashbin_find(irttp->tsaps, (int) orig, NULL)) {
 		IRDA_DEBUG(0, __FUNCTION__ "(), unable to find TSAP\n");
+		spin_unlock_irqrestore(&irttp->tsaps->hb_spinlock, flags);
 		return NULL;
 	}
+
+	/* Allocate a new instance */
 	new = kmalloc(sizeof(struct tsap_cb), GFP_ATOMIC);
 	if (!new) {
 		IRDA_DEBUG(0, __FUNCTION__ "(), unable to kmalloc\n");
+		spin_unlock_irqrestore(&irttp->tsaps->hb_spinlock, flags);
 		return NULL;
 	}
 	/* Dup */
 	memcpy(new, orig, sizeof(struct tsap_cb));
-	new->notify.instance = instance;
-	new->lsap = irlmp_dup(orig->lsap, new);
+
+	/* We don't need the old instance any more */
+	spin_unlock_irqrestore(&irttp->tsaps->hb_spinlock, flags);
 
 	/* Not everything should be copied */
+	new->notify.instance = instance;
+	new->lsap = irlmp_dup(orig->lsap, new);
 	init_timer(&new->todo_timer);
 
 	skb_queue_head_init(&new->rx_queue);
 	skb_queue_head_init(&new->tx_queue);
 	skb_queue_head_init(&new->rx_fragments);
 
+	/* This is locked */
 	hashbin_insert(irttp->tsaps, (irda_queue_t *) new, (long) new, NULL);
 
 	return new;
@@ -1723,8 +1736,8 @@ int irttp_proc_read(char *buf, char **start, off_t offset, int len)
 
 	len = 0;
 
-	save_flags(flags);
-	cli();
+	/* Protect our access to the tsap list */
+	spin_lock_irqsave(&irttp->tsaps->hb_spinlock, flags);
 
 	self = (struct tsap_cb *) hashbin_get_first(irttp->tsaps);
 	while (self != NULL) {
@@ -1770,7 +1783,7 @@ int irttp_proc_read(char *buf, char **start, off_t offset, int len)
 
 		self = (struct tsap_cb *) hashbin_get_next(irttp->tsaps);
 	}
-	restore_flags(flags);
+	spin_unlock_irqrestore(&irttp->tsaps->hb_spinlock, flags);
 
 	return len;
 }