lkml.org 
[lkml]   [2008]   [Sep]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 1/1] Use RCU for the UDP hash lock
    On 24-09-2008 19:28, Corey Minyard wrote:
    ...
    > From: Corey Minyard <cminyard@mvista.com>
    >
    > Convert access to the udp_hash table to use RCU.
    >
    > Signed-off-by: Corey Minyard <cminyard@mvista.com>
    > ---
    ...
    > diff --git a/include/net/sock.h b/include/net/sock.h
    > index 06c5259..ada44ad 100644
    > --- a/include/net/sock.h
    > +++ b/include/net/sock.h
    > @@ -42,6 +42,7 @@
    >
    > #include <linux/kernel.h>
    > #include <linux/list.h>
    > +#include <linux/rculist.h>
    > #include <linux/timer.h>
    > #include <linux/cache.h>
    > #include <linux/module.h>
    > @@ -361,6 +362,27 @@ static __inline__ int sk_del_node_init(struct sock *sk)
    > return rc;
    > }
    >
    > +static inline int __sk_del_node_rcu(struct sock *sk)
    > +{
    > + if (sk_hashed(sk)) {
    > + hlist_del_rcu(&sk->sk_node);
    > + return 1;
    > + }
    > + return 0;
    > +}
    > +
    > +static inline int sk_del_node_rcu(struct sock *sk)
    > +{
    > + int rc = __sk_del_node_rcu(sk);

    Why sk_node_init() part (or hlist_del_init_rcu) isn't used?

    > +
    > + if (rc) {
    > + /* paranoid for a while -acme */
    > + WARN_ON(atomic_read(&sk->sk_refcnt) == 1);
    > + __sock_put(sk);
    > + }
    > + return rc;
    > +}
    > +
    ...
    > diff --git a/include/net/udp.h b/include/net/udp.h
    > index addcdc6..04181f8 100644
    > --- a/include/net/udp.h
    > +++ b/include/net/udp.h
    > @@ -51,7 +51,7 @@ struct udp_skb_cb {
    > #define UDP_SKB_CB(__skb) ((struct udp_skb_cb *)((__skb)->cb))
    >
    > extern struct hlist_head udp_hash[UDP_HTABLE_SIZE];
    > -extern rwlock_t udp_hash_lock;
    > +extern spinlock_t udp_hash_wlock;
    >
    >
    > /* Note: this must match 'valbool' in sock_setsockopt */
    > @@ -112,12 +112,13 @@ static inline void udp_lib_hash(struct sock *sk)
    >
    > static inline void udp_lib_unhash(struct sock *sk)
    > {
    > - write_lock_bh(&udp_hash_lock);
    > - if (sk_del_node_init(sk)) {
    > + spin_lock_bh(&udp_hash_wlock);
    > + if (sk_del_node_rcu(sk)) {
    > inet_sk(sk)->num = 0;
    > sock_prot_inuse_add(sock_net(sk), sk->sk_prot, -1);
    > }
    > - write_unlock_bh(&udp_hash_lock);
    > + spin_unlock_bh(&udp_hash_wlock);
    > + synchronize_sched();
    > }
    >
    > static inline void udp_lib_close(struct sock *sk, long timeout)
    > diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
    > index 57e26fa..3aa04da 100644
    > --- a/net/ipv4/udp.c
    > +++ b/net/ipv4/udp.c
    ...
    > @@ -1094,7 +1103,7 @@ static int __udp4_lib_mcast_deliver(struct net *net, struct sk_buff *skb,
    > struct sock *sk;
    > int dif;
    >
    > - read_lock(&udp_hash_lock);
    > + rcu_read_lock();
    > sk = sk_head(&udptable[udp_hashfn(net, ntohs(uh->dest))]);

    Probably sk_head_rcu() is needed too.

    > dif = skb->dev->ifindex;
    > sk = udp_v4_mcast_next(sk, uh->dest, daddr, uh->source, saddr, dif);
    > @@ -1120,7 +1129,7 @@ static int __udp4_lib_mcast_deliver(struct net *net, struct sk_buff *skb,
    > } while (sknext);
    > } else
    > kfree_skb(skb);
    > - read_unlock(&udp_hash_lock);
    > + rcu_read_unlock();
    > return 0;
    > }
    ...

    Aren't other functions like sk_next() or sk_unhashed() used on the
    read side and need _rcu versions?

    Jarek P.


    \
     
     \ /
      Last update: 2008-09-25 10:47    [W:0.026 / U:1.396 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site