[BUG][PATCH -mm] bluetooth : rfcomm add get/put device in del_conn

[BUG][PATCH -mm] bluetooth : rfcomm add get/put device in del_conn

[Dave Young]

Due to 2.6.24-rc6-mm1 kernel changes (maybe kobject or driver core), If I exec:

rfcomm connect 0 1

kernel will oops after connect timeout.

hand copy some oops text:

EIP is at driver_sysfs_remove+0x1a/0x40
Call Trace:
 show_trace_log_lvl+0x1a/0x30
 show_stack_log_lvl+0x9a/0xc0
 show_registers+0xc7/0x270
 die+0x129/0x240
 do_page_fault+0x3a1/0x670
 error_code+0x72/0x78
 __device_release_driver+0x1e/0xa0
 device_release_driver+0x30/0x50
 bus_remove_device+0x63/0x90
 device_del+0x55/0x190
 del_conn+0xb/0x10 [bluetooth]
 run_workqueue+0xe1/0x210
 worker_thread+0x99/0xf0
 kthread+0x5c/0xa0
 kernel_thread_helper+0x7/0x18

(The remote bluetooth device is a mobile phone which is power off)

The reason is that in bus_remobe_dev, the klist_del function will release the device, so just add a get/put pair around the device_del in del_conn.

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

---
net/bluetooth/hci_sysfs.c |    2 ++
1 file changed, 2 insertions(+)

diff -upr linux/net/bluetooth/hci_sysfs.c linux.new/net/bluetooth/hci_sysfs.c
--- linux/net/bluetooth/hci_sysfs.c	2007-12-25 17:45:09.000000000 +0800
+++ linux.new/net/bluetooth/hci_sysfs.c	2007-12-25 17:45:51.000000000 +0800
@@ -319,7 +319,9 @@ void hci_conn_add_sysfs(struct hci_conn 
 static void del_conn(struct work_struct *work)
 {
 	struct hci_conn *conn = container_of(work, struct hci_conn, work);
+	get_device(&conn->dev);
 	device_del(&conn->dev);
+	put_device(&conn->dev);
 }
 
 void hci_conn_del_sysfs(struct hci_conn *conn)

Re: [BUG][PATCH -mm] bluetooth : rfcomm add get/put device in del_conn

[Dave Young]

On Dec 25, 2007 6:03 PM, Dave Young <hidave.darkstar@gmail.com> wrote:
> Due to 2.6.24-rc6-mm1 kernel changes (maybe kobject or driver core), If I exec:
>
> rfcomm connect 0 1
>
> kernel will oops after connect timeout.
>
> hand copy some oops text:
>
> EIP is at driver_sysfs_remove+0x1a/0x40
> Call Trace:
>  show_trace_log_lvl+0x1a/0x30
>  show_stack_log_lvl+0x9a/0xc0
>  show_registers+0xc7/0x270
>  die+0x129/0x240
>  do_page_fault+0x3a1/0x670
>  error_code+0x72/0x78
>  __device_release_driver+0x1e/0xa0
>  device_release_driver+0x30/0x50
>  bus_remove_device+0x63/0x90
>  device_del+0x55/0x190
>  del_conn+0xb/0x10 [bluetooth]
>  run_workqueue+0xe1/0x210
>  worker_thread+0x99/0xf0
>  kthread+0x5c/0xa0
>  kernel_thread_helper+0x7/0x18
>
> (The remote bluetooth device is a mobile phone which is power off)
>
> The reason is that in bus_remobe_dev, the klist_del function will release the device, so just add a get/put pair around the device_del in del_conn.
>
> Signed-off-by: Dave Young <hidave.darkstar@gmail.com>
>
> ---
> net/bluetooth/hci_sysfs.c |    2 ++
> 1 file changed, 2 insertions(+)
>
> diff -upr linux/net/bluetooth/hci_sysfs.c linux.new/net/bluetooth/hci_sysfs.c
> --- linux/net/bluetooth/hci_sysfs.c     2007-12-25 17:45:09.000000000 +0800
> +++ linux.new/net/bluetooth/hci_sysfs.c 2007-12-25 17:45:51.000000000 +0800
> @@ -319,7 +319,9 @@ void hci_conn_add_sysfs(struct hci_conn
>  static void del_conn(struct work_struct *work)
>  {
>         struct hci_conn *conn = container_of(work, struct hci_conn, work);
> +       get_device(&conn->dev);
>         device_del(&conn->dev);
> +       put_device(&conn->dev);
>  }
>
>  void hci_conn_del_sysfs(struct hci_conn *conn)
>

Hi greg,

BTW, Is it a possible bug of driver core or kobject ?

Regards
dave

Re: [BUG][PATCH -mm] bluetooth : rfcomm add get/put device in del_conn

[Dave Young]

On Tue, Dec 25, 2007 at 06:07:24PM +0800, Dave Young wrote:
> On Dec 25, 2007 6:03 PM, Dave Young <hidave.darkstar@gmail.com> wrote:
> > Due to 2.6.24-rc6-mm1 kernel changes (maybe kobject or driver core), If I exec:
> >
> > rfcomm connect 0 1
> >
> > kernel will oops after connect timeout.
> >
> > hand copy some oops text:
> >
> > EIP is at driver_sysfs_remove+0x1a/0x40
> > Call Trace:
> >  show_trace_log_lvl+0x1a/0x30
> >  show_stack_log_lvl+0x9a/0xc0
> >  show_registers+0xc7/0x270
> >  die+0x129/0x240
> >  do_page_fault+0x3a1/0x670
> >  error_code+0x72/0x78
> >  __device_release_driver+0x1e/0xa0
> >  device_release_driver+0x30/0x50
> >  bus_remove_device+0x63/0x90
> >  device_del+0x55/0x190
> >  del_conn+0xb/0x10 [bluetooth]
> >  run_workqueue+0xe1/0x210
> >  worker_thread+0x99/0xf0
> >  kthread+0x5c/0xa0
> >  kernel_thread_helper+0x7/0x18
> >
> > (The remote bluetooth device is a mobile phone which is power off)
> >
> > The reason is that in bus_remobe_dev, the klist_del function will release the device, so just add a get/put pair around the device_del in del_conn.
> >
> > Signed-off-by: Dave Young <hidave.darkstar@gmail.com>
> >
> > ---
> > net/bluetooth/hci_sysfs.c |    2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff -upr linux/net/bluetooth/hci_sysfs.c linux.new/net/bluetooth/hci_sysfs.c
> > --- linux/net/bluetooth/hci_sysfs.c     2007-12-25 17:45:09.000000000 +0800
> > +++ linux.new/net/bluetooth/hci_sysfs.c 2007-12-25 17:45:51.000000000 +0800
> > @@ -319,7 +319,9 @@ void hci_conn_add_sysfs(struct hci_conn
> >  static void del_conn(struct work_struct *work)
> >  {
> >         struct hci_conn *conn = container_of(work, struct hci_conn, work);
> > +       get_device(&conn->dev);
> >         device_del(&conn->dev);
> > +       put_device(&conn->dev);
> >  }
> >
> >  void hci_conn_del_sysfs(struct hci_conn *conn)
> >
> 
> Hi greg,
> 
> BTW, Is it a possible bug of driver core or kobject ?
> 

In device_del 

if(parent)
    klist_del()  will drop the kref of device

and then in bus_remove_device
    klist_del()  will drop it again, so the device would be released.

then the following works will oops.

So might my patch is a wrong fix. how about the below one:

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

---
drivers/base/core.c |    2 ++
1 file changed, 2 insertions(+)

diff -upr linux/drivers/base/core.c linux.new/drivers/base/core.c
--- linux/drivers/base/core.c	2007-12-26 11:00:49.000000000 +0800
+++ linux.new/drivers/base/core.c	2007-12-26 11:01:18.000000000 +0800
@@ -926,6 +926,7 @@ void device_del(struct device * dev)
 	struct device * parent = dev->parent;
 	struct class_interface *class_intf;
 
+	dev = get_device(dev);
 	if (parent)
 		klist_del(&dev->knode_parent);
 	if (MAJOR(dev->devt))
@@ -966,6 +967,7 @@ void device_del(struct device * dev)
 	cleanup_device_parent(dev);
 	kobject_del(&dev->kobj);
 	put_device(parent);
+	put_device(dev);
 }
 
 /**

Re: [BUG][PATCH -mm] bluetooth : rfcomm add get/put device in del_conn

[Dave Young]

On Dec 26, 2007 11:13 AM, Dave Young <hidave.darkstar@gmail.com> wrote:
>
> On Tue, Dec 25, 2007 at 06:07:24PM +0800, Dave Young wrote:
> > On Dec 25, 2007 6:03 PM, Dave Young <hidave.darkstar@gmail.com> wrote:
> > > Due to 2.6.24-rc6-mm1 kernel changes (maybe kobject or driver core), If I exec:
> > >
> > > rfcomm connect 0 1
> > >
> > > kernel will oops after connect timeout.
> > >
> > > hand copy some oops text:
> > >
> > > EIP is at driver_sysfs_remove+0x1a/0x40
> > > Call Trace:
> > >  show_trace_log_lvl+0x1a/0x30
> > >  show_stack_log_lvl+0x9a/0xc0
> > >  show_registers+0xc7/0x270
> > >  die+0x129/0x240
> > >  do_page_fault+0x3a1/0x670
> > >  error_code+0x72/0x78
> > >  __device_release_driver+0x1e/0xa0
> > >  device_release_driver+0x30/0x50
> > >  bus_remove_device+0x63/0x90
> > >  device_del+0x55/0x190
> > >  del_conn+0xb/0x10 [bluetooth]
> > >  run_workqueue+0xe1/0x210
> > >  worker_thread+0x99/0xf0
> > >  kthread+0x5c/0xa0
> > >  kernel_thread_helper+0x7/0x18
> > >
> > > (The remote bluetooth device is a mobile phone which is power off)
> > >
> > > The reason is that in bus_remobe_dev, the klist_del function will release the device, so just add a get/put pair around the device_del in del_conn.
> > >
> > > Signed-off-by: Dave Young <hidave.darkstar@gmail.com>
> > >
> > > ---
> > > net/bluetooth/hci_sysfs.c |    2 ++
> > > 1 file changed, 2 insertions(+)
> > >
> > > diff -upr linux/net/bluetooth/hci_sysfs.c linux.new/net/bluetooth/hci_sysfs.c
> > > --- linux/net/bluetooth/hci_sysfs.c     2007-12-25 17:45:09.000000000 +0800
> > > +++ linux.new/net/bluetooth/hci_sysfs.c 2007-12-25 17:45:51.000000000 +0800
> > > @@ -319,7 +319,9 @@ void hci_conn_add_sysfs(struct hci_conn
> > >  static void del_conn(struct work_struct *work)
> > >  {
> > >         struct hci_conn *conn = container_of(work, struct hci_conn, work);
> > > +       get_device(&conn->dev);
> > >         device_del(&conn->dev);
> > > +       put_device(&conn->dev);
> > >  }
> > >
> > >  void hci_conn_del_sysfs(struct hci_conn *conn)
> > >
> >
> > Hi greg,
> >
> > BTW, Is it a possible bug of driver core or kobject ?
> >
>
> In device_del
>
> if(parent)
>     klist_del()  will drop the kref of device
>
> and then in bus_remove_device
>     klist_del()  will drop it again, so the device would be released.
>
> then the following works will oops.
>
> So might my patch is a wrong fix. how about the below one:
>
> Signed-off-by: Dave Young <hidave.darkstar@gmail.com>
>
> ---
> drivers/base/core.c |    2 ++
> 1 file changed, 2 insertions(+)
>
> diff -upr linux/drivers/base/core.c linux.new/drivers/base/core.c
> --- linux/drivers/base/core.c   2007-12-26 11:00:49.000000000 +0800
> +++ linux.new/drivers/base/core.c       2007-12-26 11:01:18.000000000 +0800
> @@ -926,6 +926,7 @@ void device_del(struct device * dev)
>         struct device * parent = dev->parent;
>         struct class_interface *class_intf;
>
> +       dev = get_device(dev);
>         if (parent)
>                 klist_del(&dev->knode_parent);
>         if (MAJOR(dev->devt))
> @@ -966,6 +967,7 @@ void device_del(struct device * dev)
>         cleanup_device_parent(dev);
>         kobject_del(&dev->kobj);
>         put_device(parent);
> +       put_device(dev);
>  }
>
>  /**
>

Hi all,
The patches in this thread of me are wrong fixes, I'm sorry for this.

I've found the real problem(BUG), it's in bluetooth connection code,
because of workqueue delay, the device could be put before the
device_del called.

I will submit a new patch in a while.

Regards
dave