[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1407217807.3178.70.camel@edumazet-glaptop2.roam.corp.google.com>
Date: Tue, 05 Aug 2014 07:50:07 +0200
From: Eric Dumazet <eric.dumazet@...il.com>
To: David Miller <davem@...emloft.net>, Thomas Graf <tgraf@...g.ch>
Cc: sasha.levin@...cle.com, tgraf@...g.ch, netdev@...r.kernel.org,
linux-kernel@...r.kernel.org, kaber@...sh.net,
paulmck@...ux.vnet.ibm.com, josh@...htriplett.org,
challa@...ronetworks.com, walpole@...pdx.edu, dev@...nvswitch.org,
netfilter-devel@...r.kernel.org, nikolay@...hat.com
Subject: Re: [PATCH net-next 2/3] netlink: Convert netlink_lookup() to use
RCU protected hash table
On Mon, 2014-08-04 at 19:58 -0700, David Miller wrote:
> From: Sasha Levin <sasha.levin@...cle.com>
> Date: Mon, 04 Aug 2014 22:10:19 -0400
>
> > On 08/02/2014 05:47 AM, Thomas Graf wrote:
> >> static void *netlink_seq_start(struct seq_file *seq, loff_t *pos)
> >> - __acquires(nl_table_lock)
> >> {
> >> - read_lock(&nl_table_lock);
> >> + rcu_read_lock();
> >> return *pos ? netlink_seq_socket_idx(seq, *pos - 1) : SEQ_START_TOKEN;
> >> }
> >
> > I'm not sure how you expect this code to work. You're replacing a local lock
> > with a RCU critical section. Imagine you're doing spin_lock() and just going
> > back to userspace.
> >
> > It's quite easy to trigger this issue:
>
> I think he expected the end of the seq sequence to drop the RCU lock,
> via netlink_seq_stop().
> --
Yes, two places use rht_dereference() instead of rht_dereference_rcu()
[PATCH net-next] netlink: fix lockdep splats
With netlink_lookup() conversion to RCU, we need to use appropriate
rcu dereference in netlink_seq_socket_idx() & netlink_seq_next()
Reported-by: Sasha Levin <sasha.levin@...cle.com>
Signed-off-by: Eric Dumazet <edumazet@...gle.com>
Cc:
Fixes: e341694e3eb57fc ("netlink: Convert netlink_lookup() to use RCU protected hash table")
---
diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
index 0b89ca51a3af..479a344563d8 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -2899,7 +2899,7 @@ static struct sock *netlink_seq_socket_idx(struct seq_file *seq, loff_t pos)
for (i = 0; i < MAX_LINKS; i++) {
struct rhashtable *ht = &nl_table[i].hash;
- const struct bucket_table *tbl = rht_dereference(ht->tbl, ht);
+ const struct bucket_table *tbl = rht_dereference_rcu(ht->tbl, ht);
for (j = 0; j < tbl->size; j++) {
rht_for_each_entry_rcu(nlk, tbl->buckets[j], node) {
@@ -2950,7 +2950,7 @@ static void *netlink_seq_next(struct seq_file *seq, void *v, loff_t *pos)
do {
struct rhashtable *ht = &nl_table[i].hash;
- const struct bucket_table *tbl = rht_dereference(ht->tbl, ht);
+ const struct bucket_table *tbl = rht_dereference_rcu(ht->tbl, ht);
for (; j < tbl->size; j++) {
rht_for_each_entry_rcu(nlk, tbl->buckets[j], node) {
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists