* [PATCH]bluetooth: hci_sysfs connection bus_id add support for diffrent hci device @ 2007-10-31 2:30 Dave Young 2007-10-31 7:01 ` David Miller 0 siblings, 1 reply; 9+ messages in thread From: Dave Young @ 2007-10-31 2:30 UTC (permalink / raw) To: Marcel Holtmann; +Cc: LKML, bluez-devel For multi hci devices host, connection from/to same destination bluetooth device, add_conn will failed due to sysfs duplicate name. sysfs: duplicate filename 'acl0018C5B6B456' can not be created WARNING: at fs/sysfs/dir.c:424 sysfs_add_one() [<c01c0120>] sysfs_add_one+0xa0/0xe0 [<c01c0d7b>] sysfs_create_link+0x9b/0x140 [<c01c1671>] create_files+0x31/0x60 [<c02b537b>] bus_add_device+0x5b/0xf0 [<c02b391c>] device_add+0x11c/0x350 [<f8951410>] add_conn+0x0/0x90 [bluetooth] [<f895141f>] add_conn+0xf/0x90 [bluetooth] [<c013baee>] run_workqueue+0x5e/0x110 [<c013bc4c>] worker_thread+0xac/0x100 [<c0140000>] autoremove_wake_function+0x0/0x50 [<c0140000>] autoremove_wake_function+0x0/0x50 [<c013bba0>] worker_thread+0x0/0x100 [<c013fa39>] kthread+0x59/0xa0 [<c013f9e0>] kthread+0x0/0xa0 [<c0104f83>] kernel_thread_helper+0x7/0x14 ======================= add_conn: Failed to register connection device Add prefix hdev->name to bus_id to fix this problem. Signed-off-by: Dave Young <hidave.darkstar@gmail.com> --- net/bluetooth/hci_sysfs.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff -upr linux/net/bluetooth/hci_sysfs.c linux.new/net/bluetooth/hci_sysfs.c --- linux/net/bluetooth/hci_sysfs.c 2007-10-31 10:21:00.000000000 +0800 +++ linux.new/net/bluetooth/hci_sysfs.c 2007-10-31 10:21:55.000000000 +0800 @@ -302,7 +302,8 @@ void hci_conn_add_sysfs(struct hci_conn conn->dev.release = bt_release; snprintf(conn->dev.bus_id, BUS_ID_SIZE, - "%s%2.2X%2.2X%2.2X%2.2X%2.2X%2.2X", + "%s%s%2.2X%2.2X%2.2X%2.2X%2.2X%2.2X", + hdev->name, conn->type == ACL_LINK ? "acl" : "sco", ba->b[5], ba->b[4], ba->b[3], ba->b[2], ba->b[1], ba->b[0]); ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH]bluetooth: hci_sysfs connection bus_id add support for diffrent hci device 2007-10-31 2:30 [PATCH]bluetooth: hci_sysfs connection bus_id add support for diffrent hci device Dave Young @ 2007-10-31 7:01 ` David Miller 2007-10-31 7:56 ` [Bluez-devel] " Marcel Holtmann 2007-10-31 9:06 ` Dave Young 0 siblings, 2 replies; 9+ messages in thread From: David Miller @ 2007-10-31 7:01 UTC (permalink / raw) To: hidave.darkstar; +Cc: marcel, linux-kernel, bluez-devel From: Dave Young <hidave.darkstar@gmail.com> Date: Wed, 31 Oct 2007 10:30:17 +0800 > diff -upr linux/net/bluetooth/hci_sysfs.c linux.new/net/bluetooth/hci_sysfs.c > --- linux/net/bluetooth/hci_sysfs.c 2007-10-31 10:21:00.000000000 +0800 > +++ linux.new/net/bluetooth/hci_sysfs.c 2007-10-31 10:21:55.000000000 +0800 > @@ -302,7 +302,8 @@ void hci_conn_add_sysfs(struct hci_conn > conn->dev.release = bt_release; > > snprintf(conn->dev.bus_id, BUS_ID_SIZE, > - "%s%2.2X%2.2X%2.2X%2.2X%2.2X%2.2X", > + "%s%s%2.2X%2.2X%2.2X%2.2X%2.2X%2.2X", > + hdev->name, > conn->type == ACL_LINK ? "acl" : "sco", > ba->b[5], ba->b[4], ba->b[3], > ba->b[2], ba->b[1], ba->b[0]); This might not work. Your device's name is already 15 characters long, BUS_ID_SIZE is 20, and it seems hdev->name could easily overflow the 4 or 5 characters of space remaining. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Bluez-devel] [PATCH]bluetooth: hci_sysfs connection bus_id add support for diffrent hci device 2007-10-31 7:01 ` David Miller @ 2007-10-31 7:56 ` Marcel Holtmann 2007-10-31 9:13 ` Dave Young 2007-10-31 9:06 ` Dave Young 1 sibling, 1 reply; 9+ messages in thread From: Marcel Holtmann @ 2007-10-31 7:56 UTC (permalink / raw) To: David Miller; +Cc: bluez-devel, linux-kernel Hi Dave, > > diff -upr linux/net/bluetooth/hci_sysfs.c linux.new/net/bluetooth/hci_sysfs.c > > --- linux/net/bluetooth/hci_sysfs.c 2007-10-31 10:21:00.000000000 +0800 > > +++ linux.new/net/bluetooth/hci_sysfs.c 2007-10-31 10:21:55.000000000 +0800 > > @@ -302,7 +302,8 @@ void hci_conn_add_sysfs(struct hci_conn > > conn->dev.release = bt_release; > > > > snprintf(conn->dev.bus_id, BUS_ID_SIZE, > > - "%s%2.2X%2.2X%2.2X%2.2X%2.2X%2.2X", > > + "%s%s%2.2X%2.2X%2.2X%2.2X%2.2X%2.2X", > > + hdev->name, > > conn->type == ACL_LINK ? "acl" : "sco", > > ba->b[5], ba->b[4], ba->b[3], > > ba->b[2], ba->b[1], ba->b[0]); > > This might not work. > > Your device's name is already 15 characters long, > BUS_ID_SIZE is 20, and it seems hdev->name could > easily overflow the 4 or 5 characters of space > remaining. and should also be not needed since their parents are different. However we had to add the connections to a bus. Otherwise the userspace will never see them. I have to think about the right solution for this problem. Regards Marcel ------------------------------------------------------------------------- This SF.net email is sponsored by: Splunk Inc. Still grepping through log files to find problems? Stop. Now Search log events and configuration files using AJAX and a browser. Download your FREE copy of Splunk now >> http://get.splunk.com/ _______________________________________________ Bluez-devel mailing list Bluez-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/bluez-devel ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH]bluetooth: hci_sysfs connection bus_id add support for diffrent hci device 2007-10-31 7:56 ` [Bluez-devel] " Marcel Holtmann @ 2007-10-31 9:13 ` Dave Young 2007-10-31 9:59 ` [Bluez-devel] " Marcel Holtmann 0 siblings, 1 reply; 9+ messages in thread From: Dave Young @ 2007-10-31 9:13 UTC (permalink / raw) To: Marcel Holtmann; +Cc: David Miller, linux-kernel, bluez-devel On 10/31/07, Marcel Holtmann <marcel@holtmann.org> wrote: > Hi Dave, > > > > diff -upr linux/net/bluetooth/hci_sysfs.c linux.new/net/bluetooth/hci_sysfs.c > > > --- linux/net/bluetooth/hci_sysfs.c 2007-10-31 10:21:00.000000000 +0800 > > > +++ linux.new/net/bluetooth/hci_sysfs.c 2007-10-31 10:21:55.000000000 +0800 > > > @@ -302,7 +302,8 @@ void hci_conn_add_sysfs(struct hci_conn > > > conn->dev.release = bt_release; > > > > > > snprintf(conn->dev.bus_id, BUS_ID_SIZE, > > > - "%s%2.2X%2.2X%2.2X%2.2X%2.2X%2.2X", > > > + "%s%s%2.2X%2.2X%2.2X%2.2X%2.2X%2.2X", > > > + hdev->name, > > > conn->type == ACL_LINK ? "acl" : "sco", > > > ba->b[5], ba->b[4], ba->b[3], > > > ba->b[2], ba->b[1], ba->b[0]); > > > > This might not work. > > > > Your device's name is already 15 characters long, > > BUS_ID_SIZE is 20, and it seems hdev->name could > > easily overflow the 4 or 5 characters of space > > remaining. > > and should also be not needed since their parents are different. However > we had to add the connections to a bus. Otherwise the userspace will > never see them. I have to think about the right solution for this > problem. Maybe we can use this instead: snprintf(conn->dev.bus_id, BUS_ID_SIZE, - "%s%2.2X%2.2X%2.2X%2.2X%2.2X%2.2X", + "%s%s%2.2X%2.2X%2.2X%2.2X%2.2X%2.2X", + hdev->name + 3, conn->type == ACL_LINK ? "acl" : "sco", ba->b[5], ba->b[4], ba->b[3], ba->b[2], ba->b[1], ba->b[0]); Regards dave ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Bluez-devel] [PATCH]bluetooth: hci_sysfs connection bus_id add support for diffrent hci device 2007-10-31 9:13 ` Dave Young @ 2007-10-31 9:59 ` Marcel Holtmann 2007-11-01 1:05 ` Dave Young 0 siblings, 1 reply; 9+ messages in thread From: Marcel Holtmann @ 2007-10-31 9:59 UTC (permalink / raw) To: Dave Young; +Cc: bluez-devel, David Miller, linux-kernel [-- Attachment #1: Type: text/plain, Size: 1964 bytes --] Hi Dave, > > > > diff -upr linux/net/bluetooth/hci_sysfs.c linux.new/net/bluetooth/hci_sysfs.c > > > > --- linux/net/bluetooth/hci_sysfs.c 2007-10-31 10:21:00.000000000 +0800 > > > > +++ linux.new/net/bluetooth/hci_sysfs.c 2007-10-31 10:21:55.000000000 +0800 > > > > @@ -302,7 +302,8 @@ void hci_conn_add_sysfs(struct hci_conn > > > > conn->dev.release = bt_release; > > > > > > > > snprintf(conn->dev.bus_id, BUS_ID_SIZE, > > > > - "%s%2.2X%2.2X%2.2X%2.2X%2.2X%2.2X", > > > > + "%s%s%2.2X%2.2X%2.2X%2.2X%2.2X%2.2X", > > > > + hdev->name, > > > > conn->type == ACL_LINK ? "acl" : "sco", > > > > ba->b[5], ba->b[4], ba->b[3], > > > > ba->b[2], ba->b[1], ba->b[0]); > > > > > > This might not work. > > > > > > Your device's name is already 15 characters long, > > > BUS_ID_SIZE is 20, and it seems hdev->name could > > > easily overflow the 4 or 5 characters of space > > > remaining. > > > > and should also be not needed since their parents are different. However > > we had to add the connections to a bus. Otherwise the userspace will > > never see them. I have to think about the right solution for this > > problem. > > Maybe we can use this instead: > snprintf(conn->dev.bus_id, BUS_ID_SIZE, > - "%s%2.2X%2.2X%2.2X%2.2X%2.2X%2.2X", > + "%s%s%2.2X%2.2X%2.2X%2.2X%2.2X%2.2X", > + hdev->name + 3, > conn->type == ACL_LINK ? "acl" : "sco", > ba->b[5], ba->b[4], ba->b[3], > ba->b[2], ba->b[1], ba->b[0]); I had a look on how other subsystems handle this case and yes, they duplicate the id number from its parent device. So I applied the attached patch to my tree. The best would be to use '"%d-%d", hdev->id, conn->handle', but at the time we create the hci_conn structure and add the sysfs entry the connection handle is not know yet. I have to look into that and see if that can be changed. Regards Marcel [-- Attachment #2: patch --] [-- Type: text/x-patch, Size: 558 bytes --] diff --git a/net/bluetooth/hci_sysfs.c b/net/bluetooth/hci_sysfs.c index cef1e3e..0cd9bd5 100644 --- a/net/bluetooth/hci_sysfs.c +++ b/net/bluetooth/hci_sysfs.c @@ -302,8 +302,8 @@ void hci_conn_add_sysfs(struct hci_conn *conn) conn->dev.release = bt_release; snprintf(conn->dev.bus_id, BUS_ID_SIZE, - "%s%2.2X%2.2X%2.2X%2.2X%2.2X%2.2X", - conn->type == ACL_LINK ? "acl" : "sco", + "%d-%s%2.2X%2.2X%2.2X%2.2X%2.2X%2.2X", + hdev->id, conn->type == ACL_LINK ? "acl" : "sco", ba->b[5], ba->b[4], ba->b[3], ba->b[2], ba->b[1], ba->b[0]); [-- Attachment #3: Type: text/plain, Size: 314 bytes --] ------------------------------------------------------------------------- This SF.net email is sponsored by: Splunk Inc. Still grepping through log files to find problems? Stop. Now Search log events and configuration files using AJAX and a browser. Download your FREE copy of Splunk now >> http://get.splunk.com/ [-- Attachment #4: Type: text/plain, Size: 164 bytes --] _______________________________________________ Bluez-devel mailing list Bluez-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/bluez-devel ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH]bluetooth: hci_sysfs connection bus_id add support for diffrent hci device 2007-10-31 9:59 ` [Bluez-devel] " Marcel Holtmann @ 2007-11-01 1:05 ` Dave Young 2007-11-01 2:20 ` Dave Young 0 siblings, 1 reply; 9+ messages in thread From: Dave Young @ 2007-11-01 1:05 UTC (permalink / raw) To: Marcel Holtmann; +Cc: David Miller, linux-kernel, bluez-devel On 10/31/07, Marcel Holtmann <marcel@holtmann.org> wrote: > Hi Dave, > > > > > > diff -upr linux/net/bluetooth/hci_sysfs.c linux.new/net/bluetooth/hci_sysfs.c > > > > > --- linux/net/bluetooth/hci_sysfs.c 2007-10-31 10:21:00.000000000 +0800 > > > > > +++ linux.new/net/bluetooth/hci_sysfs.c 2007-10-31 10:21:55.000000000 +0800 > > > > > @@ -302,7 +302,8 @@ void hci_conn_add_sysfs(struct hci_conn > > > > > conn->dev.release = bt_release; > > > > > > > > > > snprintf(conn->dev.bus_id, BUS_ID_SIZE, > > > > > - "%s%2.2X%2.2X%2.2X%2.2X%2.2X%2.2X", > > > > > + "%s%s%2.2X%2.2X%2.2X%2.2X%2.2X%2.2X", > > > > > + hdev->name, > > > > > conn->type == ACL_LINK ? "acl" : "sco", > > > > > ba->b[5], ba->b[4], ba->b[3], > > > > > ba->b[2], ba->b[1], ba->b[0]); > > > > > > > > This might not work. > > > > > > > > Your device's name is already 15 characters long, > > > > BUS_ID_SIZE is 20, and it seems hdev->name could > > > > easily overflow the 4 or 5 characters of space > > > > remaining. > > > > > > and should also be not needed since their parents are different. However > > > we had to add the connections to a bus. Otherwise the userspace will > > > never see them. I have to think about the right solution for this > > > problem. > > > > Maybe we can use this instead: > > snprintf(conn->dev.bus_id, BUS_ID_SIZE, > > - "%s%2.2X%2.2X%2.2X%2.2X%2.2X%2.2X", > > + "%s%s%2.2X%2.2X%2.2X%2.2X%2.2X%2.2X", > > + hdev->name + 3, > > conn->type == ACL_LINK ? "acl" : "sco", > > ba->b[5], ba->b[4], ba->b[3], > > ba->b[2], ba->b[1], ba->b[0]); > > I had a look on how other subsystems handle this case and yes, they > duplicate the id number from its parent device. So I applied the > attached patch to my tree. Thanks. > > The best would be to use '"%d-%d", hdev->id, conn->handle', but at the > time we create the hci_conn structure and add the sysfs entry the > connection handle is not know yet. I have to look into that and see if > that can be changed. I agree. I wonder to know whether the conn sys issue can be delayed after the handle is availible. Regards dave ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH]bluetooth: hci_sysfs connection bus_id add support for diffrent hci device 2007-11-01 1:05 ` Dave Young @ 2007-11-01 2:20 ` Dave Young 2007-11-01 4:56 ` [Bluez-devel] " Marcel Holtmann 0 siblings, 1 reply; 9+ messages in thread From: Dave Young @ 2007-11-01 2:20 UTC (permalink / raw) To: Marcel Holtmann; +Cc: David Miller, linux-kernel, bluez-devel On 11/1/07, Dave Young <hidave.darkstar@gmail.com> wrote: > On 10/31/07, Marcel Holtmann <marcel@holtmann.org> wrote: > > Hi Dave, > > > > > > > > diff -upr linux/net/bluetooth/hci_sysfs.c linux.new/net/bluetooth/hci_sysfs.c > > > > > > --- linux/net/bluetooth/hci_sysfs.c 2007-10-31 10:21:00.000000000 +0800 > > > > > > +++ linux.new/net/bluetooth/hci_sysfs.c 2007-10-31 10:21:55.000000000 +0800 > > > > > > @@ -302,7 +302,8 @@ void hci_conn_add_sysfs(struct hci_conn > > > > > > conn->dev.release = bt_release; > > > > > > > > > > > > snprintf(conn->dev.bus_id, BUS_ID_SIZE, > > > > > > - "%s%2.2X%2.2X%2.2X%2.2X%2.2X%2.2X", > > > > > > + "%s%s%2.2X%2.2X%2.2X%2.2X%2.2X%2.2X", > > > > > > + hdev->name, > > > > > > conn->type == ACL_LINK ? "acl" : "sco", > > > > > > ba->b[5], ba->b[4], ba->b[3], > > > > > > ba->b[2], ba->b[1], ba->b[0]); > > > > > > > > > > This might not work. > > > > > > > > > > Your device's name is already 15 characters long, > > > > > BUS_ID_SIZE is 20, and it seems hdev->name could > > > > > easily overflow the 4 or 5 characters of space > > > > > remaining. > > > > > > > > and should also be not needed since their parents are different. However > > > > we had to add the connections to a bus. Otherwise the userspace will > > > > never see them. I have to think about the right solution for this > > > > problem. > > > > > > Maybe we can use this instead: > > > snprintf(conn->dev.bus_id, BUS_ID_SIZE, > > > - "%s%2.2X%2.2X%2.2X%2.2X%2.2X%2.2X", > > > + "%s%s%2.2X%2.2X%2.2X%2.2X%2.2X%2.2X", > > > + hdev->name + 3, > > > conn->type == ACL_LINK ? "acl" : "sco", > > > ba->b[5], ba->b[4], ba->b[3], > > > ba->b[2], ba->b[1], ba->b[0]); > > > > I had a look on how other subsystems handle this case and yes, they > > duplicate the id number from its parent device. So I applied the > > attached patch to my tree. Hi, Sorry for my noise. Theoretically, for the "%d-%s" version, overwrite is also possible. hdev->id is u16, imagine the id is 65535, then total length is 21. IMHO, temporarily, the ""%s" , hdev->name +3 "version is better, because the hdev->name len is 8, except for the end nul character, remain 4 to print, so 4+15 is just KOBJ_NAME_LEN - 1. Regards dave ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Bluez-devel] [PATCH]bluetooth: hci_sysfs connection bus_id add support for diffrent hci device 2007-11-01 2:20 ` Dave Young @ 2007-11-01 4:56 ` Marcel Holtmann 0 siblings, 0 replies; 9+ messages in thread From: Marcel Holtmann @ 2007-11-01 4:56 UTC (permalink / raw) To: Dave Young; +Cc: bluez-devel, David Miller, linux-kernel Hi Dave, > > > Maybe we can use this instead: > > > > snprintf(conn->dev.bus_id, BUS_ID_SIZE, > > > > - "%s%2.2X%2.2X%2.2X%2.2X%2.2X%2.2X", > > > > + "%s%s%2.2X%2.2X%2.2X%2.2X%2.2X%2.2X", > > > > + hdev->name + 3, > > > > conn->type == ACL_LINK ? "acl" : "sco", > > > > ba->b[5], ba->b[4], ba->b[3], > > > > ba->b[2], ba->b[1], ba->b[0]); > > > > > > I had a look on how other subsystems handle this case and yes, they > > > duplicate the id number from its parent device. So I applied the > > > attached patch to my tree. > > Sorry for my noise. > Theoretically, for the "%d-%s" version, overwrite is also possible. > > hdev->id is u16, imagine the id is 65535, then total length is 21. even to reach 3 digits you have to attach 100 Bluetooth devices at the same time. Tell me when you have done that ;) However, as I said, we have to move to using the connection handle. Regards Marcel ------------------------------------------------------------------------- This SF.net email is sponsored by: Splunk Inc. Still grepping through log files to find problems? Stop. Now Search log events and configuration files using AJAX and a browser. Download your FREE copy of Splunk now >> http://get.splunk.com/ _______________________________________________ Bluez-devel mailing list Bluez-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/bluez-devel ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH]bluetooth: hci_sysfs connection bus_id add support for diffrent hci device 2007-10-31 7:01 ` David Miller 2007-10-31 7:56 ` [Bluez-devel] " Marcel Holtmann @ 2007-10-31 9:06 ` Dave Young 1 sibling, 0 replies; 9+ messages in thread From: Dave Young @ 2007-10-31 9:06 UTC (permalink / raw) To: David Miller; +Cc: marcel, linux-kernel, bluez-devel On 10/31/07, David Miller <davem@davemloft.net> wrote: > From: Dave Young <hidave.darkstar@gmail.com> > Date: Wed, 31 Oct 2007 10:30:17 +0800 > > > diff -upr linux/net/bluetooth/hci_sysfs.c linux.new/net/bluetooth/hci_sysfs.c > > --- linux/net/bluetooth/hci_sysfs.c 2007-10-31 10:21:00.000000000 +0800 > > +++ linux.new/net/bluetooth/hci_sysfs.c 2007-10-31 10:21:55.000000000 +0800 > > @@ -302,7 +302,8 @@ void hci_conn_add_sysfs(struct hci_conn > > conn->dev.release = bt_release; > > > > snprintf(conn->dev.bus_id, BUS_ID_SIZE, > > - "%s%2.2X%2.2X%2.2X%2.2X%2.2X%2.2X", > > + "%s%s%2.2X%2.2X%2.2X%2.2X%2.2X%2.2X", > > + hdev->name, > > conn->type == ACL_LINK ? "acl" : "sco", > > ba->b[5], ba->b[4], ba->b[3], > > ba->b[2], ba->b[1], ba->b[0]); > > This might not work. > > Your device's name is already 15 characters long, > BUS_ID_SIZE is 20, and it seems hdev->name could > easily overflow the 4 or 5 characters of space > remaining. > Yes, this is possible due to hdev->name length is 8. The space for it ist 5 characters. IMHO,in normal usage 'hciX' is unlikely over this limit. Any better ideas? ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2007-11-01 4:56 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2007-10-31 2:30 [PATCH]bluetooth: hci_sysfs connection bus_id add support for diffrent hci device Dave Young 2007-10-31 7:01 ` David Miller 2007-10-31 7:56 ` [Bluez-devel] " Marcel Holtmann 2007-10-31 9:13 ` Dave Young 2007-10-31 9:59 ` [Bluez-devel] " Marcel Holtmann 2007-11-01 1:05 ` Dave Young 2007-11-01 2:20 ` Dave Young 2007-11-01 4:56 ` [Bluez-devel] " Marcel Holtmann 2007-10-31 9:06 ` 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).