Message ID | 5339B84E.7030708@web.de |
---|---|
State | Changes Requested, archived |
Delegated to: | David Miller |
Headers | show |
On Mon, Mar 31, 2014 at 08:47:42PM +0200, Heiner Kallweit wrote: > read_unlock_bh can be moved out of the if clause > > Signed-off-by: Heiner Kallweit <heiner.kallweit@web.de> > --- > net/ipv6/addrconf.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > > diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c > index 6c7fa08..a65812f 100644 > --- a/net/ipv6/addrconf.c > +++ b/net/ipv6/addrconf.c > @@ -2122,6 +2122,7 @@ static void manage_tempaddrs(struct inet6_dev *idev, > if (!(flags&IFA_F_TENTATIVE)) > ipv6_ifa_notify(0, ift); > } > + read_unlock_bh(&idev->lock); > > if ((create || list_empty(&idev->tempaddr_list)) && > idev->cnf.use_tempaddr > 0) { > @@ -2130,10 +2131,7 @@ static void manage_tempaddrs(struct inet6_dev *idev, > * Also create a temporary address if it's enabled but > * no temporary address currently exists. > */ > - read_unlock_bh(&idev->lock); > ipv6_create_tempaddr(ifp, NULL); > - } else { > - read_unlock_bh(&idev->lock); Doesn't the lock also protect the list_empty? I remember there is a list_empty_careful to check concurrently if empty during possible modification. I actually would leave it as is. Thanks, Hannes -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Am 31.03.2014 21:21, schrieb Hannes Frederic Sowa: > On Mon, Mar 31, 2014 at 08:47:42PM +0200, Heiner Kallweit wrote: >> read_unlock_bh can be moved out of the if clause >> >> Signed-off-by: Heiner Kallweit <heiner.kallweit@web.de> >> --- >> net/ipv6/addrconf.c | 4 +--- >> 1 file changed, 1 insertion(+), 3 deletions(-) >> >> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c >> index 6c7fa08..a65812f 100644 >> --- a/net/ipv6/addrconf.c >> +++ b/net/ipv6/addrconf.c >> @@ -2122,6 +2122,7 @@ static void manage_tempaddrs(struct inet6_dev *idev, >> if (!(flags&IFA_F_TENTATIVE)) >> ipv6_ifa_notify(0, ift); >> } >> + read_unlock_bh(&idev->lock); >> >> if ((create || list_empty(&idev->tempaddr_list)) && >> idev->cnf.use_tempaddr > 0) { >> @@ -2130,10 +2131,7 @@ static void manage_tempaddrs(struct inet6_dev *idev, >> * Also create a temporary address if it's enabled but >> * no temporary address currently exists. >> */ >> - read_unlock_bh(&idev->lock); >> ipv6_create_tempaddr(ifp, NULL); >> - } else { >> - read_unlock_bh(&idev->lock); > Doesn't the lock also protect the list_empty? I remember there is > a list_empty_careful to check concurrently if empty during possible > modification. I actually would leave it as is. > > Thanks, > > Hannes > You're right, I missed this. I was a little to fast, sorry. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c index 6c7fa08..a65812f 100644 --- a/net/ipv6/addrconf.c +++ b/net/ipv6/addrconf.c @@ -2122,6 +2122,7 @@ static void manage_tempaddrs(struct inet6_dev *idev, if (!(flags&IFA_F_TENTATIVE)) ipv6_ifa_notify(0, ift); } + read_unlock_bh(&idev->lock); if ((create || list_empty(&idev->tempaddr_list)) && idev->cnf.use_tempaddr > 0) { @@ -2130,10 +2131,7 @@ static void manage_tempaddrs(struct inet6_dev *idev, * Also create a temporary address if it's enabled but * no temporary address currently exists. */ - read_unlock_bh(&idev->lock); ipv6_create_tempaddr(ifp, NULL); - } else { - read_unlock_bh(&idev->lock); } }
read_unlock_bh can be moved out of the if clause Signed-off-by: Heiner Kallweit <heiner.kallweit@web.de> --- net/ipv6/addrconf.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)