A new structure hso_mutex_table had to be declared statically & used as as hso_device mutex_lock(&serial->parent->mutex) etc is freed in hso_serial_open & hso_serial_close by kref_put while the mutex is still in use. This is a substantial change but should make the driver much stabler. Signed-off-by: Denis Joseph Barrow --- Index: linux-2.6.28-rc6.patches/drivers/net/usb/hso.c =================================================================== --- linux-2.6.28-rc6.patches.orig/drivers/net/usb/hso.c 2008-11-24 14:19:17.000000000 +0100 +++ linux-2.6.28-rc6.patches/drivers/net/usb/hso.c 2008-11-24 14:59:29.000000000 +0100 @@ -230,6 +230,11 @@ struct work_struct retry_unthrottle_workqueue; }; +struct hso_mutex_t { + struct mutex mutex; + u8 allocated; +}; + struct hso_device { union { struct hso_serial *dev_serial; @@ -248,7 +253,7 @@ struct device *dev; struct kref ref; - struct mutex mutex; + struct hso_mutex_t *mutex; }; /* Type of interface */ @@ -364,6 +369,13 @@ static spinlock_t serial_table_lock; static struct ktermios *hso_serial_termios[HSO_SERIAL_TTY_MINORS]; static struct ktermios *hso_serial_termios_locked[HSO_SERIAL_TTY_MINORS]; +/* hso_mutex_table has to be declared statically as hso_device + * is freed in hso_serial_open & hso_serial_close while + * the mutex is still in use. + */ +#define HSO_NUM_MUTEXES (HSO_SERIAL_TTY_MINORS+HSO_MAX_NET_DEVICES) +static struct hso_mutex_t hso_mutex_table[HSO_NUM_MUTEXES]; +static spinlock_t hso_mutex_lock; static const s32 default_port_spec[] = { HSO_INTF_MUX | HSO_PORT_NETWORK, @@ -604,6 +616,34 @@ spin_unlock_irqrestore(&serial_table_lock, flags); } + +static struct hso_mutex_t *hso_get_free_mutex(void) +{ + int index; + struct hso_mutex_t *curr_hso_mutex; + + spin_lock(&hso_mutex_lock); + for (index = 0; index < HSO_NUM_MUTEXES; index++) { + curr_hso_mutex = &hso_mutex_table[index]; + if (!curr_hso_mutex->allocated) { + curr_hso_mutex->allocated = 1; + spin_unlock(&hso_mutex_lock); + return curr_hso_mutex; + } + } + printk(KERN_ERR "BUG %s: no free hso_mutexs devices in table\n" + , __func__); + spin_unlock(&hso_mutex_lock); + return NULL; +} + +static void hso_free_mutex(struct hso_mutex_t *mutex) +{ + spin_lock(&hso_mutex_lock); + mutex->allocated = 0; + spin_unlock(&hso_mutex_lock); +} + /* log a meaningful explanation of an USB status */ static void log_usb_status(int status, const char *function) { @@ -1225,7 +1265,9 @@ static int hso_serial_open(struct tty_struct *tty, struct file *filp) { struct hso_serial *serial = get_serial_by_index(tty->index); - int result; + int result1 = 0, result2 = 0; + struct mutex *hso_mutex = NULL; + int refcnt = 1; /* sanity check */ if (serial == NULL || serial->magic != HSO_SERIAL_MAGIC) { @@ -1234,15 +1276,16 @@ return -ENODEV; } - mutex_lock(&serial->parent->mutex); + hso_mutex = &serial->parent->mutex->mutex; + mutex_lock(hso_mutex); /* check for port already opened, if not set the termios */ /* The serial->open count needs to be here as hso_serial_close * will be called even if hso_serial_open returns -ENODEV. */ serial->open_count++; - result = usb_autopm_get_interface(serial->parent->interface); - if (result < 0) - goto err_out; + result1 = usb_autopm_get_interface(serial->parent->interface); + if (result1 < 0) + goto err_out; D1("Opening %d", serial->minor); kref_get(&serial->parent->ref); @@ -1261,11 +1304,10 @@ (unsigned long)serial); INIT_WORK(&serial->retry_unthrottle_workqueue, hso_unthrottle_workfunc); - result = hso_start_serial_device(serial->parent, GFP_KERNEL); - if (result) { + result2 = hso_start_serial_device(serial->parent, GFP_KERNEL); + if (result2) { hso_stop_serial_device(serial->parent); serial->open_count--; - kref_put(&serial->parent->ref, hso_serial_ref_free); } } else { D1("Port was already open"); @@ -1274,11 +1316,16 @@ usb_autopm_put_interface(serial->parent->interface); /* done */ - if (result) + if (result1) hso_serial_tiocmset(tty, NULL, TIOCM_RTS | TIOCM_DTR, 0); err_out: - mutex_unlock(&serial->parent->mutex); - return result; + if (result2) + refcnt = kref_put(&serial->parent->ref, hso_serial_ref_free); + mutex_unlock(hso_mutex); + if (refcnt == 0) + hso_free_mutex(container_of(hso_mutex, + struct hso_mutex_t, mutex)); + return result1 == 0 ? result2 : result1; } /* close the requested serial port */ @@ -1286,6 +1333,8 @@ { struct hso_serial *serial = tty->driver_data; u8 usb_gone; + struct mutex *hso_mutex; + int refcnt; D1("Closing serial port"); if (serial == NULL || serial->magic != HSO_SERIAL_MAGIC) { @@ -1293,8 +1342,9 @@ return; } - mutex_lock(&serial->parent->mutex); usb_gone = serial->parent->usb_gone; + hso_mutex = &serial->parent->mutex->mutex; + mutex_lock(hso_mutex); if (!usb_gone) usb_autopm_get_interface(serial->parent->interface); @@ -1302,7 +1352,6 @@ /* reset the rts and dtr */ /* do the actual close */ serial->open_count--; - kref_put(&serial->parent->ref, hso_serial_ref_free); if (serial->open_count <= 0) { serial->open_count = 0; if (serial->tty) { @@ -1317,8 +1366,11 @@ if (!usb_gone) usb_autopm_put_interface(serial->parent->interface); - - mutex_unlock(&serial->parent->mutex); + refcnt = kref_put(&serial->parent->ref, hso_serial_ref_free); + mutex_unlock(hso_mutex); + if (refcnt == 0) + hso_free_mutex(container_of(hso_mutex, + struct hso_mutex_t, mutex)); } /* close the requested serial port */ @@ -2084,8 +2136,12 @@ hso_dev->usb = interface_to_usbdev(intf); hso_dev->interface = intf; kref_init(&hso_dev->ref); - mutex_init(&hso_dev->mutex); - + hso_dev->mutex = hso_get_free_mutex(); + if (!hso_dev->mutex) { + kfree(hso_dev); + return NULL; + } + mutex_init(&hso_dev->mutex->mutex); INIT_WORK(&hso_dev->async_get_intf, async_get_intf); INIT_WORK(&hso_dev->async_put_intf, async_put_intf); @@ -2131,7 +2187,7 @@ unregister_netdev(hso_net->net); free_netdev(hso_net->net); } - + hso_free_mutex(hso_dev->mutex); hso_free_device(hso_dev); } @@ -2180,14 +2236,14 @@ int enabled = (state == RFKILL_STATE_ON); int rv; - mutex_lock(&hso_dev->mutex); + mutex_lock(&hso_dev->mutex->mutex); if (hso_dev->usb_gone) rv = 0; else rv = usb_control_msg(hso_dev->usb, usb_rcvctrlpipe(hso_dev->usb, 0), enabled ? 0x82 : 0x81, 0x40, 0, 0, NULL, 0, USB_CTRL_SET_TIMEOUT); - mutex_unlock(&hso_dev->mutex); + mutex_unlock(&hso_dev->mutex->mutex); return rv; } @@ -2795,6 +2851,8 @@ { struct hso_serial *hso_dev; int i; + struct mutex *hso_mutex = NULL; + int refcnt = 1; for (i = 0; i < HSO_SERIAL_TTY_MINORS; i++) { if (serial_table[i] @@ -2802,10 +2860,12 @@ hso_dev = dev2ser(serial_table[i]); if (hso_dev->tty) tty_hangup(hso_dev->tty); - mutex_lock(&hso_dev->parent->mutex); + hso_mutex = &hso_dev->parent->mutex->mutex; + mutex_lock(hso_mutex); hso_dev->parent->usb_gone = 1; - mutex_unlock(&hso_dev->parent->mutex); - kref_put(&serial_table[i]->ref, hso_serial_ref_free); + refcnt = kref_put(&serial_table[i]->ref, + hso_serial_ref_free); + mutex_unlock(hso_mutex); } } @@ -2824,6 +2884,9 @@ hso_free_net_device(network_table[i]); } } + if (refcnt == 0) + hso_free_mutex(container_of(hso_mutex, + struct hso_mutex_t, mutex)); } /* Helper functions */ @@ -2922,6 +2985,7 @@ /* Initialise the serial table semaphore and table */ spin_lock_init(&serial_table_lock); + spin_lock_init(&hso_mutex_lock); for (i = 0; i < HSO_SERIAL_TTY_MINORS; i++) serial_table[i] = NULL;