lkml.org 
[lkml]   [2006]   [Jun]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
Subject[patch] undo AF_UNIX _bh locking changes and split lock-type instead

* Herbert Xu <herbert@gondor.apana.org.au> wrote:

> > Maybe it's enough to introduce a separate key for AF_UNIX alone (and
> > still having all other protocols share the locking rules for
> > sk_receive_queue.lock) , by reinitializing its spinlock after
> > sock_init_data()?
>
> This could work. AF_UNIX is probably the only family that does not
> interact with hardware.

ok, great. The patch below does the trick on my box.

regarding your point wrt. path of integration - it is pretty much the
only practical way to do this centrally as part of the lock validator
patches, but to collect ACKs from subsystem maintainers in the process.
So if you like it i'd like to have your ACK but this patch depends on
the other lock validator patches (and only makes sense together with
them), so they should temporarily stay in the lock validator queue.
Hopefully this wont be a state that lasts too long and once the
validator is upstream, all patches of course go via the subsystem
submission rules.

(the #ifdef LOCKDEP should probably be converted to some sort of
lockdep_split_lock_key(&sk->sk_receive_queue.lock) op - i'll do that
later)

Ingo

------
Subject: undo AF_UNIX _bh locking changes and split lock-type instead
From: Ingo Molnar <mingo@elte.hu>
this cleans up lock-validator-special-locking-af_unix.patch: instead
of adding _bh locking to AF_UNIX, this patch splits their
sk_receive_queue.lock type from the other networking skb-queue locks.

Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
net/unix/af_unix.c | 17 +++++++++++++----
net/unix/garbage.c | 8 ++++----
2 files changed, 17 insertions(+), 8 deletions(-)
Index: linux/net/unix/af_unix.c
===================================================================
--- linux.orig/net/unix/af_unix.c
+++ linux/net/unix/af_unix.c
@@ -557,6 +557,15 @@ static struct sock * unix_create1(struct
atomic_inc(&unix_nr_socks);

sock_init_data(sock,sk);
+#ifdef CONFIG_LOCKDEP
+ /*
+ * AF_UNIX sockets do not interact with hardware, hence they
+ * dont trigger interrupts - so it's safe for them to have
+ * bh-unsafe locking for their sk_receive_queue.lock. Split off
+ * this special lock-type by reinitializing the spinlock.
+ */
+ spin_lock_init(&sk->sk_receive_queue.lock);
+#endif

sk->sk_write_space = unix_write_space;
sk->sk_max_ack_backlog = sysctl_unix_max_dgram_qlen;
@@ -1073,12 +1082,12 @@ restart:
unix_state_wunlock(sk);

/* take ten and and send info to listening sock */
- spin_lock_bh(&other->sk_receive_queue.lock);
+ spin_lock(&other->sk_receive_queue.lock);
__skb_queue_tail(&other->sk_receive_queue, skb);
/* Undo artificially decreased inflight after embrion
* is installed to listening socket. */
atomic_inc(&newu->inflight);
- spin_unlock_bh(&other->sk_receive_queue.lock);
+ spin_unlock(&other->sk_receive_queue.lock);
unix_state_runlock(other);
other->sk_data_ready(other, 0);
sock_put(other);
@@ -1843,7 +1852,7 @@ static int unix_ioctl(struct socket *soc
break;
}

- spin_lock_bh(&sk->sk_receive_queue.lock);
+ spin_lock(&sk->sk_receive_queue.lock);
if (sk->sk_type == SOCK_STREAM ||
sk->sk_type == SOCK_SEQPACKET) {
skb_queue_walk(&sk->sk_receive_queue, skb)
@@ -1853,7 +1862,7 @@ static int unix_ioctl(struct socket *soc
if (skb)
amount=skb->len;
}
- spin_unlock_bh(&sk->sk_receive_queue.lock);
+ spin_unlock(&sk->sk_receive_queue.lock);
err = put_user(amount, (int __user *)arg);
break;
}
Index: linux/net/unix/garbage.c
===================================================================
--- linux.orig/net/unix/garbage.c
+++ linux/net/unix/garbage.c
@@ -235,7 +235,7 @@ void unix_gc(void)
struct sock *x = pop_stack();
struct sock *sk;

- spin_lock_bh(&x->sk_receive_queue.lock);
+ spin_lock(&x->sk_receive_queue.lock);
skb = skb_peek(&x->sk_receive_queue);

/*
@@ -270,7 +270,7 @@ void unix_gc(void)
maybe_unmark_and_push(skb->sk);
skb=skb->next;
}
- spin_unlock_bh(&x->sk_receive_queue.lock);
+ spin_unlock(&x->sk_receive_queue.lock);
sock_put(x);
}

@@ -283,7 +283,7 @@ void unix_gc(void)
if (u->gc_tree == GC_ORPHAN) {
struct sk_buff *nextsk;

- spin_lock_bh(&s->sk_receive_queue.lock);
+ spin_lock(&s->sk_receive_queue.lock);
skb = skb_peek(&s->sk_receive_queue);
while (skb &&
skb != (struct sk_buff *)&s->sk_receive_queue) {
@@ -298,7 +298,7 @@ void unix_gc(void)
}
skb = nextsk;
}
- spin_unlock_bh(&s->sk_receive_queue.lock);
+ spin_unlock(&s->sk_receive_queue.lock);
}
u->gc_tree = GC_ORPHAN;
}
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
\
 
 \ /
  Last update: 2006-06-12 09:00    [from the cache]
©2003-2014 Jasper Spaans. Advertise on this site