respin of hso patches for linux-2.6.28-rc6 hso_mutex.patch [patch 4/6]

respin of hso patches for linux-2.6.28-rc6 hso_mutex.patch [patch 4/6]

[Denis Joseph Barrow]

[-- Attachment #1: Type: text/plain, Size: 7790 bytes --]

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 <D.Barow-x9gZzRpC1QbQT0dZR+AlfA@public.gmane.org>
---
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;
 

-- 
best regards,
D.J. Barrow

[-- Attachment #2: hso_mutex.patch --]
[-- Type: text/x-diff, Size: 7759 bytes --]

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 <D.Barow-x9gZzRpC1QbQT0dZR+AlfA@public.gmane.org>
---
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;
 

Re: respin of hso patches for linux-2.6.28-rc6 hso_mutex.patch [patch 4/6]

[Marcel Holtmann]

Hi Denis,

> 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 <D.Barow@option.com>
> ---
> 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;
> };

this looks pretty ugly to me. Can we not find a more elegant way to  
solve this problem?   It might involve fixing or changing the TTY  
layer, but that is actually fine.

Regards

Marcel

Re: respin of hso patches for linux-2.6.28-rc6 hso_mutex.patch [patch 4/6]

[David Miller]

From: Denis Joseph Barrow <D.Barow-x9gZzRpC1QbQT0dZR+AlfA@public.gmane.org>
Date: Mon, 24 Nov 2008 15:48:47 +0100

> 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 <D.Barow-x9gZzRpC1QbQT0dZR+AlfA@public.gmane.org>

Applied, but I had to fixup a lot of whitespace errors.

All the lines that add code had a single initial space and
then tabs.

I know where this comes from.  You applied the patch to your
tree, there were rejects, and you copied and pasted the hunks
from the foo.rej file into the source file.

In doing so you left the initial space character that sits
after each "+" in the foo.rej file.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: respin of hso patches for linux-2.6.28-rc6 hso_mutex.patch [patch 4/6]

[Alan Cox]

On Tue, 25 Nov 2008 00:34:27 -0800 (PST)
David Miller <davem@davemloft.net> wrote:

> From: Denis Joseph Barrow <D.Barow@option.com>
> Date: Mon, 24 Nov 2008 15:48:47 +0100
> 
> > 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 <D.Barow@option.com>
> 
> Applied, but I had to fixup a lot of whitespace errors.

Dave - I don't put random network layer patches in the ttydev tree,
please don't put random tty driver patches in the network tree. Those
patches

a) are woefully inadequate
b) will clash with the ttydev work which is where this belongs

Alan

Re: respin of hso patches for linux-2.6.28-rc6 hso_mutex.patch [patch 4/6]

[David Miller]

From: Alan Cox <alan@lxorguk.ukuu.org.uk>
Date: Tue, 25 Nov 2008 10:20:43 +0000

> On Tue, 25 Nov 2008 00:34:27 -0800 (PST)
> David Miller <davem@davemloft.net> wrote:
> 
> > From: Denis Joseph Barrow <D.Barow@option.com>
> > Date: Mon, 24 Nov 2008 15:48:47 +0100
> > 
> > > 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 <D.Barow@option.com>
> > 
> > Applied, but I had to fixup a lot of whitespace errors.
> 
> Dave - I don't put random network layer patches in the ttydev tree,
> please don't put random tty driver patches in the network tree. Those
> patches
> 
> a) are woefully inadequate
> b) will clash with the ttydev work which is where this belongs

I thought this was a network driver?

If so, this poor guy has respun these patches so many times
my head is spinning, and making him wait for one more procedural
thing is just a good way to deter yet another developer.

Most of these patches looked fine, is there a specific one I
should leave out?

I'm not leaving all of them out, most of them looked perfectly
fine and didn't do any "tty" stuff.

Re: respin of hso patches for linux-2.6.28-rc6 hso_mutex.patch [patch 4/6]

[Alan Cox]

> I thought this was a network driver?

It's both, but the resource handling mess is on the tty side, and several
of the chunks (modem control etc) directly hit the tty parts and will
clash with other tty layer changes.
> 
> If so, this poor guy has respun these patches so many times
> my head is spinning, and making him wait for one more procedural
> thing is just a good way to deter yet another developer.

Well if you check the list I sent him a set of patches to review in
response to his posting. Those patches implement proper tty side
refcounting and have a chance of actually fixing the problems.

> I'm not leaving all of them out, most of them looked perfectly
> fine and didn't do any "tty" stuff.

That patch series is a complete mix:

1, 3 and 6 are network
2 is a broken tty layer patch
4 is a broken tty layer patch
5 is an ok tty layer patch

5 will go into the tty tree

2 and 4 clash with the tty tree already and don't deal with the tty side
locking at all sanely.

So I've no problem with the network parts (1,3,6) going into the netdev
tree but the other bits will make a mess if they do so.

Re: respin of hso patches for linux-2.6.28-rc6 hso_mutex.patch [patch 4/6]

[David Miller]

From: Alan Cox <alan-qBU/x9rampVanCEyBjwyrvXRex20P6io@public.gmane.org>
Date: Tue, 25 Nov 2008 10:42:13 +0000

> So I've no problem with the network parts (1,3,6) going into the netdev
> tree but the other bits will make a mess if they do so.

Ok, I'll revert the other ones.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: respin of hso patches for linux-2.6.28-rc6 hso_mutex.patch [patch 4/6]

[David Miller]

From: Alan Cox <alan-qBU/x9rampVanCEyBjwyrvXRex20P6io@public.gmane.org>
Date: Tue, 25 Nov 2008 10:42:13 +0000

> That patch series is a complete mix:
> 
> 1, 3 and 6 are network
> 2 is a broken tty layer patch
> 4 is a broken tty layer patch
> 5 is an ok tty layer patch
> 
> 5 will go into the tty tree
> 
> 2 and 4 clash with the tty tree already and don't deal with the tty side
> locking at all sanely.
> 
> So I've no problem with the network parts (1,3,6) going into the netdev
> tree but the other bits will make a mess if they do so.

Actually Alan, can you do me a favor?

These patches were not numbered, and when you say patch N it's ambiguous
(especially since they arrived on the mailing list in a different
order than they arrived in my private inbox, the latter of which is
the order I applied them to my tree).

So please let me know which patches exactly are which and I'll do the
reverting.

Thanks!
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: respin of hso patches for linux-2.6.28-rc6 hso_mutex.patch [patch 4/6]

[Alan Cox]

> Actually Alan, can you do me a favor?
> 
> These patches were not numbered, and when you say patch N it's ambiguous

See the subject line including the one you replied to .. they were
numbered but not in the usual way.

Alan

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html