| Message ID | 20190601222738.6856-3-joel@joelfernandes.org (mailing list archive) |
|---|---|
| State | RFC, archived |
| Headers | show |
| Series | Harden list_for_each_entry_rcu() and family | expand |
On Sat 2019-06-01 18:27:34, Joel Fernandes (Google) wrote: > Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org> This really needs to be merged to previous patch, you can't break compilation in middle of series... Or probably you need hlist_for_each_entry_rcu_lockdep() macro with additional argument, and switch users to it. Pavel > net/ipv4/fib_frontend.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/net/ipv4/fib_frontend.c b/net/ipv4/fib_frontend.c > index b298255f6fdb..ef7c9f8e8682 100644 > --- a/net/ipv4/fib_frontend.c > +++ b/net/ipv4/fib_frontend.c > @@ -127,7 +127,8 @@ struct fib_table *fib_get_table(struct net *net, u32 id) > h = id & (FIB_TABLE_HASHSZ - 1); > > head = &net->ipv4.fib_table_hash[h]; > - hlist_for_each_entry_rcu(tb, head, tb_hlist) { > + hlist_for_each_entry_rcu(tb, head, tb_hlist, > + lockdep_rtnl_is_held()) { > if (tb->tb_id == id) > return tb; > }
On Sun, Jun 2, 2019 at 3:00 AM Pavel Machek <pavel@denx.de> wrote: > > On Sat 2019-06-01 18:27:34, Joel Fernandes (Google) wrote: > > Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org> > > This really needs to be merged to previous patch, you can't break > compilation in middle of series... > > Or probably you need hlist_for_each_entry_rcu_lockdep() macro with > additional argument, and switch users to it. Good point. I can also just add a temporary transition macro, and then remove it in the last patch. That way no new macro is needed. Thanks!
On Sun, Jun 2, 2019 at 8:20 AM Joel Fernandes <joel@joelfernandes.org> wrote: > > On Sun, Jun 2, 2019 at 3:00 AM Pavel Machek <pavel@denx.de> wrote: > > > > On Sat 2019-06-01 18:27:34, Joel Fernandes (Google) wrote: > > > Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org> > > > > This really needs to be merged to previous patch, you can't break > > compilation in middle of series... > > > > Or probably you need hlist_for_each_entry_rcu_lockdep() macro with > > additional argument, and switch users to it. > > Good point. I can also just add a temporary transition macro, and then > remove it in the last patch. That way no new macro is needed. Actually, no. There is no compilation break so I did not follow what you mean. The fourth argument to the hlist_for_each_entry_rcu is optional. The only thing that happens is new lockdep warnings will arise which later parts of the series fix by passing in that fourth argument.
On Sun 2019-06-02 08:24:35, Joel Fernandes wrote: > On Sun, Jun 2, 2019 at 8:20 AM Joel Fernandes <joel@joelfernandes.org> wrote: > > > > On Sun, Jun 2, 2019 at 3:00 AM Pavel Machek <pavel@denx.de> wrote: > > > > > > On Sat 2019-06-01 18:27:34, Joel Fernandes (Google) wrote: > > > > Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org> > > > > > > This really needs to be merged to previous patch, you can't break > > > compilation in middle of series... > > > > > > Or probably you need hlist_for_each_entry_rcu_lockdep() macro with > > > additional argument, and switch users to it. > > > > Good point. I can also just add a temporary transition macro, and then > > remove it in the last patch. That way no new macro is needed. > > Actually, no. There is no compilation break so I did not follow what > you mean. The fourth argument to the hlist_for_each_entry_rcu is > optional. The only thing that happens is new lockdep warnings will > arise which later parts of the series fix by passing in that fourth > argument. Sorry, I missed that subtlety. Might be worth it enabling the lockdep warning last in the series... Pavel
On Mon, Jun 3, 2019 at 2:42 AM Pavel Machek <pavel@denx.de> wrote: > > On Sun 2019-06-02 08:24:35, Joel Fernandes wrote: > > On Sun, Jun 2, 2019 at 8:20 AM Joel Fernandes <joel@joelfernandes.org> wrote: > > > > > > On Sun, Jun 2, 2019 at 3:00 AM Pavel Machek <pavel@denx.de> wrote: > > > > > > > > On Sat 2019-06-01 18:27:34, Joel Fernandes (Google) wrote: > > > > > Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org> > > > > > > > > This really needs to be merged to previous patch, you can't break > > > > compilation in middle of series... > > > > > > > > Or probably you need hlist_for_each_entry_rcu_lockdep() macro with > > > > additional argument, and switch users to it. > > > > > > Good point. I can also just add a temporary transition macro, and then > > > remove it in the last patch. That way no new macro is needed. > > > > Actually, no. There is no compilation break so I did not follow what > > you mean. The fourth argument to the hlist_for_each_entry_rcu is > > optional. The only thing that happens is new lockdep warnings will > > arise which later parts of the series fix by passing in that fourth > > argument. > > Sorry, I missed that subtlety. Might be worth it enabling the lockdep > warning last in the series... Good idea, will do! Thanks.
diff --git a/net/ipv4/fib_frontend.c b/net/ipv4/fib_frontend.c index b298255f6fdb..ef7c9f8e8682 100644 --- a/net/ipv4/fib_frontend.c +++ b/net/ipv4/fib_frontend.c @@ -127,7 +127,8 @@ struct fib_table *fib_get_table(struct net *net, u32 id) h = id & (FIB_TABLE_HASHSZ - 1); head = &net->ipv4.fib_table_hash[h]; - hlist_for_each_entry_rcu(tb, head, tb_hlist) { + hlist_for_each_entry_rcu(tb, head, tb_hlist, + lockdep_rtnl_is_held()) { if (tb->tb_id == id) return tb; }
Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org> --- net/ipv4/fib_frontend.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)