Patrick McHardy a écrit :
> Mariusz Kozlowski wrote:
>>     Recent kernels (i.e. 2.6.30-rc1) will panic while doing rmmod of
>> nf_conntrack_irc.
>>
>> (gdb) l *(nf_conntrack_helper_unregister+0x158)
>> 0x4f8 is in nf_conntrack_helper_unregister
>> (/home/mako/linux/lkt/sources/linux-2.6/include/net/netfilter/nf_conntrack.h:133).
>>
>> 128    };
>> 129   
>> 130    static inline struct nf_conn *
>> 131    nf_ct_tuplehash_to_ctrack(const struct nf_conntrack_tuple_hash
>> *hash)
>> 132    {
>> 133        return container_of(hash, struct nf_conn,
>> 134                    tuplehash[hash->tuple.dst.dir]);
>> 135    }
>> 136   
>> 137    static inline u_int16_t nf_ct_l3num(const struct nf_conn *ct)
>>
>>
>> I bisected it down to
>>
>>     netfilter: nf_conntrack: use SLAB_DESTROY_BY_RCU and get rid of
>> call_rcu()
> 
> Thanks for the report. Does this patch fix it?
> 

Hi Patrick, sorry for the delay, I was in holidays.


I should have used different fields names (from "next", "first", ...) to catch 
this
kind of errors at compile time :(

Something like :

diff --git a/include/linux/list_nulls.h b/include/linux/list_nulls.h
index 93150ec..b66c553 100644
--- a/include/linux/list_nulls.h
+++ b/include/linux/list_nulls.h
@@ -15,14 +15,14 @@
  */
 
 struct hlist_nulls_head {
-       struct hlist_nulls_node *first;
+       struct hlist_nulls_node *nfirst;
 };
 
 struct hlist_nulls_node {
-       struct hlist_nulls_node *next, **pprev;
+       struct hlist_nulls_node *nnext, **npprev;
 };
 #define INIT_HLIST_NULLS_HEAD(ptr, nulls) \
-       ((ptr)->first = (struct hlist_nulls_node *) (1UL | (((long)nulls) << 
1)))
+       ((ptr)->nfirst = (struct hlist_nulls_node *) (1UL | (((long)nulls) << 
1)))
 
 #define hlist_nulls_entry(ptr, type, member) container_of(ptr,type,member)
 /**
@@ -48,21 +48,21 @@ static inline unsigned long get_nulls_value(const struct 
hlist_nulls_node *ptr)
 
 static inline int hlist_nulls_unhashed(const struct hlist_nulls_node *h)
 {
-       return !h->pprev;
+       return !h->npprev;
 }
 
 static inline int hlist_nulls_empty(const struct hlist_nulls_head *h)
 {
-       return is_a_nulls(h->first);
+       return is_a_nulls(h->nfirst);
 }
 
 static inline void __hlist_nulls_del(struct hlist_nulls_node *n)
 {
-       struct hlist_nulls_node *next = n->next;
-       struct hlist_nulls_node **pprev = n->pprev;
+       struct hlist_nulls_node *next = n->nnext;
+       struct hlist_nulls_node **pprev = n->npprev;
        *pprev = next;
        if (!is_a_nulls(next))
-               next->pprev = pprev;
+               next->npprev = pprev;
 }
 
 /**
@@ -74,10 +74,10 @@ static inline void __hlist_nulls_del(struct 
hlist_nulls_node *n)
  *
  */
 #define hlist_nulls_for_each_entry(tpos, pos, head, member)                   \
-       for (pos = (head)->first;                                              \
+       for (pos = (head)->nfirst;                                             \
             (!is_a_nulls(pos)) &&                                             \
                ({ tpos = hlist_nulls_entry(pos, typeof(*tpos), member); 1;}); \
-            pos = pos->next)
+            pos = pos->nnext)
 
 /**
  * hlist_nulls_for_each_entry_from - iterate over a hlist continuing from 
current point
@@ -89,6 +89,6 @@ static inline void __hlist_nulls_del(struct hlist_nulls_node 
*n)
 #define hlist_nulls_for_each_entry_from(tpos, pos, member)     \
        for (; (!is_a_nulls(pos)) &&                            \
                ({ tpos = hlist_nulls_entry(pos, typeof(*tpos), member); 1;}); \
-            pos = pos->next)
+            pos = pos->nnext)
 
 #endif
diff --git a/include/linux/rculist_nulls.h b/include/linux/rculist_nulls.h
index f9ddd03..2378c7c 100644
--- a/include/linux/rculist_nulls.h
+++ b/include/linux/rculist_nulls.h
@@ -33,7 +33,7 @@ static inline void hlist_nulls_del_init_rcu(struct 
hlist_nulls_node *n)
 {
        if (!hlist_nulls_unhashed(n)) {
                __hlist_nulls_del(n);
-               n->pprev = NULL;
+               n->npprev = NULL;
        }
 }
 
@@ -59,7 +59,7 @@ static inline void hlist_nulls_del_init_rcu(struct 
hlist_nulls_node *n)
 static inline void hlist_nulls_del_rcu(struct hlist_nulls_node *n)
 {
        __hlist_nulls_del(n);
-       n->pprev = LIST_POISON2;
+       n->npprev = LIST_POISON2;
 }
 
 /**
@@ -84,13 +84,13 @@ static inline void hlist_nulls_del_rcu(struct 
hlist_nulls_node *n)
 static inline void hlist_nulls_add_head_rcu(struct hlist_nulls_node *n,
                                        struct hlist_nulls_head *h)
 {
-       struct hlist_nulls_node *first = h->first;
+       struct hlist_nulls_node *first = h->nfirst;
 
-       n->next = first;
-       n->pprev = &h->first;
-       rcu_assign_pointer(h->first, n);
+       n->nnext = first;
+       n->npprev = &h->nfirst;
+       rcu_assign_pointer(h->nfirst, n);
        if (!is_a_nulls(first))
-               first->pprev = &n->next;
+               first->npprev = &n->nnext;
 }
 /**
  * hlist_nulls_for_each_entry_rcu - iterate over rcu list of given type
@@ -101,10 +101,10 @@ static inline void hlist_nulls_add_head_rcu(struct 
hlist_nulls_node *n,
  *
  */
 #define hlist_nulls_for_each_entry_rcu(tpos, pos, head, member) \
-       for (pos = rcu_dereference((head)->first);                       \
+       for (pos = rcu_dereference((head)->nfirst);                      \
                (!is_a_nulls(pos)) &&                   \
                ({ tpos = hlist_nulls_entry(pos, typeof(*tpos), member); 1; }); 
\
-               pos = rcu_dereference(pos->next))
+               pos = rcu_dereference(pos->nnext))
 
 #endif
 #endif
diff --git a/include/net/sock.h b/include/net/sock.h
index 4bb1ff9..5d94186 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -309,7 +309,7 @@ static inline struct sock *sk_head(const struct hlist_head 
*head)
 
 static inline struct sock *__sk_nulls_head(const struct hlist_nulls_head *head)
 {
-       return hlist_nulls_entry(head->first, struct sock, sk_nulls_node);
+       return hlist_nulls_entry(head->nfirst, struct sock, sk_nulls_node);
 }
 
 static inline struct sock *sk_nulls_head(const struct hlist_nulls_head *head)
@@ -325,8 +325,8 @@ static inline struct sock *sk_next(const struct sock *sk)
 
 static inline struct sock *sk_nulls_next(const struct sock *sk)
 {
-       return (!is_a_nulls(sk->sk_nulls_node.next)) ?
-               hlist_nulls_entry(sk->sk_nulls_node.next,
+       return (!is_a_nulls(sk->sk_nulls_node.nnext)) ?
+               hlist_nulls_entry(sk->sk_nulls_node.nnext,
                                  struct sock, sk_nulls_node) :
                NULL;
 }
@@ -348,7 +348,7 @@ static __inline__ void sk_node_init(struct hlist_node *node)
 
 static __inline__ void sk_nulls_node_init(struct hlist_nulls_node *node)
 {
-       node->pprev = NULL;
+       node->npprev = NULL;
 }
 
 static __inline__ void __sk_del_node(struct sock *sk)
diff --git a/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4_compat.c 
b/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4_compat.c
index 8668a3d..0bb6059 100644
--- a/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4_compat.c
+++ b/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4_compat.c
@@ -34,7 +34,7 @@ static struct hlist_nulls_node *ct_get_first(struct seq_file 
*seq)
        for (st->bucket = 0;
             st->bucket < nf_conntrack_htable_size;
             st->bucket++) {
-               n = rcu_dereference(net->ct.hash[st->bucket].first);
+               n = rcu_dereference(net->ct.hash[st->bucket].nfirst);
                if (!is_a_nulls(n))
                        return n;
        }
@@ -47,13 +47,13 @@ static struct hlist_nulls_node *ct_get_next(struct seq_file 
*seq,
        struct net *net = seq_file_net(seq);
        struct ct_iter_state *st = seq->private;
 
-       head = rcu_dereference(head->next);
+       head = rcu_dereference(head->nnext);
        while (is_a_nulls(head)) {
                if (likely(get_nulls_value(head) == st->bucket)) {
                        if (++st->bucket >= nf_conntrack_htable_size)
                                return NULL;
                }
-               head = rcu_dereference(net->ct.hash[st->bucket].first);
+               head = rcu_dereference(net->ct.hash[st->bucket].nfirst);
        }
        return head;
 }
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index 5d427f8..1219f2d 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -1851,13 +1851,13 @@ EXPORT_SYMBOL(tcp_v4_destroy_sock);
 static inline struct inet_timewait_sock *tw_head(struct hlist_nulls_head *head)
 {
        return hlist_nulls_empty(head) ? NULL :
-               list_entry(head->first, struct inet_timewait_sock, tw_node);
+               list_entry(head->nfirst, struct inet_timewait_sock, tw_node);
 }
 
 static inline struct inet_timewait_sock *tw_next(struct inet_timewait_sock *tw)
 {
-       return !is_a_nulls(tw->tw_node.next) ?
-               hlist_nulls_entry(tw->tw_node.next, typeof(*tw), tw_node) : 
NULL;
+       return !is_a_nulls(tw->tw_node.nnext) ?
+               hlist_nulls_entry(tw->tw_node.nnext, typeof(*tw), tw_node) : 
NULL;
 }
 
 static void *listening_get_next(struct seq_file *seq, void *cur)
diff --git a/net/netfilter/nf_conntrack_core.c 
b/net/netfilter/nf_conntrack_core.c
index 8020db6..56f9dc3 100644
--- a/net/netfilter/nf_conntrack_core.c
+++ b/net/netfilter/nf_conntrack_core.c
@@ -1140,7 +1140,7 @@ int nf_conntrack_set_hashsize(const char *val, struct 
kernel_param *kp)
        spin_lock_bh(&nf_conntrack_lock);
        for (i = 0; i < nf_conntrack_htable_size; i++) {
                while (!hlist_nulls_empty(&init_net.ct.hash[i])) {
-                       h = hlist_nulls_entry(init_net.ct.hash[i].first,
+                       h = hlist_nulls_entry(init_net.ct.hash[i].nfirst,
                                        struct nf_conntrack_tuple_hash, hnnode);
                        hlist_nulls_del_rcu(&h->hnnode);
                        bucket = __hash_conntrack(&h->tuple, hashsize, rnd);
diff --git a/net/netfilter/nf_conntrack_helper.c 
b/net/netfilter/nf_conntrack_helper.c
index 30b8e90..0fa5a42 100644
--- a/net/netfilter/nf_conntrack_helper.c
+++ b/net/netfilter/nf_conntrack_helper.c
@@ -176,7 +176,7 @@ static void __nf_conntrack_helper_unregister(struct 
nf_conntrack_helper *me,
        }
 
        /* Get rid of expecteds, set helpers to NULL. */
-       hlist_for_each_entry(h, nn, &net->ct.unconfirmed, hnnode)
+       hlist_nulls_for_each_entry(h, nn, &net->ct.unconfirmed, hnnode)
                unhelp(h, me);
        for (i = 0; i < nf_conntrack_htable_size; i++) {
                hlist_nulls_for_each_entry(h, nn, &net->ct.hash[i], hnnode)
diff --git a/net/netfilter/nf_conntrack_standalone.c 
b/net/netfilter/nf_conntrack_standalone.c
index 1935153..e6881db 100644
--- a/net/netfilter/nf_conntrack_standalone.c
+++ b/net/netfilter/nf_conntrack_standalone.c
@@ -53,7 +53,7 @@ static struct hlist_nulls_node *ct_get_first(struct seq_file 
*seq)
        for (st->bucket = 0;
             st->bucket < nf_conntrack_htable_size;
             st->bucket++) {
-               n = rcu_dereference(net->ct.hash[st->bucket].first);
+               n = rcu_dereference(net->ct.hash[st->bucket].nfirst);
                if (!is_a_nulls(n))
                        return n;
        }
@@ -66,13 +66,13 @@ static struct hlist_nulls_node *ct_get_next(struct seq_file 
*seq,
        struct net *net = seq_file_net(seq);
        struct ct_iter_state *st = seq->private;
 
-       head = rcu_dereference(head->next);
+       head = rcu_dereference(head->nnext);
        while (is_a_nulls(head)) {
                if (likely(get_nulls_value(head) == st->bucket)) {
                        if (++st->bucket >= nf_conntrack_htable_size)
                                return NULL;
                }
-               head = rcu_dereference(net->ct.hash[st->bucket].first);
+               head = rcu_dereference(net->ct.hash[st->bucket].nfirst);
        }
        return head;
 }


--
To unsubscribe from this list: send the line "unsubscribe kernel-testers" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to