* [Linux-kernel-mentees] [PATCH] tcp: Pass lockdep expression to RCU lists @ 2020-02-19 10:05 ` Amol Grover 0 siblings, 0 replies; 6+ messages in thread From: Amol Grover @ 2020-02-19 10:05 UTC (permalink / raw) To: Eric Dumazet, David S . Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI, Jakub Kicinski Cc: Paul E . McKenney, netdev, linux-kernel, Madhuparna Bhowmik, Joel Fernandes, linux-kernel-mentees tcp_cong_list is traversed using list_for_each_entry_rcu outside an RCU read-side critical section but under the protection of tcp_cong_list_lock. Hence, add corresponding lockdep expression to silence false-positive warnings, and harden RCU lists. Signed-off-by: Amol Grover <frextrite@gmail.com> --- net/ipv4/tcp_cong.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/net/ipv4/tcp_cong.c b/net/ipv4/tcp_cong.c index 3737ec096650..8d4446ed309e 100644 --- a/net/ipv4/tcp_cong.c +++ b/net/ipv4/tcp_cong.c @@ -25,7 +25,8 @@ static struct tcp_congestion_ops *tcp_ca_find(const char *name) { struct tcp_congestion_ops *e; - list_for_each_entry_rcu(e, &tcp_cong_list, list) { + list_for_each_entry_rcu(e, &tcp_cong_list, list, + lockdep_is_held(&tcp_cong_list_lock)) { if (strcmp(e->name, name) == 0) return e; } @@ -55,7 +56,8 @@ struct tcp_congestion_ops *tcp_ca_find_key(u32 key) { struct tcp_congestion_ops *e; - list_for_each_entry_rcu(e, &tcp_cong_list, list) { + list_for_each_entry_rcu(e, &tcp_cong_list, list, + lockdep_is_held(&tcp_cong_list_lock)) { if (e->key == key) return e; } @@ -317,7 +319,8 @@ int tcp_set_allowed_congestion_control(char *val) } /* pass 2 clear old values */ - list_for_each_entry_rcu(ca, &tcp_cong_list, list) + list_for_each_entry_rcu(ca, &tcp_cong_list, list, + lockdep_is_held(&tcp_cong_list_lock)) ca->flags &= ~TCP_CONG_NON_RESTRICTED; /* pass 3 mark as allowed */ -- 2.24.1 _______________________________________________ Linux-kernel-mentees mailing list Linux-kernel-mentees@lists.linuxfoundation.org https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH] tcp: Pass lockdep expression to RCU lists @ 2020-02-19 10:05 ` Amol Grover 0 siblings, 0 replies; 6+ messages in thread From: Amol Grover @ 2020-02-19 10:05 UTC (permalink / raw) To: Eric Dumazet, David S . Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI, Jakub Kicinski Cc: linux-kernel, linux-kernel-mentees, Joel Fernandes, Madhuparna Bhowmik, Paul E . McKenney, netdev, Amol Grover tcp_cong_list is traversed using list_for_each_entry_rcu outside an RCU read-side critical section but under the protection of tcp_cong_list_lock. Hence, add corresponding lockdep expression to silence false-positive warnings, and harden RCU lists. Signed-off-by: Amol Grover <frextrite@gmail.com> --- net/ipv4/tcp_cong.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/net/ipv4/tcp_cong.c b/net/ipv4/tcp_cong.c index 3737ec096650..8d4446ed309e 100644 --- a/net/ipv4/tcp_cong.c +++ b/net/ipv4/tcp_cong.c @@ -25,7 +25,8 @@ static struct tcp_congestion_ops *tcp_ca_find(const char *name) { struct tcp_congestion_ops *e; - list_for_each_entry_rcu(e, &tcp_cong_list, list) { + list_for_each_entry_rcu(e, &tcp_cong_list, list, + lockdep_is_held(&tcp_cong_list_lock)) { if (strcmp(e->name, name) == 0) return e; } @@ -55,7 +56,8 @@ struct tcp_congestion_ops *tcp_ca_find_key(u32 key) { struct tcp_congestion_ops *e; - list_for_each_entry_rcu(e, &tcp_cong_list, list) { + list_for_each_entry_rcu(e, &tcp_cong_list, list, + lockdep_is_held(&tcp_cong_list_lock)) { if (e->key == key) return e; } @@ -317,7 +319,8 @@ int tcp_set_allowed_congestion_control(char *val) } /* pass 2 clear old values */ - list_for_each_entry_rcu(ca, &tcp_cong_list, list) + list_for_each_entry_rcu(ca, &tcp_cong_list, list, + lockdep_is_held(&tcp_cong_list_lock)) ca->flags &= ~TCP_CONG_NON_RESTRICTED; /* pass 3 mark as allowed */ -- 2.24.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Linux-kernel-mentees] [PATCH] tcp: Pass lockdep expression to RCU lists 2020-02-19 10:05 ` Amol Grover @ 2020-02-19 19:35 ` Eric Dumazet -1 siblings, 0 replies; 6+ messages in thread From: Eric Dumazet @ 2020-02-19 19:35 UTC (permalink / raw) To: Amol Grover, Eric Dumazet, David S . Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI, Jakub Kicinski Cc: Paul E . McKenney, netdev, linux-kernel, Madhuparna Bhowmik, Joel Fernandes, linux-kernel-mentees On 2/19/20 2:05 AM, Amol Grover wrote: > tcp_cong_list is traversed using list_for_each_entry_rcu > outside an RCU read-side critical section but under the protection > of tcp_cong_list_lock. > This is not true. There are cases where RCU read lock is held, and others where the tcp_cong_list_lock is held. I believe you need to be more precise in the changelog. If there was a bug, net tree would be the target for this patch, with a required Fixes: tag. Otherwise, if net-next tree is the intended target, you have to signal it, as instructed in Documentation/networking/netdev-FAQ.rst Thanks. > Hence, add corresponding lockdep expression to silence false-positive > warnings, and harden RCU lists. > > Signed-off-by: Amol Grover <frextrite@gmail.com> > --- > net/ipv4/tcp_cong.c | 9 ++++++--- > 1 file changed, 6 insertions(+), 3 deletions(-) > > diff --git a/net/ipv4/tcp_cong.c b/net/ipv4/tcp_cong.c > index 3737ec096650..8d4446ed309e 100644 > --- a/net/ipv4/tcp_cong.c > +++ b/net/ipv4/tcp_cong.c > @@ -25,7 +25,8 @@ static struct tcp_congestion_ops *tcp_ca_find(const char *name) > { > struct tcp_congestion_ops *e; > > - list_for_each_entry_rcu(e, &tcp_cong_list, list) { > + list_for_each_entry_rcu(e, &tcp_cong_list, list, > + lockdep_is_held(&tcp_cong_list_lock)) { > if (strcmp(e->name, name) == 0) > return e; > } > @@ -55,7 +56,8 @@ struct tcp_congestion_ops *tcp_ca_find_key(u32 key) > { > struct tcp_congestion_ops *e; > > - list_for_each_entry_rcu(e, &tcp_cong_list, list) { > + list_for_each_entry_rcu(e, &tcp_cong_list, list, > + lockdep_is_held(&tcp_cong_list_lock)) { > if (e->key == key) > return e; > } > @@ -317,7 +319,8 @@ int tcp_set_allowed_congestion_control(char *val) > } > > /* pass 2 clear old values */ > - list_for_each_entry_rcu(ca, &tcp_cong_list, list) > + list_for_each_entry_rcu(ca, &tcp_cong_list, list, > + lockdep_is_held(&tcp_cong_list_lock)) > ca->flags &= ~TCP_CONG_NON_RESTRICTED; > > /* pass 3 mark as allowed */ > _______________________________________________ Linux-kernel-mentees mailing list Linux-kernel-mentees@lists.linuxfoundation.org https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] tcp: Pass lockdep expression to RCU lists @ 2020-02-19 19:35 ` Eric Dumazet 0 siblings, 0 replies; 6+ messages in thread From: Eric Dumazet @ 2020-02-19 19:35 UTC (permalink / raw) To: Amol Grover, Eric Dumazet, David S . Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI, Jakub Kicinski Cc: linux-kernel, linux-kernel-mentees, Joel Fernandes, Madhuparna Bhowmik, Paul E . McKenney, netdev On 2/19/20 2:05 AM, Amol Grover wrote: > tcp_cong_list is traversed using list_for_each_entry_rcu > outside an RCU read-side critical section but under the protection > of tcp_cong_list_lock. > This is not true. There are cases where RCU read lock is held, and others where the tcp_cong_list_lock is held. I believe you need to be more precise in the changelog. If there was a bug, net tree would be the target for this patch, with a required Fixes: tag. Otherwise, if net-next tree is the intended target, you have to signal it, as instructed in Documentation/networking/netdev-FAQ.rst Thanks. > Hence, add corresponding lockdep expression to silence false-positive > warnings, and harden RCU lists. > > Signed-off-by: Amol Grover <frextrite@gmail.com> > --- > net/ipv4/tcp_cong.c | 9 ++++++--- > 1 file changed, 6 insertions(+), 3 deletions(-) > > diff --git a/net/ipv4/tcp_cong.c b/net/ipv4/tcp_cong.c > index 3737ec096650..8d4446ed309e 100644 > --- a/net/ipv4/tcp_cong.c > +++ b/net/ipv4/tcp_cong.c > @@ -25,7 +25,8 @@ static struct tcp_congestion_ops *tcp_ca_find(const char *name) > { > struct tcp_congestion_ops *e; > > - list_for_each_entry_rcu(e, &tcp_cong_list, list) { > + list_for_each_entry_rcu(e, &tcp_cong_list, list, > + lockdep_is_held(&tcp_cong_list_lock)) { > if (strcmp(e->name, name) == 0) > return e; > } > @@ -55,7 +56,8 @@ struct tcp_congestion_ops *tcp_ca_find_key(u32 key) > { > struct tcp_congestion_ops *e; > > - list_for_each_entry_rcu(e, &tcp_cong_list, list) { > + list_for_each_entry_rcu(e, &tcp_cong_list, list, > + lockdep_is_held(&tcp_cong_list_lock)) { > if (e->key == key) > return e; > } > @@ -317,7 +319,8 @@ int tcp_set_allowed_congestion_control(char *val) > } > > /* pass 2 clear old values */ > - list_for_each_entry_rcu(ca, &tcp_cong_list, list) > + list_for_each_entry_rcu(ca, &tcp_cong_list, list, > + lockdep_is_held(&tcp_cong_list_lock)) > ca->flags &= ~TCP_CONG_NON_RESTRICTED; > > /* pass 3 mark as allowed */ > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Linux-kernel-mentees] [PATCH] tcp: Pass lockdep expression to RCU lists 2020-02-19 19:35 ` Eric Dumazet @ 2020-02-20 3:47 ` Amol Grover -1 siblings, 0 replies; 6+ messages in thread From: Amol Grover @ 2020-02-20 3:47 UTC (permalink / raw) To: Eric Dumazet Cc: Paul E . McKenney, Hideaki YOSHIFUJI, netdev, linux-kernel, Madhuparna Bhowmik, Eric Dumazet, Jakub Kicinski, Joel Fernandes, Alexey Kuznetsov, linux-kernel-mentees, David S . Miller On Wed, Feb 19, 2020 at 11:35:21AM -0800, Eric Dumazet wrote: > > > On 2/19/20 2:05 AM, Amol Grover wrote: > > tcp_cong_list is traversed using list_for_each_entry_rcu > > outside an RCU read-side critical section but under the protection > > of tcp_cong_list_lock. > > > > This is not true. > > There are cases where RCU read lock is held, > and others where the tcp_cong_list_lock is held. > That's true but this patch specifically fixes those occurences of list_for_each_entry_rcu() that are traversed under tcp_cong_list_lock. Moreover, an implicit check is done for being inside RCU read-side critical section along with testing for this newly added lockdep expression. > I believe you need to be more precise in the changelog. > > If there was a bug, net tree would be the target for this patch, > with a required Fixes: tag. > > Otherwise, if net-next tree is the intended target, you have to signal > it, as instructed in Documentation/networking/netdev-FAQ.rst > I don't think this fixes a "bug". However, this may fix potential bugs that may creep in. Should I send it against net-next tree? Thanks Amol > Thanks. > > > > Hence, add corresponding lockdep expression to silence false-positive > > warnings, and harden RCU lists. > > > > Signed-off-by: Amol Grover <frextrite@gmail.com> > > --- > > net/ipv4/tcp_cong.c | 9 ++++++--- > > 1 file changed, 6 insertions(+), 3 deletions(-) > > > > diff --git a/net/ipv4/tcp_cong.c b/net/ipv4/tcp_cong.c > > index 3737ec096650..8d4446ed309e 100644 > > --- a/net/ipv4/tcp_cong.c > > +++ b/net/ipv4/tcp_cong.c > > @@ -25,7 +25,8 @@ static struct tcp_congestion_ops *tcp_ca_find(const char *name) > > { > > struct tcp_congestion_ops *e; > > > > - list_for_each_entry_rcu(e, &tcp_cong_list, list) { > > + list_for_each_entry_rcu(e, &tcp_cong_list, list, > > + lockdep_is_held(&tcp_cong_list_lock)) { > > if (strcmp(e->name, name) == 0) > > return e; > > } > > @@ -55,7 +56,8 @@ struct tcp_congestion_ops *tcp_ca_find_key(u32 key) > > { > > struct tcp_congestion_ops *e; > > > > - list_for_each_entry_rcu(e, &tcp_cong_list, list) { > > + list_for_each_entry_rcu(e, &tcp_cong_list, list, > > + lockdep_is_held(&tcp_cong_list_lock)) { > > if (e->key == key) > > return e; > > } > > @@ -317,7 +319,8 @@ int tcp_set_allowed_congestion_control(char *val) > > } > > > > /* pass 2 clear old values */ > > - list_for_each_entry_rcu(ca, &tcp_cong_list, list) > > + list_for_each_entry_rcu(ca, &tcp_cong_list, list, > > + lockdep_is_held(&tcp_cong_list_lock)) > > ca->flags &= ~TCP_CONG_NON_RESTRICTED; > > > > /* pass 3 mark as allowed */ > > _______________________________________________ Linux-kernel-mentees mailing list Linux-kernel-mentees@lists.linuxfoundation.org https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] tcp: Pass lockdep expression to RCU lists @ 2020-02-20 3:47 ` Amol Grover 0 siblings, 0 replies; 6+ messages in thread From: Amol Grover @ 2020-02-20 3:47 UTC (permalink / raw) To: Eric Dumazet Cc: Eric Dumazet, David S . Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI, Jakub Kicinski, linux-kernel, linux-kernel-mentees, Joel Fernandes, Madhuparna Bhowmik, Paul E . McKenney, netdev On Wed, Feb 19, 2020 at 11:35:21AM -0800, Eric Dumazet wrote: > > > On 2/19/20 2:05 AM, Amol Grover wrote: > > tcp_cong_list is traversed using list_for_each_entry_rcu > > outside an RCU read-side critical section but under the protection > > of tcp_cong_list_lock. > > > > This is not true. > > There are cases where RCU read lock is held, > and others where the tcp_cong_list_lock is held. > That's true but this patch specifically fixes those occurences of list_for_each_entry_rcu() that are traversed under tcp_cong_list_lock. Moreover, an implicit check is done for being inside RCU read-side critical section along with testing for this newly added lockdep expression. > I believe you need to be more precise in the changelog. > > If there was a bug, net tree would be the target for this patch, > with a required Fixes: tag. > > Otherwise, if net-next tree is the intended target, you have to signal > it, as instructed in Documentation/networking/netdev-FAQ.rst > I don't think this fixes a "bug". However, this may fix potential bugs that may creep in. Should I send it against net-next tree? Thanks Amol > Thanks. > > > > Hence, add corresponding lockdep expression to silence false-positive > > warnings, and harden RCU lists. > > > > Signed-off-by: Amol Grover <frextrite@gmail.com> > > --- > > net/ipv4/tcp_cong.c | 9 ++++++--- > > 1 file changed, 6 insertions(+), 3 deletions(-) > > > > diff --git a/net/ipv4/tcp_cong.c b/net/ipv4/tcp_cong.c > > index 3737ec096650..8d4446ed309e 100644 > > --- a/net/ipv4/tcp_cong.c > > +++ b/net/ipv4/tcp_cong.c > > @@ -25,7 +25,8 @@ static struct tcp_congestion_ops *tcp_ca_find(const char *name) > > { > > struct tcp_congestion_ops *e; > > > > - list_for_each_entry_rcu(e, &tcp_cong_list, list) { > > + list_for_each_entry_rcu(e, &tcp_cong_list, list, > > + lockdep_is_held(&tcp_cong_list_lock)) { > > if (strcmp(e->name, name) == 0) > > return e; > > } > > @@ -55,7 +56,8 @@ struct tcp_congestion_ops *tcp_ca_find_key(u32 key) > > { > > struct tcp_congestion_ops *e; > > > > - list_for_each_entry_rcu(e, &tcp_cong_list, list) { > > + list_for_each_entry_rcu(e, &tcp_cong_list, list, > > + lockdep_is_held(&tcp_cong_list_lock)) { > > if (e->key == key) > > return e; > > } > > @@ -317,7 +319,8 @@ int tcp_set_allowed_congestion_control(char *val) > > } > > > > /* pass 2 clear old values */ > > - list_for_each_entry_rcu(ca, &tcp_cong_list, list) > > + list_for_each_entry_rcu(ca, &tcp_cong_list, list, > > + lockdep_is_held(&tcp_cong_list_lock)) > > ca->flags &= ~TCP_CONG_NON_RESTRICTED; > > > > /* pass 3 mark as allowed */ > > ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-02-20 3:47 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2020-02-19 10:05 [Linux-kernel-mentees] [PATCH] tcp: Pass lockdep expression to RCU lists Amol Grover 2020-02-19 10:05 ` Amol Grover 2020-02-19 19:35 ` [Linux-kernel-mentees] " Eric Dumazet 2020-02-19 19:35 ` Eric Dumazet 2020-02-20 3:47 ` [Linux-kernel-mentees] " Amol Grover 2020-02-20 3:47 ` Amol Grover
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.