Thread (7 messages) 7 messages, 2 authors, 2012-01-21

Re: [PATCH] Bluetooth: silence lockdep warning

From: Marcel Holtmann <marcel@holtmann.org>
Date: 2012-01-21 19:28:10

Hi Octavian,
quoted
quoted
quoted
quoted
+void bt_sock_reclassify_lock(struct sock *sk, int proto)
 {
-     struct sock *sk = sock->sk;
-
      if (!sk)
              return;
Why are we keeping the !sk check here if we already hand in the sk. It
is most likely checked by the caller already.
In rfcomm_sock_create (called from bt_sock_create) sock->sk can be set
to NULL so I think we should keep the check.
it has been too long since I looked at this part of the code. You need
to walk me through it why this is still true.
Hi Marcel,

In bt_sock_create we have:

        if (bt_proto[proto] && try_module_get(bt_proto[proto]->owner)) {
        	err = bt_proto[proto]->create(net, sock, proto, kern);
                bt_sock_reclassify_lock(sock->sk, proto);

and create can be rfcomm_sock_create where we have

        sk = rfcomm_sock_alloc(net, sock, protocol, GFP_ATOMIC);
        if (!sk)
                return -ENOMEM;

So after calling ->create() sock->sk can be NULL and thus we can call
bt_sock_reclassify with a NULL parameter.
so instead of keep checking sock->sk multiple times in different places,
why not actually check the error from bt_proto[]->create() and skip the
reclassify_lock call.

And if we wanna be really paranoid, then lets add a BUG_ON for sk so we
get some nice backtraces if we have a wrongful caller.

Regards

Marcel

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help