LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH retry] bluetooth : add conn add/del workqueues to avoid connection fail
@ 2008-01-30  2:23 Dave Young
  2008-01-30  5:14 ` David Miller
  2008-01-31 13:09 ` Jens Axboe
  0 siblings, 2 replies; 8+ messages in thread
From: Dave Young @ 2008-01-30  2:23 UTC (permalink / raw)
  To: marcel; +Cc: davem, linux-kernel, bluez-devel, netdev


The bluetooth hci_conn sysfs add/del executed in the default workqueue.
If the del_conn is executed after the new add_conn with same target,
add_conn will failed with warning of "same kobject name".

Here add btaddconn & btdelconn workqueues,
flush the btdelconn workqueue in the add_conn function to avoid the issue.

Signed-off-by: Dave Young <hidave.darkstar@gmail.com> 

---
diff -upr a/net/bluetooth/hci_sysfs.c b/net/bluetooth/hci_sysfs.c
--- a/net/bluetooth/hci_sysfs.c	2008-01-30 10:14:27.000000000 +0800
+++ b/net/bluetooth/hci_sysfs.c	2008-01-30 10:14:14.000000000 +0800
@@ -12,6 +12,8 @@
 #undef  BT_DBG
 #define BT_DBG(D...)
 #endif
+static struct workqueue_struct *btaddconn;
+static struct workqueue_struct *btdelconn;
 
 static inline char *typetostr(int type)
 {
@@ -279,6 +281,7 @@ static void add_conn(struct work_struct 
 	struct hci_conn *conn = container_of(work, struct hci_conn, work);
 	int i;
 
+	flush_workqueue(btdelconn);
 	if (device_add(&conn->dev) < 0) {
 		BT_ERR("Failed to register connection device");
 		return;
@@ -313,6 +316,7 @@ void hci_conn_add_sysfs(struct hci_conn 
 
 	INIT_WORK(&conn->work, add_conn);
 
+	queue_work(btaddconn, &conn->work);
 	schedule_work(&conn->work);
 }
 
@@ -349,6 +353,7 @@ void hci_conn_del_sysfs(struct hci_conn 
 
 	INIT_WORK(&conn->work, del_conn);
 
+	queue_work(btdelconn, &conn->work);
 	schedule_work(&conn->work);
 }
 
@@ -398,31 +403,52 @@ int __init bt_sysfs_init(void)
 {
 	int err;
 
+	btaddconn = create_singlethread_workqueue("btaddconn");
+	if (!btaddconn) {
+		err = -ENOMEM;
+		goto out;
+	}
+	btdelconn = create_singlethread_workqueue("btdelconn");
+	if (!btdelconn) {
+		err = -ENOMEM;
+		goto out_del;
+	}
+
 	bt_platform = platform_device_register_simple("bluetooth", -1, NULL, 0);
-	if (IS_ERR(bt_platform))
-		return PTR_ERR(bt_platform);
+	if (IS_ERR(bt_platform)) {
+		err = PTR_ERR(bt_platform);
+		goto out_platform;
+	}
 
 	err = bus_register(&bt_bus);
-	if (err < 0) {
-		platform_device_unregister(bt_platform);
-		return err;
-	}
+	if (err < 0)
+		goto out_bus;
 
 	bt_class = class_create(THIS_MODULE, "bluetooth");
 	if (IS_ERR(bt_class)) {
-		bus_unregister(&bt_bus);
-		platform_device_unregister(bt_platform);
-		return PTR_ERR(bt_class);
+		err = PTR_ERR(bt_class);
+		goto out_class;
 	}
 
 	return 0;
+
+out_class:
+	bus_unregister(&bt_bus);
+out_bus:
+	platform_device_unregister(bt_platform);
+out_platform:
+	destroy_workqueue(btdelconn);
+out_del:
+	destroy_workqueue(btaddconn);
+out:
+	return err;
 }
 
 void bt_sysfs_cleanup(void)
 {
+	destroy_workqueue(btaddconn);
+	destroy_workqueue(btdelconn);
 	class_destroy(bt_class);
-
 	bus_unregister(&bt_bus);
-
 	platform_device_unregister(bt_platform);
 }

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH retry] bluetooth : add conn add/del workqueues to avoid connection fail
  2008-01-30  2:23 [PATCH retry] bluetooth : add conn add/del workqueues to avoid connection fail Dave Young
@ 2008-01-30  5:14 ` David Miller
  2008-01-30 10:21   ` Marcel Holtmann
  2008-01-31 13:09 ` Jens Axboe
  1 sibling, 1 reply; 8+ messages in thread
From: David Miller @ 2008-01-30  5:14 UTC (permalink / raw)
  To: hidave.darkstar; +Cc: marcel, linux-kernel, bluez-devel, netdev

From: Dave Young <hidave.darkstar@gmail.com>
Date: Wed, 30 Jan 2008 10:23:54 +0800

> 
> The bluetooth hci_conn sysfs add/del executed in the default workqueue.
> If the del_conn is executed after the new add_conn with same target,
> add_conn will failed with warning of "same kobject name".
> 
> Here add btaddconn & btdelconn workqueues,
> flush the btdelconn workqueue in the add_conn function to avoid the issue.
> 
> Signed-off-by: Dave Young <hidave.darkstar@gmail.com> 

This looks good, applied, thanks Dave.

I've queued this up for 2.6.25 merging, if you want me to
schedule it for -stable, just let me know.

Thanks again.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH retry] bluetooth : add conn add/del workqueues to avoid connection fail
  2008-01-30  5:14 ` David Miller
@ 2008-01-30 10:21   ` Marcel Holtmann
  2008-01-31  1:29     ` Dave Young
  0 siblings, 1 reply; 8+ messages in thread
From: Marcel Holtmann @ 2008-01-30 10:21 UTC (permalink / raw)
  To: David Miller; +Cc: hidave.darkstar, linux-kernel, bluez-devel, netdev

Hi Dave,

> > The bluetooth hci_conn sysfs add/del executed in the default workqueue.
> > If the del_conn is executed after the new add_conn with same target,
> > add_conn will failed with warning of "same kobject name".
> > 
> > Here add btaddconn & btdelconn workqueues,
> > flush the btdelconn workqueue in the add_conn function to avoid the issue.
> > 
> > Signed-off-by: Dave Young <hidave.darkstar@gmail.com> 
> 
> This looks good, applied, thanks Dave.
> 
> I've queued this up for 2.6.25 merging, if you want me to
> schedule it for -stable, just let me know.

don't include it. I first have to stress test it on one of my machines.
Besides that I have to do some coding style cleanups.

Regards

Marcel



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH retry] bluetooth : add conn add/del workqueues to avoid connection fail
  2008-01-30 10:21   ` Marcel Holtmann
@ 2008-01-31  1:29     ` Dave Young
  0 siblings, 0 replies; 8+ messages in thread
From: Dave Young @ 2008-01-31  1:29 UTC (permalink / raw)
  To: Marcel Holtmann; +Cc: David Miller, linux-kernel, bluez-devel, netdev

On Jan 30, 2008 6:21 PM, Marcel Holtmann <marcel@holtmann.org> wrote:
> Hi Dave,
>
> > > The bluetooth hci_conn sysfs add/del executed in the default workqueue.
> > > If the del_conn is executed after the new add_conn with same target,
> > > add_conn will failed with warning of "same kobject name".
> > >
> > > Here add btaddconn & btdelconn workqueues,
> > > flush the btdelconn workqueue in the add_conn function to avoid the issue.
> > >
> > > Signed-off-by: Dave Young <hidave.darkstar@gmail.com>
> >
> > This looks good, applied, thanks Dave.
> >
> > I've queued this up for 2.6.25 merging, if you want me to
> > schedule it for -stable, just let me know.
>
> don't include it. I first have to stress test it on one of my machines.
> Besides that I have to do some coding style cleanups.

Sorry, I thought you forgot it.
Thanks.

BTW, for the bus_id bug, If there's no urgent need I think we could do
the fix after driver core bus_id changes of kay which will be there
soon.

>
> Regards
>
> Marcel
>
>
>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH retry] bluetooth : add conn add/del workqueues to avoid  connection fail
  2008-01-30  2:23 [PATCH retry] bluetooth : add conn add/del workqueues to avoid connection fail Dave Young
  2008-01-30  5:14 ` David Miller
@ 2008-01-31 13:09 ` Jens Axboe
  2008-02-01  1:24   ` Dave Young
  1 sibling, 1 reply; 8+ messages in thread
From: Jens Axboe @ 2008-01-31 13:09 UTC (permalink / raw)
  To: Dave Young; +Cc: marcel, davem, linux-kernel, bluez-devel, netdev

On Wed, Jan 30 2008, Dave Young wrote:
> 
> The bluetooth hci_conn sysfs add/del executed in the default workqueue.
> If the del_conn is executed after the new add_conn with same target,
> add_conn will failed with warning of "same kobject name".
> 
> Here add btaddconn & btdelconn workqueues,
> flush the btdelconn workqueue in the add_conn function to avoid the issue.
> 
> Signed-off-by: Dave Young <hidave.darkstar@gmail.com> 
> 
> ---
> diff -upr a/net/bluetooth/hci_sysfs.c b/net/bluetooth/hci_sysfs.c
> --- a/net/bluetooth/hci_sysfs.c	2008-01-30 10:14:27.000000000 +0800
> +++ b/net/bluetooth/hci_sysfs.c	2008-01-30 10:14:14.000000000 +0800
> @@ -12,6 +12,8 @@
>  #undef  BT_DBG
>  #define BT_DBG(D...)
>  #endif
> +static struct workqueue_struct *btaddconn;
> +static struct workqueue_struct *btdelconn;
>  
>  static inline char *typetostr(int type)
>  {
> @@ -279,6 +281,7 @@ static void add_conn(struct work_struct 
>  	struct hci_conn *conn = container_of(work, struct hci_conn, work);
>  	int i;
>  
> +	flush_workqueue(btdelconn);
>  	if (device_add(&conn->dev) < 0) {
>  		BT_ERR("Failed to register connection device");
>  		return;
> @@ -313,6 +316,7 @@ void hci_conn_add_sysfs(struct hci_conn 
>  
>  	INIT_WORK(&conn->work, add_conn);
>  
> +	queue_work(btaddconn, &conn->work);
>  	schedule_work(&conn->work);
>  }

So you queue &conn->work on both btaddconn and keventd_wq?

-- 
Jens Axboe


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH retry] bluetooth : add conn add/del workqueues to avoid connection fail
  2008-01-31 13:09 ` Jens Axboe
@ 2008-02-01  1:24   ` Dave Young
  2008-02-01  2:33     ` David Miller
  0 siblings, 1 reply; 8+ messages in thread
From: Dave Young @ 2008-02-01  1:24 UTC (permalink / raw)
  To: Jens Axboe; +Cc: marcel, davem, linux-kernel, bluez-devel, netdev

On Thu, Jan 31, 2008 at 02:09:30PM +0100, Jens Axboe wrote:
> On Wed, Jan 30 2008, Dave Young wrote:
> > 
> > The bluetooth hci_conn sysfs add/del executed in the default workqueue.
> > If the del_conn is executed after the new add_conn with same target,
> > add_conn will failed with warning of "same kobject name".
> > 
> > Here add btaddconn & btdelconn workqueues,
> > flush the btdelconn workqueue in the add_conn function to avoid the issue.
> > 
> > Signed-off-by: Dave Young <hidave.darkstar@gmail.com> 
> > 
> > ---
> > diff -upr a/net/bluetooth/hci_sysfs.c b/net/bluetooth/hci_sysfs.c
> > --- a/net/bluetooth/hci_sysfs.c	2008-01-30 10:14:27.000000000 +0800
> > +++ b/net/bluetooth/hci_sysfs.c	2008-01-30 10:14:14.000000000 +0800
> > @@ -12,6 +12,8 @@
> >  #undef  BT_DBG
> >  #define BT_DBG(D...)
> >  #endif
> > +static struct workqueue_struct *btaddconn;
> > +static struct workqueue_struct *btdelconn;
> >  
> >  static inline char *typetostr(int type)
> >  {
> > @@ -279,6 +281,7 @@ static void add_conn(struct work_struct 
> >  	struct hci_conn *conn = container_of(work, struct hci_conn, work);
> >  	int i;
> >  
> > +	flush_workqueue(btdelconn);
> >  	if (device_add(&conn->dev) < 0) {
> >  		BT_ERR("Failed to register connection device");
> >  		return;
> > @@ -313,6 +316,7 @@ void hci_conn_add_sysfs(struct hci_conn 
> >  
> >  	INIT_WORK(&conn->work, add_conn);
> >  
> > +	queue_work(btaddconn, &conn->work);
> >  	schedule_work(&conn->work);
> >  }
> 
> So you queue &conn->work on both btaddconn and keventd_wq?

My fault. Thanks for pointing out.

new patch as following (some fixes according to marcel's style as well)

---
[PATCH] bluetooth : add conn add/del workqueues to avoid connection fail

The bluetooth hci conn sysfs add/del executed in the default workqueue.
If the conn del function is executed after the new conn add function
with same bluetooth target address, the connection add will failed
and will warn about same kobject name.

Here add btaddconn & btdelconn workqueues,
flush the btdelconn workqueue in the add_conn function to avoid the issue.

Signed-off-by: Dave Young <hidave.darkstar@gmail.com> 

---
net/bluetooth/hci_sysfs.c |   52 ++++++++++++++++++++++++++++++++++++----------
1 file changed, 41 insertions(+), 11 deletions(-)

diff -upr a/net/bluetooth/hci_sysfs.c b/net/bluetooth/hci_sysfs.c
--- a/net/bluetooth/hci_sysfs.c	2008-02-01 09:18:40.000000000 +0800
+++ b/net/bluetooth/hci_sysfs.c	2008-02-01 09:18:40.000000000 +0800
@@ -12,6 +12,8 @@
 #undef  BT_DBG
 #define BT_DBG(D...)
 #endif
+static struct workqueue_struct *btaddconn;
+static struct workqueue_struct *btdelconn;
 
 static inline char *typetostr(int type)
 {
@@ -279,6 +281,8 @@ static void add_conn(struct work_struct 
 	struct hci_conn *conn = container_of(work, struct hci_conn, work);
 	int i;
 
+	flush_workqueue(btdelconn);
+
 	if (device_add(&conn->dev) < 0) {
 		BT_ERR("Failed to register connection device");
 		return;
@@ -313,7 +317,7 @@ void hci_conn_add_sysfs(struct hci_conn 
 
 	INIT_WORK(&conn->work, add_conn);
 
-	schedule_work(&conn->work);
+	queue_work(btaddconn, &conn->work);
 }
 
 static int __match_tty(struct device *dev, void *data)
@@ -349,7 +353,7 @@ void hci_conn_del_sysfs(struct hci_conn 
 
 	INIT_WORK(&conn->work, del_conn);
 
-	schedule_work(&conn->work);
+	queue_work(btdelconn, &conn->work);
 }
 
 int hci_register_sysfs(struct hci_dev *hdev)
@@ -398,28 +402,54 @@ int __init bt_sysfs_init(void)
 {
 	int err;
 
+	btaddconn = create_singlethread_workqueue("btaddconn");
+	if (!btaddconn) {
+		err = -ENOMEM;
+		goto out;
+	}
+
+	btdelconn = create_singlethread_workqueue("btdelconn");
+	if (!btdelconn) {
+		err = -ENOMEM;
+		goto out_del;
+	}
+
 	bt_platform = platform_device_register_simple("bluetooth", -1, NULL, 0);
-	if (IS_ERR(bt_platform))
-		return PTR_ERR(bt_platform);
+	if (IS_ERR(bt_platform)) {
+		err = PTR_ERR(bt_platform);
+		goto out_platform;
+	}
 
 	err = bus_register(&bt_bus);
-	if (err < 0) {
-		platform_device_unregister(bt_platform);
-		return err;
-	}
+	if (err < 0)
+		goto out_bus;
 
 	bt_class = class_create(THIS_MODULE, "bluetooth");
 	if (IS_ERR(bt_class)) {
-		bus_unregister(&bt_bus);
-		platform_device_unregister(bt_platform);
-		return PTR_ERR(bt_class);
+		err = PTR_ERR(bt_class);
+		goto out_class;
 	}
 
 	return 0;
+
+out_class:
+	bus_unregister(&bt_bus);
+out_bus:
+	platform_device_unregister(bt_platform);
+out_platform:
+	destroy_workqueue(btdelconn);
+out_del:
+	destroy_workqueue(btaddconn);
+out:
+	return err;
 }
 
 void bt_sysfs_cleanup(void)
 {
+	destroy_workqueue(btaddconn);
+
+	destroy_workqueue(btdelconn);
+
 	class_destroy(bt_class);
 
 	bus_unregister(&bt_bus);

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH retry] bluetooth : add conn add/del workqueues to avoid connection fail
  2008-02-01  1:24   ` Dave Young
@ 2008-02-01  2:33     ` David Miller
  2008-02-01  2:57       ` Dave Young
  0 siblings, 1 reply; 8+ messages in thread
From: David Miller @ 2008-02-01  2:33 UTC (permalink / raw)
  To: hidave.darkstar; +Cc: jens.axboe, marcel, linux-kernel, bluez-devel, netdev

From: Dave Young <hidave.darkstar@gmail.com>
Date: Fri, 1 Feb 2008 09:24:41 +0800

> On Thu, Jan 31, 2008 at 02:09:30PM +0100, Jens Axboe wrote:
> > On Wed, Jan 30 2008, Dave Young wrote:
> > > 
> > > The bluetooth hci_conn sysfs add/del executed in the default workqueue.
> > > If the del_conn is executed after the new add_conn with same target,
> > > add_conn will failed with warning of "same kobject name".
> > > 
> > > Here add btaddconn & btdelconn workqueues,
> > > flush the btdelconn workqueue in the add_conn function to avoid the issue.
> > > 
> > > Signed-off-by: Dave Young <hidave.darkstar@gmail.com> 
> > > 
> > > ---
> > > diff -upr a/net/bluetooth/hci_sysfs.c b/net/bluetooth/hci_sysfs.c
> > > --- a/net/bluetooth/hci_sysfs.c	2008-01-30 10:14:27.000000000 +0800
> > > +++ b/net/bluetooth/hci_sysfs.c	2008-01-30 10:14:14.000000000 +0800
> > > @@ -12,6 +12,8 @@
> > >  #undef  BT_DBG
> > >  #define BT_DBG(D...)
> > >  #endif
> > > +static struct workqueue_struct *btaddconn;
> > > +static struct workqueue_struct *btdelconn;
> > >  
> > >  static inline char *typetostr(int type)
> > >  {
> > > @@ -279,6 +281,7 @@ static void add_conn(struct work_struct 
> > >  	struct hci_conn *conn = container_of(work, struct hci_conn, work);
> > >  	int i;
> > >  
> > > +	flush_workqueue(btdelconn);
> > >  	if (device_add(&conn->dev) < 0) {
> > >  		BT_ERR("Failed to register connection device");
> > >  		return;
> > > @@ -313,6 +316,7 @@ void hci_conn_add_sysfs(struct hci_conn 
> > >  
> > >  	INIT_WORK(&conn->work, add_conn);
> > >  
> > > +	queue_work(btaddconn, &conn->work);
> > >  	schedule_work(&conn->work);
> > >  }
> > 
> > So you queue &conn->work on both btaddconn and keventd_wq?
> 
> My fault. Thanks for pointing out.
> 
> new patch as following (some fixes according to marcel's style as well)

Your original patch was already in the tree, so I just checked
in the relative changes.

Please don't me do this next time :-)


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH retry] bluetooth : add conn add/del workqueues to avoid connection fail
  2008-02-01  2:33     ` David Miller
@ 2008-02-01  2:57       ` Dave Young
  0 siblings, 0 replies; 8+ messages in thread
From: Dave Young @ 2008-02-01  2:57 UTC (permalink / raw)
  To: David Miller; +Cc: jens.axboe, marcel, linux-kernel, bluez-devel, netdev

On Feb 1, 2008 10:33 AM, David Miller <davem@davemloft.net> wrote:
> From: Dave Young <hidave.darkstar@gmail.com>
> Date: Fri, 1 Feb 2008 09:24:41 +0800
>
>
> > On Thu, Jan 31, 2008 at 02:09:30PM +0100, Jens Axboe wrote:
> > > On Wed, Jan 30 2008, Dave Young wrote:
> > > >
> > > > The bluetooth hci_conn sysfs add/del executed in the default workqueue.
> > > > If the del_conn is executed after the new add_conn with same target,
> > > > add_conn will failed with warning of "same kobject name".
> > > >
> > > > Here add btaddconn & btdelconn workqueues,
> > > > flush the btdelconn workqueue in the add_conn function to avoid the issue.
> > > >
> > > > Signed-off-by: Dave Young <hidave.darkstar@gmail.com>
> > > >
> > > > ---
> > > > diff -upr a/net/bluetooth/hci_sysfs.c b/net/bluetooth/hci_sysfs.c
> > > > --- a/net/bluetooth/hci_sysfs.c   2008-01-30 10:14:27.000000000 +0800
> > > > +++ b/net/bluetooth/hci_sysfs.c   2008-01-30 10:14:14.000000000 +0800
> > > > @@ -12,6 +12,8 @@
> > > >  #undef  BT_DBG
> > > >  #define BT_DBG(D...)
> > > >  #endif
> > > > +static struct workqueue_struct *btaddconn;
> > > > +static struct workqueue_struct *btdelconn;
> > > >
> > > >  static inline char *typetostr(int type)
> > > >  {
> > > > @@ -279,6 +281,7 @@ static void add_conn(struct work_struct
> > > >   struct hci_conn *conn = container_of(work, struct hci_conn, work);
> > > >   int i;
> > > >
> > > > + flush_workqueue(btdelconn);
> > > >   if (device_add(&conn->dev) < 0) {
> > > >           BT_ERR("Failed to register connection device");
> > > >           return;
> > > > @@ -313,6 +316,7 @@ void hci_conn_add_sysfs(struct hci_conn
> > > >
> > > >   INIT_WORK(&conn->work, add_conn);
> > > >
> > > > + queue_work(btaddconn, &conn->work);
> > > >   schedule_work(&conn->work);
> > > >  }
> > >
> > > So you queue &conn->work on both btaddconn and keventd_wq?
> >
> > My fault. Thanks for pointing out.
> >
> > new patch as following (some fixes according to marcel's style as well)
>
> Your original patch was already in the tree, so I just checked
> in the relative changes.
>
> Please don't me do this next time :-)

David, sorry for it. Should be more careful.

Thanks :)

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2008-02-01  2:58 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-01-30  2:23 [PATCH retry] bluetooth : add conn add/del workqueues to avoid connection fail Dave Young
2008-01-30  5:14 ` David Miller
2008-01-30 10:21   ` Marcel Holtmann
2008-01-31  1:29     ` Dave Young
2008-01-31 13:09 ` Jens Axboe
2008-02-01  1:24   ` Dave Young
2008-02-01  2:33     ` David Miller
2008-02-01  2:57       ` Dave Young

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).