On Fri, Jul 28, 2006 at 10:12:52PM +0900, Masatake YAMATO wrote: > Hi, > > On Fri, Jul 28, 2006 at 06:17:56PM +0900, Masatake YAMATO wrote: > > > > I think that the bluetooth-guard-bt_proto-with-rwlock.patch introduced the following > > > > BUG: > > > > [ 43.232000] BUG: sleeping function called from invalid context at mm/slab.c:2903 > > > > [ 43.232000] in_atomic():1, irqs_disabled():0 > > > > [ 43.232000] [] show_trace_log_lvl+0x197/0x1ba > > > > [ 43.232000] [] show_trace+0x27/0x29 > > > > [ 43.232000] [] dump_stack+0x26/0x28 > > > > [ 43.232000] [] __might_sleep+0xa2/0xaa > > > > [ 43.232000] [] __kmalloc+0x9c/0xb3 > > > > [ 43.232000] [] sk_alloc+0x1bc/0x1de > > > > [ 43.232000] [] hci_sock_create+0x42/0x8a > > > > [ 43.236000] [] bt_sock_create+0xb5/0x154 > > > > [ 43.236000] [] __sock_create+0x131/0x356 > > > > [ 43.236000] [] sock_create+0x2e/0x30 > > > > [ 43.236000] [] sys_socket+0x27/0x53 > > > > [ 43.240000] [] sys_socketcall+0xa9/0x277 > > > > [ 43.240000] [] sysenter_past_esp+0x56/0x8d > > > > [ 43.240000] [] 0xb7f38410 > > > > > > > > > > > > This patch makes sk_alloc GFP_ATOMIC, because we are holding the bt_proto_rwlock, for > > > > the following functions: > > > > - bnep_sock_create > > > > - cmtp_sock_create > > > > - hci_sock_create > > > > - hidp_sock_create > > > > - l2cap_sock_create > > > > - rfcomm_sock_create > > > > - sco_sock_create > > > > > > There is very similar code in i net/socket.c(I guess some part of > > > bluetooth/af_bluetooth.c is derived from net/socket.c): > > > > > [... skip net/socket.c code ...] > > > > > > So there are two ways to avoid the bug: > > > 1. As proposed by Frederik, use sk_alloc with GFP_ATOMIC or > > > 2. use net_family_{read|writ}_{lock|unlock} in af_bluetooth.c. > > > > > I'd say that using a net_family_* like function set is much better, > > if only in terms of preemptibility. > > > > I'll write an implementation that allows to use the same code > > in socket.c and in af_bluetooth.c > > > I found one more similar code set at net/dccp/ccid.c for the same purpose: OK, thanks, I modified it too. The following set of patches adds a struct lw_rwlock (for lightweight rwlock) which contains a spin_lock_t and an atomic_t. It is defined in include/linux/lw_rwlock.h. This lw_rwlock aims at protecting structures that are modified very rarely and quickly. This assumptions allow the reader to merely increment the atomic_t, allowing it to sleep why the lw_rwlock is help. There were already two users of this kind of API: net/socket.c to protect 'net_families' and net/dccp/ccid.c to protect 'ccids'. As suggested by Masatake YAMATO, this looked like good way to protect 'bt_proto' in net/bluetooth/af_bluetooth.c as well. This patchset aims at having only one implementation in the kernel. Signed-off-by: Frederik Deweerdt include/linux/lw_rwlock.h | 71 +++++++++++++++++++++++++++++++++++++++++++ net/bluetooth/af_bluetooth.c | 15 ++++----- net/dccp/ccid.c | 63 +++++++------------------------------- net/socket.c | 58 ++++------------------------------- 4 files changed, 100 insertions(+), 107 deletions(-)