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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
