2019-06-01 22:32:38

by Joel Fernandes

[permalink] [raw]
Subject: [RFC 2/6] ipv4: add lockdep condition to fix for_each_entry

Signed-off-by: Joel Fernandes (Google) <[email protected]>
---
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;
}
--
2.22.0.rc1.311.g5d7573a151-goog


2019-06-02 07:11:08

by Pavel Machek

[permalink] [raw]
Subject: Re: [RFC 2/6] ipv4: add lockdep condition to fix for_each_entry

On Sat 2019-06-01 18:27:34, Joel Fernandes (Google) wrote:
> Signed-off-by: Joel Fernandes (Google) <[email protected]>

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;
> }

--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


Attachments:
(No filename) (1.10 kB)
signature.asc (188.00 B)
Digital signature
Download all attachments

2019-06-02 12:22:23

by Joel Fernandes

[permalink] [raw]
Subject: Re: [RFC 2/6] ipv4: add lockdep condition to fix for_each_entry

On Sun, Jun 2, 2019 at 3:00 AM Pavel Machek <[email protected]> wrote:
>
> On Sat 2019-06-01 18:27:34, Joel Fernandes (Google) wrote:
> > Signed-off-by: Joel Fernandes (Google) <[email protected]>
>
> 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!

2019-06-02 12:26:09

by Joel Fernandes

[permalink] [raw]
Subject: Re: [RFC 2/6] ipv4: add lockdep condition to fix for_each_entry

On Sun, Jun 2, 2019 at 8:20 AM Joel Fernandes <[email protected]> wrote:
>
> On Sun, Jun 2, 2019 at 3:00 AM Pavel Machek <[email protected]> wrote:
> >
> > On Sat 2019-06-01 18:27:34, Joel Fernandes (Google) wrote:
> > > Signed-off-by: Joel Fernandes (Google) <[email protected]>
> >
> > 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.

2019-06-03 06:44:20

by Pavel Machek

[permalink] [raw]
Subject: Re: [RFC 2/6] ipv4: add lockdep condition to fix for_each_entry

On Sun 2019-06-02 08:24:35, Joel Fernandes wrote:
> On Sun, Jun 2, 2019 at 8:20 AM Joel Fernandes <[email protected]> wrote:
> >
> > On Sun, Jun 2, 2019 at 3:00 AM Pavel Machek <[email protected]> wrote:
> > >
> > > On Sat 2019-06-01 18:27:34, Joel Fernandes (Google) wrote:
> > > > Signed-off-by: Joel Fernandes (Google) <[email protected]>
> > >
> > > 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
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany


Attachments:
(No filename) (1.29 kB)
signature.asc (188.00 B)
Digital signature
Download all attachments

2019-06-03 12:31:37

by Joel Fernandes

[permalink] [raw]
Subject: Re: [RFC 2/6] ipv4: add lockdep condition to fix for_each_entry

On Mon, Jun 3, 2019 at 2:42 AM Pavel Machek <[email protected]> wrote:
>
> On Sun 2019-06-02 08:24:35, Joel Fernandes wrote:
> > On Sun, Jun 2, 2019 at 8:20 AM Joel Fernandes <[email protected]> wrote:
> > >
> > > On Sun, Jun 2, 2019 at 3:00 AM Pavel Machek <[email protected]> wrote:
> > > >
> > > > On Sat 2019-06-01 18:27:34, Joel Fernandes (Google) wrote:
> > > > > Signed-off-by: Joel Fernandes (Google) <[email protected]>
> > > >
> > > > 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.