* Re: 2.6.21-rc7: BUG: sleeping function called from invalid context at net/core/sock.c:1523
@ 2007-05-11 13:29 ` Satyam Sharma
0 siblings, 0 replies; 31+ messages in thread
From: Satyam Sharma @ 2007-05-11 13:29 UTC (permalink / raw)
To: Jiri Kosina
Cc: Jeremy Fitzhardinge, netdev, Marcel Holtmann, Greg KH,
Linux Kernel Mailing List, Cedric Le Goater, bluez-devel, maxk
Hi Jiri,
On 4/26/07, Jiri Kosina <jikos@jikos.cz> wrote:
> On Mon, 23 Apr 2007, Jiri Kosina wrote:
>
> > > BUG: sleeping function called from invalid context at net/core/sock.c:1523
> > > in_atomic():1, irqs_disabled():0
> > > 1 lock held by khubd/180:
> > > #0: (old_style_rw_init#2){-.-?}, at: [<f88c5816>] hci_sock_dev_event+0x42/0xc5 [bluetooth]
> [...]
> > OK, this probably started happening since b40df5743. Before that commit,
> > hci_sock_dev_event() used bh_lock_sock() to lock the corresponding
> > struct sock. This was obviously buggy - not deadlock safe against
> > l2cap_connect_cfm() from softirq context. This however introduced
> > another problem - hci_sock_dev_event() is now obviously being triggered
> > (for HCI_DEV_UNREG event, when suspending) in atomic context with
I saw that hci_sock_dev_event() is _always_ triggered in atomic
context. It's the callout for hci_notifier which is defined as an
atomic notifier chain (hence executed in an RCU read section -- and
sleeping inside that would be illegal).
> > preemption disabled. This is what lock_sock_nested() complains about, as
> > it is allowed to sleep inside __lock_sock(), waiting for the lock owner.
> [...]
> Bluetooth: postpone hci_dev unregistration
>
> Commit b40df57 substituted bh_lock_sock() in hci_sock_dev_event() for
> lock_sock() when unregistering HCI device, in order to prevent deadlock
> against locking in l2cap_connect_cfm() from softirq context.
Isn't this a problem faced by other places in the kernel already
(where simply using bh_lock_sock() would potentially deadlock with
another thread? I wonder what's the "recommended" (or one that's
generally used) way to handle such a case.
> This however introduces another problem - hci_sock_dev_event() for
> HCI_DEV_UNREG can also be triggered in atomic context, in which calling
Actually, I remember going over the hci_sock_dev_event() calling
codepath (in reverse) quite exhaustively, and did not find a
legitimate reason why anybody would want it to be atomic. hci_notify()
has six call sites, and all are sleep-capable, IMO. In the case of
hci_unregister_dev(), for example, what's happening is as follows:
__device_release_driver (can sleep)
usb_unbind_interface (-"-)
hci_usb_disconnect [hci_usb] (can sleep *[1])
hci_unregister_dev [bluetooth] (-"-)
hci_notify [bluetooth] (-"-)
atomic_notifier_call_chain (contains RCU read section)
notifier_call_chain (therefore, CANNOT SLEEP [2])
hci_sock_dev_event [bluetooth] (-"-)
lock_sock_nested (MIGHT SLEEP *BUG*)
__might_sleep
[1] This is the first problem point. However, I didn't find any reason
why this particular driver's .disconnect() couldn't sleep. In fact, a
comment in include/linux/usb.h:811 says:
"The probe() and disconnect() methods are called in a context where
they can sleep, but they should avoid abusing the privilege. Most
work to connect to a device should be done when the device is opened,
and undone at the last close. The disconnect code needs to address
concurrency issues with respect to open() and close() methods, as
well as forcing all pending I/O requests to complete (by unlinking
them as necessary, and blocking until the unlinks complete)."
I'm assuming the comment is not obsolete, of course, but although the
first sentence says .disconnect() shouldn't abuse the privilege to
sleep, the last sentence makes it quite evident that we are _allowed_
to do so anyway, and that is how things are (with the hci_usb driver,
at least, I didn't check the .remove() or .disconnect() functions of other
USB drivers, however).
[2] This is a bogus (and unnecessary) can-sleep-to-cannot-sleep
transition point, IMO. I had copied Alan Stern in another thread a few
days back, and he wasn't sure why hci_notifier was classified as an
atomic notifier chain (when that classification happened with the new
notifier chains API). I had submitted a patch that merely changed 4
lines in net/bluetooth/hci_core.c to convert hci_notifier to a blocking
notifier chain, but couldn't test as I own no bluetooth hardware myself.
So do we ever really _need_ hci_sock_dev_event() to run in atomic
context at all?
> lock_sock() is not safe as it could sleep.
>
> This patch moves the detaching of sockets from hci_device into workqueue,
> so that lock_sock() can be used safely. This requires movement of
I did a workqueue conversion myself, but ran into the following problem:
In the scheduled work function we have:
> + read_lock(&hci_sk_list.lock);
> + sk_for_each(sk, node, &hci_sk_list.head) {
> + lock_sock(sk);
This would still be illegal, we can't sleep while holding an rwlock
(hci_sk_list.lock above). Converting hci_sk_list.lock to an rwsem
is _even_ more problematic, because hci_send_to_sock()
just *cannot* sleep.
> deallocation of hci_dev - deallocating device just after
> hci_unregister_dev() would be too soon, as it could happen before the
> workqueue has been run.
Suggest a better solution for this: just introduce a flush_scheduled_work()
after hci_unregister_dev() but before hci_free_dev() in all those places.
Less disruptive that way.
So this is quite an interesting problem indeed, but I can't help wondering
that this must be faced elsewhere in the kernel (other users of lock_sock)
too. CC'ing netdev@, for any ideas.
(later)
I Googled a bit to see if this problem was faced elsewhere in the kernel
too. Saw the following commit by Ingo Molnar
(9883a13c72dbf8c518814b6091019643cdb34429):
- lock_sock(sock->sk);
+ local_bh_disable();
+ bh_lock_sock_nested(sock->sk);
rc = selinux_netlbl_socket_setsid(sock, sksec->sid);
- release_sock(sock->sk);
+ bh_unlock_sock(sock->sk);
+ local_bh_enable();
Is it _really_ *this* simple?
Satyam
-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: 2.6.21-rc7: BUG: sleeping function called from invalid context at net/core/sock.c:1523
@ 2007-05-11 13:29 ` Satyam Sharma
0 siblings, 0 replies; 31+ messages in thread
From: Satyam Sharma @ 2007-05-11 13:29 UTC (permalink / raw)
To: Jiri Kosina
Cc: Marcel Holtmann, Greg KH, Jeremy Fitzhardinge, maxk, bluez-devel,
Cedric Le Goater, Linux Kernel Mailing List, netdev
Hi Jiri,
On 4/26/07, Jiri Kosina <jikos@jikos.cz> wrote:
> On Mon, 23 Apr 2007, Jiri Kosina wrote:
>
> > > BUG: sleeping function called from invalid context at net/core/sock.c:1523
> > > in_atomic():1, irqs_disabled():0
> > > 1 lock held by khubd/180:
> > > #0: (old_style_rw_init#2){-.-?}, at: [<f88c5816>] hci_sock_dev_event+0x42/0xc5 [bluetooth]
> [...]
> > OK, this probably started happening since b40df5743. Before that commit,
> > hci_sock_dev_event() used bh_lock_sock() to lock the corresponding
> > struct sock. This was obviously buggy - not deadlock safe against
> > l2cap_connect_cfm() from softirq context. This however introduced
> > another problem - hci_sock_dev_event() is now obviously being triggered
> > (for HCI_DEV_UNREG event, when suspending) in atomic context with
I saw that hci_sock_dev_event() is _always_ triggered in atomic
context. It's the callout for hci_notifier which is defined as an
atomic notifier chain (hence executed in an RCU read section -- and
sleeping inside that would be illegal).
> > preemption disabled. This is what lock_sock_nested() complains about, as
> > it is allowed to sleep inside __lock_sock(), waiting for the lock owner.
> [...]
> Bluetooth: postpone hci_dev unregistration
>
> Commit b40df57 substituted bh_lock_sock() in hci_sock_dev_event() for
> lock_sock() when unregistering HCI device, in order to prevent deadlock
> against locking in l2cap_connect_cfm() from softirq context.
Isn't this a problem faced by other places in the kernel already
(where simply using bh_lock_sock() would potentially deadlock with
another thread? I wonder what's the "recommended" (or one that's
generally used) way to handle such a case.
> This however introduces another problem - hci_sock_dev_event() for
> HCI_DEV_UNREG can also be triggered in atomic context, in which calling
Actually, I remember going over the hci_sock_dev_event() calling
codepath (in reverse) quite exhaustively, and did not find a
legitimate reason why anybody would want it to be atomic. hci_notify()
has six call sites, and all are sleep-capable, IMO. In the case of
hci_unregister_dev(), for example, what's happening is as follows:
__device_release_driver (can sleep)
usb_unbind_interface (-"-)
hci_usb_disconnect [hci_usb] (can sleep *[1])
hci_unregister_dev [bluetooth] (-"-)
hci_notify [bluetooth] (-"-)
atomic_notifier_call_chain (contains RCU read section)
notifier_call_chain (therefore, CANNOT SLEEP [2])
hci_sock_dev_event [bluetooth] (-"-)
lock_sock_nested (MIGHT SLEEP *BUG*)
__might_sleep
[1] This is the first problem point. However, I didn't find any reason
why this particular driver's .disconnect() couldn't sleep. In fact, a
comment in include/linux/usb.h:811 says:
"The probe() and disconnect() methods are called in a context where
they can sleep, but they should avoid abusing the privilege. Most
work to connect to a device should be done when the device is opened,
and undone at the last close. The disconnect code needs to address
concurrency issues with respect to open() and close() methods, as
well as forcing all pending I/O requests to complete (by unlinking
them as necessary, and blocking until the unlinks complete)."
I'm assuming the comment is not obsolete, of course, but although the
first sentence says .disconnect() shouldn't abuse the privilege to
sleep, the last sentence makes it quite evident that we are _allowed_
to do so anyway, and that is how things are (with the hci_usb driver,
at least, I didn't check the .remove() or .disconnect() functions of other
USB drivers, however).
[2] This is a bogus (and unnecessary) can-sleep-to-cannot-sleep
transition point, IMO. I had copied Alan Stern in another thread a few
days back, and he wasn't sure why hci_notifier was classified as an
atomic notifier chain (when that classification happened with the new
notifier chains API). I had submitted a patch that merely changed 4
lines in net/bluetooth/hci_core.c to convert hci_notifier to a blocking
notifier chain, but couldn't test as I own no bluetooth hardware myself.
So do we ever really _need_ hci_sock_dev_event() to run in atomic
context at all?
> lock_sock() is not safe as it could sleep.
>
> This patch moves the detaching of sockets from hci_device into workqueue,
> so that lock_sock() can be used safely. This requires movement of
I did a workqueue conversion myself, but ran into the following problem:
In the scheduled work function we have:
> + read_lock(&hci_sk_list.lock);
> + sk_for_each(sk, node, &hci_sk_list.head) {
> + lock_sock(sk);
This would still be illegal, we can't sleep while holding an rwlock
(hci_sk_list.lock above). Converting hci_sk_list.lock to an rwsem
is _even_ more problematic, because hci_send_to_sock()
just *cannot* sleep.
> deallocation of hci_dev - deallocating device just after
> hci_unregister_dev() would be too soon, as it could happen before the
> workqueue has been run.
Suggest a better solution for this: just introduce a flush_scheduled_work()
after hci_unregister_dev() but before hci_free_dev() in all those places.
Less disruptive that way.
So this is quite an interesting problem indeed, but I can't help wondering
that this must be faced elsewhere in the kernel (other users of lock_sock)
too. CC'ing netdev@, for any ideas.
(later)
I Googled a bit to see if this problem was faced elsewhere in the kernel
too. Saw the following commit by Ingo Molnar
(9883a13c72dbf8c518814b6091019643cdb34429):
- lock_sock(sock->sk);
+ local_bh_disable();
+ bh_lock_sock_nested(sock->sk);
rc = selinux_netlbl_socket_setsid(sock, sksec->sid);
- release_sock(sock->sk);
+ bh_unlock_sock(sock->sk);
+ local_bh_enable();
Is it _really_ *this* simple?
Satyam
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: 2.6.21-rc7: BUG: sleeping function called from invalid context at net/core/sock.c:1523
2007-05-11 13:29 ` Satyam Sharma
(?)
(?)
@ 2007-05-13 9:20 ` Greg KH
-1 siblings, 0 replies; 31+ messages in thread
From: Greg KH @ 2007-05-13 9:20 UTC (permalink / raw)
To: Satyam Sharma
Cc: Jiri Kosina, Marcel Holtmann, Jeremy Fitzhardinge, maxk,
bluez-devel, Cedric Le Goater, Linux Kernel Mailing List, netdev
On Fri, May 11, 2007 at 06:59:31PM +0530, Satyam Sharma wrote:
> [1] This is the first problem point. However, I didn't find any reason
> why this particular driver's .disconnect() couldn't sleep. In fact, a
> comment in include/linux/usb.h:811 says:
>
> "The probe() and disconnect() methods are called in a context where
> they can sleep, but they should avoid abusing the privilege. Most
> work to connect to a device should be done when the device is opened,
> and undone at the last close. The disconnect code needs to address
> concurrency issues with respect to open() and close() methods, as
> well as forcing all pending I/O requests to complete (by unlinking
> them as necessary, and blocking until the unlinks complete)."
>
> I'm assuming the comment is not obsolete, of course, but although the
> first sentence says .disconnect() shouldn't abuse the privilege to
> sleep, the last sentence makes it quite evident that we are _allowed_
> to do so anyway, and that is how things are (with the hci_usb driver,
> at least, I didn't check the .remove() or .disconnect() functions of other
> USB drivers, however).
Yes, this is true, you are running in thread context for .disconnect of
usb drivers, so you can sleep if you need to, but you will block all
other device's disconnect and probe functions while you do. So, it's
good to try to not abuse this if possible.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: 2.6.21-rc7: BUG: sleeping function called from invalid context at net/core/sock.c:1523
2007-05-11 13:29 ` Satyam Sharma
@ 2007-05-16 9:29 ` Jiri Kosina
-1 siblings, 0 replies; 31+ messages in thread
From: Jiri Kosina @ 2007-05-16 9:29 UTC (permalink / raw)
To: Satyam Sharma
Cc: Marcel Holtmann, Greg KH, Jeremy Fitzhardinge, maxk, bluez-devel,
Cedric Le Goater, Linux Kernel Mailing List, netdev
On Fri, 11 May 2007, Satyam Sharma wrote:
> (later)
> I Googled a bit to see if this problem was faced elsewhere in the kernel
> too. Saw the following commit by Ingo Molnar
> (9883a13c72dbf8c518814b6091019643cdb34429):
> - lock_sock(sock->sk);
> + local_bh_disable();
> + bh_lock_sock_nested(sock->sk);
> rc = selinux_netlbl_socket_setsid(sock, sksec->sid);
> - release_sock(sock->sk);
> + bh_unlock_sock(sock->sk);
> + local_bh_enable();
> Is it _really_ *this* simple?
Hi Satyam,
actually this *seems* to be proper solution also for our case, thanks for
pointing this out. I will think about it once again, do some more tests
with this locking scheme, and will let you know.
Thanks,
--
Jiri Kosina
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: 2.6.21-rc7: BUG: sleeping function called from invalid context at net/core/sock.c:1523
@ 2007-05-16 9:29 ` Jiri Kosina
0 siblings, 0 replies; 31+ messages in thread
From: Jiri Kosina @ 2007-05-16 9:29 UTC (permalink / raw)
To: Satyam Sharma
Cc: Jeremy Fitzhardinge, netdev, Marcel Holtmann, Greg KH,
Linux Kernel Mailing List, Cedric Le Goater, bluez-devel, maxk
On Fri, 11 May 2007, Satyam Sharma wrote:
> (later)
> I Googled a bit to see if this problem was faced elsewhere in the kernel
> too. Saw the following commit by Ingo Molnar
> (9883a13c72dbf8c518814b6091019643cdb34429):
> - lock_sock(sock->sk);
> + local_bh_disable();
> + bh_lock_sock_nested(sock->sk);
> rc = selinux_netlbl_socket_setsid(sock, sksec->sid);
> - release_sock(sock->sk);
> + bh_unlock_sock(sock->sk);
> + local_bh_enable();
> Is it _really_ *this* simple?
Hi Satyam,
actually this *seems* to be proper solution also for our case, thanks for
pointing this out. I will think about it once again, do some more tests
with this locking scheme, and will let you know.
Thanks,
--
Jiri Kosina
-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: 2.6.21-rc7: BUG: sleeping function called from invalid context at net/core/sock.c:1523
2007-05-16 9:29 ` Jiri Kosina
(?)
@ 2007-05-16 11:36 ` Satyam Sharma
2007-05-16 11:45 ` Marcel Holtmann
-1 siblings, 1 reply; 31+ messages in thread
From: Satyam Sharma @ 2007-05-16 11:36 UTC (permalink / raw)
To: Jiri Kosina
Cc: Marcel Holtmann, Greg KH, Jeremy Fitzhardinge, maxk, bluez-devel,
Cedric Le Goater, Linux Kernel Mailing List, netdev
Hi Jiri,
On 5/16/07, Jiri Kosina <jikos@jikos.cz> wrote:
> On Fri, 11 May 2007, Satyam Sharma wrote:
> > (later)
> > I Googled a bit to see if this problem was faced elsewhere in the kernel
> > too. Saw the following commit by Ingo Molnar
> > (9883a13c72dbf8c518814b6091019643cdb34429):
> > - lock_sock(sock->sk);
> > + local_bh_disable();
> > + bh_lock_sock_nested(sock->sk);
> > rc = selinux_netlbl_socket_setsid(sock, sksec->sid);
> > - release_sock(sock->sk);
> > + bh_unlock_sock(sock->sk);
> > + local_bh_enable();
> > Is it _really_ *this* simple?
> [...]
> actually this *seems* to be proper solution also for our case, thanks for
> pointing this out. I will think about it once again, do some more tests
> with this locking scheme, and will let you know.
Yes, I can almost confirm that this (open-coding of spin_lock_bh,
effectively) is the proper solution (Rusty's unreliable guide to
kernel-locking needs to be next to every developer's keyboard :-)
I also came across this idiom in other places in the networking code
so it seems to be pretty much the standard way. I wish I owned
bluetooth hardware, could've tested this for you myself.
Thanks,
Satyam
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Bluez-devel] 2.6.21-rc7: BUG: sleeping function called from invalid context at net/core/sock.c:1523
2007-05-16 11:36 ` Satyam Sharma
2007-05-16 11:45 ` Marcel Holtmann
@ 2007-05-16 11:45 ` Marcel Holtmann
0 siblings, 0 replies; 31+ messages in thread
From: Marcel Holtmann @ 2007-05-16 11:45 UTC (permalink / raw)
To: Satyam Sharma
Cc: Jeremy Fitzhardinge, netdev, Greg KH, Linux Kernel Mailing List,
Jiri Kosina, Cedric Le Goater, bluez-devel, maxk
Hi Satayam,
> > > (later)
> > > I Googled a bit to see if this problem was faced elsewhere in the kernel
> > > too. Saw the following commit by Ingo Molnar
> > > (9883a13c72dbf8c518814b6091019643cdb34429):
> > > - lock_sock(sock->sk);
> > > + local_bh_disable();
> > > + bh_lock_sock_nested(sock->sk);
> > > rc = selinux_netlbl_socket_setsid(sock, sksec->sid);
> > > - release_sock(sock->sk);
> > > + bh_unlock_sock(sock->sk);
> > > + local_bh_enable();
> > > Is it _really_ *this* simple?
> > [...]
> > actually this *seems* to be proper solution also for our case, thanks for
> > pointing this out. I will think about it once again, do some more tests
> > with this locking scheme, and will let you know.
>
> Yes, I can almost confirm that this (open-coding of spin_lock_bh,
> effectively) is the proper solution (Rusty's unreliable guide to
> kernel-locking needs to be next to every developer's keyboard :-)
> I also came across this idiom in other places in the networking code
> so it seems to be pretty much the standard way. I wish I owned
> bluetooth hardware, could've tested this for you myself.
does this mean we should revert previous changes to the locking or only
apply this on top of it?
Regards
Marcel
-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: 2.6.21-rc7: BUG: sleeping function called from invalid context at net/core/sock.c:1523
@ 2007-05-16 11:45 ` Marcel Holtmann
0 siblings, 0 replies; 31+ messages in thread
From: Marcel Holtmann @ 2007-05-16 11:45 UTC (permalink / raw)
To: Satyam Sharma
Cc: Jeremy Fitzhardinge, netdev, Greg KH, Linux Kernel Mailing List,
Jiri Kosina, Cedric Le Goater, bluez-devel, maxk
Hi Satayam,
> > > (later)
> > > I Googled a bit to see if this problem was faced elsewhere in the kernel
> > > too. Saw the following commit by Ingo Molnar
> > > (9883a13c72dbf8c518814b6091019643cdb34429):
> > > - lock_sock(sock->sk);
> > > + local_bh_disable();
> > > + bh_lock_sock_nested(sock->sk);
> > > rc = selinux_netlbl_socket_setsid(sock, sksec->sid);
> > > - release_sock(sock->sk);
> > > + bh_unlock_sock(sock->sk);
> > > + local_bh_enable();
> > > Is it _really_ *this* simple?
> > [...]
> > actually this *seems* to be proper solution also for our case, thanks for
> > pointing this out. I will think about it once again, do some more tests
> > with this locking scheme, and will let you know.
>
> Yes, I can almost confirm that this (open-coding of spin_lock_bh,
> effectively) is the proper solution (Rusty's unreliable guide to
> kernel-locking needs to be next to every developer's keyboard :-)
> I also came across this idiom in other places in the networking code
> so it seems to be pretty much the standard way. I wish I owned
> bluetooth hardware, could've tested this for you myself.
does this mean we should revert previous changes to the locking or only
apply this on top of it?
Regards
Marcel
-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: 2.6.21-rc7: BUG: sleeping function called from invalid context at net/core/sock.c:1523
@ 2007-05-16 11:45 ` Marcel Holtmann
0 siblings, 0 replies; 31+ messages in thread
From: Marcel Holtmann @ 2007-05-16 11:45 UTC (permalink / raw)
To: Satyam Sharma
Cc: Jiri Kosina, Greg KH, Jeremy Fitzhardinge, maxk, bluez-devel,
Cedric Le Goater, Linux Kernel Mailing List, netdev
Hi Satayam,
> > > (later)
> > > I Googled a bit to see if this problem was faced elsewhere in the kernel
> > > too. Saw the following commit by Ingo Molnar
> > > (9883a13c72dbf8c518814b6091019643cdb34429):
> > > - lock_sock(sock->sk);
> > > + local_bh_disable();
> > > + bh_lock_sock_nested(sock->sk);
> > > rc = selinux_netlbl_socket_setsid(sock, sksec->sid);
> > > - release_sock(sock->sk);
> > > + bh_unlock_sock(sock->sk);
> > > + local_bh_enable();
> > > Is it _really_ *this* simple?
> > [...]
> > actually this *seems* to be proper solution also for our case, thanks for
> > pointing this out. I will think about it once again, do some more tests
> > with this locking scheme, and will let you know.
>
> Yes, I can almost confirm that this (open-coding of spin_lock_bh,
> effectively) is the proper solution (Rusty's unreliable guide to
> kernel-locking needs to be next to every developer's keyboard :-)
> I also came across this idiom in other places in the networking code
> so it seems to be pretty much the standard way. I wish I owned
> bluetooth hardware, could've tested this for you myself.
does this mean we should revert previous changes to the locking or only
apply this on top of it?
Regards
Marcel
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: 2.6.21-rc7: BUG: sleeping function called from invalid context at net/core/sock.c:1523
2007-05-16 11:45 ` Marcel Holtmann
@ 2007-05-16 11:56 ` Satyam Sharma
-1 siblings, 0 replies; 31+ messages in thread
From: Satyam Sharma @ 2007-05-16 11:56 UTC (permalink / raw)
To: Marcel Holtmann
Cc: Jiri Kosina, Greg KH, Jeremy Fitzhardinge, maxk, bluez-devel,
Cedric Le Goater, Linux Kernel Mailing List, netdev
Hi Marcel,
On 5/16/07, Marcel Holtmann <marcel@holtmann.org> wrote:
> Hi Satayam,
>
> > > > (later)
> > > > I Googled a bit to see if this problem was faced elsewhere in the kernel
> > > > too. Saw the following commit by Ingo Molnar
> > > > (9883a13c72dbf8c518814b6091019643cdb34429):
> > > > - lock_sock(sock->sk);
> > > > + local_bh_disable();
> > > > + bh_lock_sock_nested(sock->sk);
> > > > rc = selinux_netlbl_socket_setsid(sock, sksec->sid);
> > > > - release_sock(sock->sk);
> > > > + bh_unlock_sock(sock->sk);
> > > > + local_bh_enable();
> > > > Is it _really_ *this* simple?
> > > [...]
> > > actually this *seems* to be proper solution also for our case, thanks for
> > > pointing this out. I will think about it once again, do some more tests
> > > with this locking scheme, and will let you know.
> >
> > Yes, I can almost confirm that this (open-coding of spin_lock_bh,
> > effectively) is the proper solution (Rusty's unreliable guide to
> > kernel-locking needs to be next to every developer's keyboard :-)
> > I also came across this idiom in other places in the networking code
> > so it seems to be pretty much the standard way. I wish I owned
> > bluetooth hardware, could've tested this for you myself.
>
> does this mean we should revert previous changes to the locking or only
> apply this on top of it?
I've fixed a simple patch on top of 2.6.22-rc1 below.
Signed-off-by: Satyam Sharma <ssatyam@cse.iitk.ac.in>
diff -ruNp a/net/bluetooth/hci_sock.c b/net/bluetooth/hci_sock.c
--- a/net/bluetooth/hci_sock.c 2007-05-16 17:31:06.000000000 +0530
+++ b/net/bluetooth/hci_sock.c 2007-05-16 17:33:36.000000000 +0530
@@ -665,7 +665,8 @@ static int hci_sock_dev_event(struct not
/* Detach sockets from device */
read_lock(&hci_sk_list.lock);
sk_for_each(sk, node, &hci_sk_list.head) {
- lock_sock(sk);
+ local_bh_disable();
+ bh_lock_sock_nested(sk);
if (hci_pi(sk)->hdev == hdev) {
hci_pi(sk)->hdev = NULL;
sk->sk_err = EPIPE;
@@ -674,6 +675,8 @@ static int hci_sock_dev_event(struct not
hci_dev_put(hdev);
}
+ bh_unlock_sock(sk);
+ local_bh_enable();
release_sock(sk);
}
read_unlock(&hci_sk_list.lock);
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: 2.6.21-rc7: BUG: sleeping function called from invalid context at net/core/sock.c:1523
@ 2007-05-16 11:56 ` Satyam Sharma
0 siblings, 0 replies; 31+ messages in thread
From: Satyam Sharma @ 2007-05-16 11:56 UTC (permalink / raw)
To: Marcel Holtmann
Cc: Jeremy Fitzhardinge, netdev, Greg KH, Linux Kernel Mailing List,
Jiri Kosina, Cedric Le Goater, bluez-devel, maxk
Hi Marcel,
On 5/16/07, Marcel Holtmann <marcel@holtmann.org> wrote:
> Hi Satayam,
>
> > > > (later)
> > > > I Googled a bit to see if this problem was faced elsewhere in the kernel
> > > > too. Saw the following commit by Ingo Molnar
> > > > (9883a13c72dbf8c518814b6091019643cdb34429):
> > > > - lock_sock(sock->sk);
> > > > + local_bh_disable();
> > > > + bh_lock_sock_nested(sock->sk);
> > > > rc = selinux_netlbl_socket_setsid(sock, sksec->sid);
> > > > - release_sock(sock->sk);
> > > > + bh_unlock_sock(sock->sk);
> > > > + local_bh_enable();
> > > > Is it _really_ *this* simple?
> > > [...]
> > > actually this *seems* to be proper solution also for our case, thanks for
> > > pointing this out. I will think about it once again, do some more tests
> > > with this locking scheme, and will let you know.
> >
> > Yes, I can almost confirm that this (open-coding of spin_lock_bh,
> > effectively) is the proper solution (Rusty's unreliable guide to
> > kernel-locking needs to be next to every developer's keyboard :-)
> > I also came across this idiom in other places in the networking code
> > so it seems to be pretty much the standard way. I wish I owned
> > bluetooth hardware, could've tested this for you myself.
>
> does this mean we should revert previous changes to the locking or only
> apply this on top of it?
I've fixed a simple patch on top of 2.6.22-rc1 below.
Signed-off-by: Satyam Sharma <ssatyam@cse.iitk.ac.in>
diff -ruNp a/net/bluetooth/hci_sock.c b/net/bluetooth/hci_sock.c
--- a/net/bluetooth/hci_sock.c 2007-05-16 17:31:06.000000000 +0530
+++ b/net/bluetooth/hci_sock.c 2007-05-16 17:33:36.000000000 +0530
@@ -665,7 +665,8 @@ static int hci_sock_dev_event(struct not
/* Detach sockets from device */
read_lock(&hci_sk_list.lock);
sk_for_each(sk, node, &hci_sk_list.head) {
- lock_sock(sk);
+ local_bh_disable();
+ bh_lock_sock_nested(sk);
if (hci_pi(sk)->hdev == hdev) {
hci_pi(sk)->hdev = NULL;
sk->sk_err = EPIPE;
@@ -674,6 +675,8 @@ static int hci_sock_dev_event(struct not
hci_dev_put(hdev);
}
+ bh_unlock_sock(sk);
+ local_bh_enable();
release_sock(sk);
}
read_unlock(&hci_sk_list.lock);
-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: 2.6.21-rc7: BUG: sleeping function called from invalid context at net/core/sock.c:1523
2007-05-16 11:56 ` Satyam Sharma
@ 2007-05-16 11:59 ` Satyam Sharma
-1 siblings, 0 replies; 31+ messages in thread
From: Satyam Sharma @ 2007-05-16 11:59 UTC (permalink / raw)
To: Marcel Holtmann
Cc: Jiri Kosina, Greg KH, Jeremy Fitzhardinge, maxk, bluez-devel,
Cedric Le Goater, Linux Kernel Mailing List, netdev
On 5/16/07, Satyam Sharma <satyam.sharma@gmail.com> wrote:
> Hi Marcel,
> [...]
> > > > > (later)
> > > > > I Googled a bit to see if this problem was faced elsewhere in the kernel
> > > > > too. Saw the following commit by Ingo Molnar
> > > > > (9883a13c72dbf8c518814b6091019643cdb34429):
> > > > > - lock_sock(sock->sk);
> > > > > + local_bh_disable();
> > > > > + bh_lock_sock_nested(sock->sk);
> > > > > rc = selinux_netlbl_socket_setsid(sock, sksec->sid);
> > > > > - release_sock(sock->sk);
> > > > > + bh_unlock_sock(sock->sk);
> > > > > + local_bh_enable();
> > > > > Is it _really_ *this* simple?
> > > > [...]
> > > > actually this *seems* to be proper solution also for our case, thanks for
> > > > pointing this out. I will think about it once again, do some more tests
> > > > with this locking scheme, and will let you know.
> > >
> > > Yes, I can almost confirm that this (open-coding of spin_lock_bh,
> > > effectively) is the proper solution (Rusty's unreliable guide to
> > > kernel-locking needs to be next to every developer's keyboard :-)
> > > I also came across this idiom in other places in the networking code
> > > so it seems to be pretty much the standard way. I wish I owned
> > > bluetooth hardware, could've tested this for you myself.
> >
> > does this mean we should revert previous changes to the locking or only
> > apply this on top of it?
>
> I've fixed a simple patch on top of 2.6.22-rc1 below.
Eek, please ignore previous one. This one's correct.
Signed-off-by: Satyam Sharma <ssatyam@cse.iitk.ac.in>
diff -ruNp a/net/bluetooth/hci_sock.c b/net/bluetooth/hci_sock.c
--- a/net/bluetooth/hci_sock.c 2007-05-16 17:31:06.000000000 +0530
+++ b/net/bluetooth/hci_sock.c 2007-05-16 17:38:35.000000000 +0530
@@ -665,7 +665,8 @@ static int hci_sock_dev_event(struct not
/* Detach sockets from device */
read_lock(&hci_sk_list.lock);
sk_for_each(sk, node, &hci_sk_list.head) {
- lock_sock(sk);
+ local_bh_disable();
+ bh_lock_sock_nested(sk);
if (hci_pi(sk)->hdev == hdev) {
hci_pi(sk)->hdev = NULL;
sk->sk_err = EPIPE;
@@ -674,7 +675,8 @@ static int hci_sock_dev_event(struct not
hci_dev_put(hdev);
}
- release_sock(sk);
+ bh_unlock_sock(sk);
+ local_bh_enable();
}
read_unlock(&hci_sk_list.lock);
}
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: 2.6.21-rc7: BUG: sleeping function called from invalid context at net/core/sock.c:1523
@ 2007-05-16 11:59 ` Satyam Sharma
0 siblings, 0 replies; 31+ messages in thread
From: Satyam Sharma @ 2007-05-16 11:59 UTC (permalink / raw)
To: Marcel Holtmann
Cc: Jeremy Fitzhardinge, netdev, Greg KH, Linux Kernel Mailing List,
Jiri Kosina, Cedric Le Goater, bluez-devel, maxk
On 5/16/07, Satyam Sharma <satyam.sharma@gmail.com> wrote:
> Hi Marcel,
> [...]
> > > > > (later)
> > > > > I Googled a bit to see if this problem was faced elsewhere in the kernel
> > > > > too. Saw the following commit by Ingo Molnar
> > > > > (9883a13c72dbf8c518814b6091019643cdb34429):
> > > > > - lock_sock(sock->sk);
> > > > > + local_bh_disable();
> > > > > + bh_lock_sock_nested(sock->sk);
> > > > > rc = selinux_netlbl_socket_setsid(sock, sksec->sid);
> > > > > - release_sock(sock->sk);
> > > > > + bh_unlock_sock(sock->sk);
> > > > > + local_bh_enable();
> > > > > Is it _really_ *this* simple?
> > > > [...]
> > > > actually this *seems* to be proper solution also for our case, thanks for
> > > > pointing this out. I will think about it once again, do some more tests
> > > > with this locking scheme, and will let you know.
> > >
> > > Yes, I can almost confirm that this (open-coding of spin_lock_bh,
> > > effectively) is the proper solution (Rusty's unreliable guide to
> > > kernel-locking needs to be next to every developer's keyboard :-)
> > > I also came across this idiom in other places in the networking code
> > > so it seems to be pretty much the standard way. I wish I owned
> > > bluetooth hardware, could've tested this for you myself.
> >
> > does this mean we should revert previous changes to the locking or only
> > apply this on top of it?
>
> I've fixed a simple patch on top of 2.6.22-rc1 below.
Eek, please ignore previous one. This one's correct.
Signed-off-by: Satyam Sharma <ssatyam@cse.iitk.ac.in>
diff -ruNp a/net/bluetooth/hci_sock.c b/net/bluetooth/hci_sock.c
--- a/net/bluetooth/hci_sock.c 2007-05-16 17:31:06.000000000 +0530
+++ b/net/bluetooth/hci_sock.c 2007-05-16 17:38:35.000000000 +0530
@@ -665,7 +665,8 @@ static int hci_sock_dev_event(struct not
/* Detach sockets from device */
read_lock(&hci_sk_list.lock);
sk_for_each(sk, node, &hci_sk_list.head) {
- lock_sock(sk);
+ local_bh_disable();
+ bh_lock_sock_nested(sk);
if (hci_pi(sk)->hdev == hdev) {
hci_pi(sk)->hdev = NULL;
sk->sk_err = EPIPE;
@@ -674,7 +675,8 @@ static int hci_sock_dev_event(struct not
hci_dev_put(hdev);
}
- release_sock(sk);
+ bh_unlock_sock(sk);
+ local_bh_enable();
}
read_unlock(&hci_sk_list.lock);
}
-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [Bluez-devel] 2.6.21-rc7: BUG: sleeping function called from invalid context at net/core/sock.c:1523
2007-05-16 11:59 ` Satyam Sharma
(?)
@ 2007-05-16 12:16 ` Marcel Holtmann
-1 siblings, 0 replies; 31+ messages in thread
From: Marcel Holtmann @ 2007-05-16 12:16 UTC (permalink / raw)
To: Satyam Sharma
Cc: Jeremy Fitzhardinge, netdev, Greg KH, Linux Kernel Mailing List,
Jiri Kosina, Cedric Le Goater, bluez-devel, maxk
Hi Satyam,
> > > > > > (later)
> > > > > > I Googled a bit to see if this problem was faced elsewhere in the kernel
> > > > > > too. Saw the following commit by Ingo Molnar
> > > > > > (9883a13c72dbf8c518814b6091019643cdb34429):
> > > > > > - lock_sock(sock->sk);
> > > > > > + local_bh_disable();
> > > > > > + bh_lock_sock_nested(sock->sk);
> > > > > > rc = selinux_netlbl_socket_setsid(sock, sksec->sid);
> > > > > > - release_sock(sock->sk);
> > > > > > + bh_unlock_sock(sock->sk);
> > > > > > + local_bh_enable();
> > > > > > Is it _really_ *this* simple?
> > > > > [...]
> > > > > actually this *seems* to be proper solution also for our case, thanks for
> > > > > pointing this out. I will think about it once again, do some more tests
> > > > > with this locking scheme, and will let you know.
> > > >
> > > > Yes, I can almost confirm that this (open-coding of spin_lock_bh,
> > > > effectively) is the proper solution (Rusty's unreliable guide to
> > > > kernel-locking needs to be next to every developer's keyboard :-)
> > > > I also came across this idiom in other places in the networking code
> > > > so it seems to be pretty much the standard way. I wish I owned
> > > > bluetooth hardware, could've tested this for you myself.
> > >
> > > does this mean we should revert previous changes to the locking or only
> > > apply this on top of it?
> >
> > I've fixed a simple patch on top of 2.6.22-rc1 below.
>
> Eek, please ignore previous one. This one's correct.
>
> Signed-off-by: Satyam Sharma <ssatyam@cse.iitk.ac.in>
>
> diff -ruNp a/net/bluetooth/hci_sock.c b/net/bluetooth/hci_sock.c
> --- a/net/bluetooth/hci_sock.c 2007-05-16 17:31:06.000000000 +0530
> +++ b/net/bluetooth/hci_sock.c 2007-05-16 17:38:35.000000000 +0530
> @@ -665,7 +665,8 @@ static int hci_sock_dev_event(struct not
> /* Detach sockets from device */
> read_lock(&hci_sk_list.lock);
> sk_for_each(sk, node, &hci_sk_list.head) {
> - lock_sock(sk);
> + local_bh_disable();
> + bh_lock_sock_nested(sk);
> if (hci_pi(sk)->hdev == hdev) {
> hci_pi(sk)->hdev = NULL;
> sk->sk_err = EPIPE;
> @@ -674,7 +675,8 @@ static int hci_sock_dev_event(struct not
>
> hci_dev_put(hdev);
> }
> - release_sock(sk);
> + bh_unlock_sock(sk);
> + local_bh_enable();
> }
> read_unlock(&hci_sk_list.lock);
> }
since Jiri has a good test case for it, I leave it to him for testing.
If he confirms that this fixes the locking issues, then this is
Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
Regards
Marcel
-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: 2.6.21-rc7: BUG: sleeping function called from invalid context at net/core/sock.c:1523
@ 2007-05-16 12:16 ` Marcel Holtmann
0 siblings, 0 replies; 31+ messages in thread
From: Marcel Holtmann @ 2007-05-16 12:16 UTC (permalink / raw)
To: Satyam Sharma
Cc: Jeremy Fitzhardinge, netdev, Greg KH, Linux Kernel Mailing List,
Jiri Kosina, Cedric Le Goater, bluez-devel, maxk
Hi Satyam,
> > > > > > (later)
> > > > > > I Googled a bit to see if this problem was faced elsewhere in the kernel
> > > > > > too. Saw the following commit by Ingo Molnar
> > > > > > (9883a13c72dbf8c518814b6091019643cdb34429):
> > > > > > - lock_sock(sock->sk);
> > > > > > + local_bh_disable();
> > > > > > + bh_lock_sock_nested(sock->sk);
> > > > > > rc = selinux_netlbl_socket_setsid(sock, sksec->sid);
> > > > > > - release_sock(sock->sk);
> > > > > > + bh_unlock_sock(sock->sk);
> > > > > > + local_bh_enable();
> > > > > > Is it _really_ *this* simple?
> > > > > [...]
> > > > > actually this *seems* to be proper solution also for our case, thanks for
> > > > > pointing this out. I will think about it once again, do some more tests
> > > > > with this locking scheme, and will let you know.
> > > >
> > > > Yes, I can almost confirm that this (open-coding of spin_lock_bh,
> > > > effectively) is the proper solution (Rusty's unreliable guide to
> > > > kernel-locking needs to be next to every developer's keyboard :-)
> > > > I also came across this idiom in other places in the networking code
> > > > so it seems to be pretty much the standard way. I wish I owned
> > > > bluetooth hardware, could've tested this for you myself.
> > >
> > > does this mean we should revert previous changes to the locking or only
> > > apply this on top of it?
> >
> > I've fixed a simple patch on top of 2.6.22-rc1 below.
>
> Eek, please ignore previous one. This one's correct.
>
> Signed-off-by: Satyam Sharma <ssatyam@cse.iitk.ac.in>
>
> diff -ruNp a/net/bluetooth/hci_sock.c b/net/bluetooth/hci_sock.c
> --- a/net/bluetooth/hci_sock.c 2007-05-16 17:31:06.000000000 +0530
> +++ b/net/bluetooth/hci_sock.c 2007-05-16 17:38:35.000000000 +0530
> @@ -665,7 +665,8 @@ static int hci_sock_dev_event(struct not
> /* Detach sockets from device */
> read_lock(&hci_sk_list.lock);
> sk_for_each(sk, node, &hci_sk_list.head) {
> - lock_sock(sk);
> + local_bh_disable();
> + bh_lock_sock_nested(sk);
> if (hci_pi(sk)->hdev == hdev) {
> hci_pi(sk)->hdev = NULL;
> sk->sk_err = EPIPE;
> @@ -674,7 +675,8 @@ static int hci_sock_dev_event(struct not
>
> hci_dev_put(hdev);
> }
> - release_sock(sk);
> + bh_unlock_sock(sk);
> + local_bh_enable();
> }
> read_unlock(&hci_sk_list.lock);
> }
since Jiri has a good test case for it, I leave it to him for testing.
If he confirms that this fixes the locking issues, then this is
Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
Regards
Marcel
-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: 2.6.21-rc7: BUG: sleeping function called from invalid context at net/core/sock.c:1523
@ 2007-05-16 12:16 ` Marcel Holtmann
0 siblings, 0 replies; 31+ messages in thread
From: Marcel Holtmann @ 2007-05-16 12:16 UTC (permalink / raw)
To: Satyam Sharma
Cc: Jiri Kosina, Greg KH, Jeremy Fitzhardinge, maxk, bluez-devel,
Cedric Le Goater, Linux Kernel Mailing List, netdev
Hi Satyam,
> > > > > > (later)
> > > > > > I Googled a bit to see if this problem was faced elsewhere in the kernel
> > > > > > too. Saw the following commit by Ingo Molnar
> > > > > > (9883a13c72dbf8c518814b6091019643cdb34429):
> > > > > > - lock_sock(sock->sk);
> > > > > > + local_bh_disable();
> > > > > > + bh_lock_sock_nested(sock->sk);
> > > > > > rc = selinux_netlbl_socket_setsid(sock, sksec->sid);
> > > > > > - release_sock(sock->sk);
> > > > > > + bh_unlock_sock(sock->sk);
> > > > > > + local_bh_enable();
> > > > > > Is it _really_ *this* simple?
> > > > > [...]
> > > > > actually this *seems* to be proper solution also for our case, thanks for
> > > > > pointing this out. I will think about it once again, do some more tests
> > > > > with this locking scheme, and will let you know.
> > > >
> > > > Yes, I can almost confirm that this (open-coding of spin_lock_bh,
> > > > effectively) is the proper solution (Rusty's unreliable guide to
> > > > kernel-locking needs to be next to every developer's keyboard :-)
> > > > I also came across this idiom in other places in the networking code
> > > > so it seems to be pretty much the standard way. I wish I owned
> > > > bluetooth hardware, could've tested this for you myself.
> > >
> > > does this mean we should revert previous changes to the locking or only
> > > apply this on top of it?
> >
> > I've fixed a simple patch on top of 2.6.22-rc1 below.
>
> Eek, please ignore previous one. This one's correct.
>
> Signed-off-by: Satyam Sharma <ssatyam@cse.iitk.ac.in>
>
> diff -ruNp a/net/bluetooth/hci_sock.c b/net/bluetooth/hci_sock.c
> --- a/net/bluetooth/hci_sock.c 2007-05-16 17:31:06.000000000 +0530
> +++ b/net/bluetooth/hci_sock.c 2007-05-16 17:38:35.000000000 +0530
> @@ -665,7 +665,8 @@ static int hci_sock_dev_event(struct not
> /* Detach sockets from device */
> read_lock(&hci_sk_list.lock);
> sk_for_each(sk, node, &hci_sk_list.head) {
> - lock_sock(sk);
> + local_bh_disable();
> + bh_lock_sock_nested(sk);
> if (hci_pi(sk)->hdev == hdev) {
> hci_pi(sk)->hdev = NULL;
> sk->sk_err = EPIPE;
> @@ -674,7 +675,8 @@ static int hci_sock_dev_event(struct not
>
> hci_dev_put(hdev);
> }
> - release_sock(sk);
> + bh_unlock_sock(sk);
> + local_bh_enable();
> }
> read_unlock(&hci_sk_list.lock);
> }
since Jiri has a good test case for it, I leave it to him for testing.
If he confirms that this fixes the locking issues, then this is
Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
Regards
Marcel
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: 2.6.21-rc7: BUG: sleeping function called from invalid context at net/core/sock.c:1523
2007-05-16 12:16 ` Marcel Holtmann
@ 2007-05-16 12:19 ` Jiri Kosina
-1 siblings, 0 replies; 31+ messages in thread
From: Jiri Kosina @ 2007-05-16 12:19 UTC (permalink / raw)
To: Marcel Holtmann
Cc: Satyam Sharma, Greg KH, Jeremy Fitzhardinge, maxk, bluez-devel,
Cedric Le Goater, Linux Kernel Mailing List, netdev
On Wed, 16 May 2007, Marcel Holtmann wrote:
> since Jiri has a good test case for it, I leave it to him for testing.
> If he confirms that this fixes the locking issues, then this is
> Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
I will verify later this evening and will let you know. I am however
pretty convinced now that this is the right fix.
Thanks,
--
Jiri Kosina
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: 2.6.21-rc7: BUG: sleeping function called from invalid context at net/core/sock.c:1523
@ 2007-05-16 12:19 ` Jiri Kosina
0 siblings, 0 replies; 31+ messages in thread
From: Jiri Kosina @ 2007-05-16 12:19 UTC (permalink / raw)
To: Marcel Holtmann
Cc: Jeremy Fitzhardinge, netdev, Greg KH, Linux Kernel Mailing List,
Cedric Le Goater, bluez-devel, maxk, Satyam Sharma
On Wed, 16 May 2007, Marcel Holtmann wrote:
> since Jiri has a good test case for it, I leave it to him for testing.
> If he confirms that this fixes the locking issues, then this is
> Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
I will verify later this evening and will let you know. I am however
pretty convinced now that this is the right fix.
Thanks,
--
Jiri Kosina
-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: 2.6.21-rc7: BUG: sleeping function called from invalid context at net/core/sock.c:1523
2007-05-16 12:19 ` Jiri Kosina
@ 2007-05-16 23:03 ` Jiri Kosina
-1 siblings, 0 replies; 31+ messages in thread
From: Jiri Kosina @ 2007-05-16 23:03 UTC (permalink / raw)
To: Satyam Sharma
Cc: Marcel Holtmann, Greg KH, Jeremy Fitzhardinge, maxk, bluez-devel,
Cedric Le Goater, Linux Kernel Mailing List, netdev
On Wed, 16 May 2007, Jiri Kosina wrote:
> > since Jiri has a good test case for it, I leave it to him for testing.
> > If he confirms that this fixes the locking issues, then this is
> > Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
> I will verify later this evening and will let you know. I am however
> pretty convinced now that this is the right fix.
Satyam,
I have just verified that this locking scheme is indeed correct. So you
can add
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
if you wish to, and submit the patch to Andrew.
Thanks,
--
Jiri Kosina
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: 2.6.21-rc7: BUG: sleeping function called from invalid context at net/core/sock.c:1523
@ 2007-05-16 23:03 ` Jiri Kosina
0 siblings, 0 replies; 31+ messages in thread
From: Jiri Kosina @ 2007-05-16 23:03 UTC (permalink / raw)
To: Satyam Sharma
Cc: Jeremy Fitzhardinge, netdev, Marcel Holtmann, Greg KH,
Linux Kernel Mailing List, Cedric Le Goater, bluez-devel, maxk
On Wed, 16 May 2007, Jiri Kosina wrote:
> > since Jiri has a good test case for it, I leave it to him for testing.
> > If he confirms that this fixes the locking issues, then this is
> > Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
> I will verify later this evening and will let you know. I am however
> pretty convinced now that this is the right fix.
Satyam,
I have just verified that this locking scheme is indeed correct. So you
can add
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
if you wish to, and submit the patch to Andrew.
Thanks,
--
Jiri Kosina
-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: 2.6.21-rc7: BUG: sleeping function called from invalid context at net/core/sock.c:1523
2007-05-16 23:03 ` Jiri Kosina
(?)
@ 2007-05-16 23:16 ` David Miller
2007-05-16 23:20 ` Jiri Kosina
-1 siblings, 1 reply; 31+ messages in thread
From: David Miller @ 2007-05-16 23:16 UTC (permalink / raw)
To: jikos
Cc: satyam.sharma, marcel, gregkh, jeremy, maxk, bluez-devel, clg,
linux-kernel, netdev
From: Jiri Kosina <jikos@jikos.cz>
Date: Thu, 17 May 2007 01:03:55 +0200 (CEST)
> On Wed, 16 May 2007, Jiri Kosina wrote:
>
> > > since Jiri has a good test case for it, I leave it to him for testing.
> > > If he confirms that this fixes the locking issues, then this is
> > > Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
> > I will verify later this evening and will let you know. I am however
> > pretty convinced now that this is the right fix.
>
> Satyam,
>
> I have just verified that this locking scheme is indeed correct. So you
> can add
>
> Signed-off-by: Jiri Kosina <jkosina@suse.cz>
>
> if you wish to, and submit the patch to Andrew.
I guess I don't get sent networking patches any more?
:-)
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: 2.6.21-rc7: BUG: sleeping function called from invalid context at net/core/sock.c:1523
2007-05-16 23:16 ` David Miller
@ 2007-05-16 23:20 ` Jiri Kosina
2007-05-17 6:04 ` Marcel Holtmann
0 siblings, 1 reply; 31+ messages in thread
From: Jiri Kosina @ 2007-05-16 23:20 UTC (permalink / raw)
To: David Miller
Cc: satyam.sharma, Marcel Holtmann, Greg KH, jeremy, maxk,
bluez-devel, clg, linux-kernel, netdev
On Wed, 16 May 2007, David Miller wrote:
> > I have just verified that this locking scheme is indeed correct. So you
> > can add
> >
> > Signed-off-by: Jiri Kosina <jkosina@suse.cz>
> >
> > if you wish to, and submit the patch to Andrew.
> I guess I don't get sent networking patches any more?
> :-)
Well, this is bluetooth-specific, but it seemed to me that Marcel wasn't
going to send pull requests to Linus any time soon, therefore I thought
going through akpm is a thing to do.
Honestly, I really don't care through which tree this goes in, so sorry if
any offence was caused here :)
--
Jiri Kosina
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: 2.6.21-rc7: BUG: sleeping function called from invalid context at net/core/sock.c:1523
2007-05-16 23:20 ` Jiri Kosina
@ 2007-05-17 6:04 ` Marcel Holtmann
0 siblings, 0 replies; 31+ messages in thread
From: Marcel Holtmann @ 2007-05-17 6:04 UTC (permalink / raw)
To: Jiri Kosina
Cc: David Miller, satyam.sharma, Greg KH, jeremy, maxk, bluez-devel,
clg, linux-kernel, netdev
Hi Jiri,
> > > I have just verified that this locking scheme is indeed correct. So you
> > > can add
> > >
> > > Signed-off-by: Jiri Kosina <jkosina@suse.cz>
> > >
> > > if you wish to, and submit the patch to Andrew.
> > I guess I don't get sent networking patches any more?
> > :-)
>
> Well, this is bluetooth-specific, but it seemed to me that Marcel wasn't
> going to send pull requests to Linus any time soon, therefore I thought
> going through akpm is a thing to do.
actually everything net/ related goes to Dave first. No exception. This
includes the Bluetooth subsystem. I even send drivers/bluetooth/ through
Dave before they go to Linus.
> Honestly, I really don't care through which tree this goes in, so sorry if
> any offence was caused here :)
Having these small ones passed through Andrew is only a convenience
since he is really good in picking them up and make sure that they get
merged by Linus.
Regards
Marcel
^ permalink raw reply [flat|nested] 31+ messages in thread