| Message ID | 1430772572.27254.3.camel@edumazet-glaptop2.roam.corp.google.com |
|---|---|
| State | RFC, archived |
| Delegated to: | David Miller |
| Headers | show |
On Mon, May 4, 2015 at 1:49 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote: > On Mon, 2015-05-04 at 11:45 -0700, Cong Wang wrote: >> On Mon, May 4, 2015 at 6:39 AM, Dominick Grift <dac.override@gmail.com> wrote: >> > On Thu, Apr 23, 2015 at 01:07:45PM -0400, Josh Boyer wrote: >> >> Hi All, >> >> >> >> We've had a user report the following backtrace from the bridge module >> >> with a recent Linus' tree. Has anything like this been reported yet? >> >> If you have any questions on setup, the user is CC'd. >> >> >> >> josh >> >> >> >> [ 29.382235] br0: port 1(tap0) entered forwarding state >> >> >> >> [ 29.382286] =============================== >> >> [ 29.382315] [ INFO: suspicious RCU usage. ] >> >> [ 29.382344] 4.1.0-0.rc0.git11.1.fc23.x86_64 #1 Not tainted >> >> [ 29.382380] ------------------------------- >> >> [ 29.382409] net/bridge/br_private.h:626 suspicious >> >> rcu_dereference_check() usage! >> > >> > <snip> >> > >> > With 4.1.0-0.rc1.git1.1.fc23.x86_64 the situation seems to have slightly changed: >> > >> >> Should be the same issue. Please give the attached patch a try, >> it is compile-tested only. >> >> Thanks! > > Please send inline patches, otherwise its hard for us to review them and > give feedback. Compile-test only patch is never ready for review, I thought it is too obvious to mention. > > At first glance, it is way too complicated. > > br_get_vlan_info() change is not needed : > > rcu_dereference_rtnl() can already be used from RCU or RTNL protected > code. > > (Quite different from rtnl_dereference()) > > What about : Feel free to submit a formal patch, I apparently don't have time to read more into this, nor have time to argument on details. Thanks. -- 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
On Mon, 2015-05-04 at 14:38 -0700, Cong Wang wrote: > Compile-test only patch is never ready for review, I thought it is too obvious > to mention. Thats not relevant. Obvious or not, you are not making things easy for reviewers. Our time is precious too, even if you believe its a minor detail. If you don't care for our feedback, send private patches. When I hit 'Reply' to your mail, I don't have the patch available at hand, and need to copy/paste it. Surely I am not alone. Surely you can change the way you provide patches, even if they are in RFC state. If you don't have time, just ignore these bug reports, and do whatever you need to, nobody will complain. -- 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
On Mon, May 4, 2015 at 3:06 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote: > On Mon, 2015-05-04 at 14:38 -0700, Cong Wang wrote: > >> Compile-test only patch is never ready for review, I thought it is too obvious >> to mention. > > Thats not relevant. > > Obvious or not, you are not making things easy for reviewers. > Why should I make a NOT-ready-for-review patch easy for reviewers? What's the point here? I don't have the right to choose my patch NOT to be reviewed? Huh... > Our time is precious too, even if you believe its a minor detail. > > If you don't care for our feedback, send private patches. Not until it is ready, apparently. > > When I hit 'Reply' to your mail, I don't have the patch available at > hand, and need to copy/paste it. That is EXACTLY what I want, because it is NOT READY FOR REVIEW. > > Surely I am not alone. > > Surely you can change the way you provide patches, even if they are in > RFC state. If every patch in this mailing list has to be ready, yes. Otherwise no, sometimes we do need a patch (or a diff if it is a better name) ONLY for discussion, aka, NOT for review. > > If you don't have time, just ignore these bug reports, and do whatever > you need to, nobody will complain. > Apparently you did nothing before I just sent a reply without a patch, only patch draws your attention for some reason, I _guess_ you only rush for patches. -- 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
On Mon, 2015-05-04 at 15:17 -0700, Cong Wang wrote: > On Mon, May 4, 2015 at 3:06 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote: > > On Mon, 2015-05-04 at 14:38 -0700, Cong Wang wrote: > > > >> Compile-test only patch is never ready for review, I thought it is too obvious > >> to mention. > > > > Thats not relevant. > > > > Obvious or not, you are not making things easy for reviewers. > > > > Why should I make a NOT-ready-for-review patch easy for reviewers? > What's the point here? I don't have the right to choose my patch NOT > to be reviewed? Huh... > Look, I only said : "Please send inline patches, otherwise its hard for us to review them and give feedback." I gave a quite reasonable request, with a "Please" I was not expecting this becoming a war. Next time, I will let David deal with your patches. -- 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
On Mon, May 4, 2015 at 3:29 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote: > On Mon, 2015-05-04 at 15:17 -0700, Cong Wang wrote: >> On Mon, May 4, 2015 at 3:06 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote: >> > On Mon, 2015-05-04 at 14:38 -0700, Cong Wang wrote: >> > >> >> Compile-test only patch is never ready for review, I thought it is too obvious >> >> to mention. >> > >> > Thats not relevant. >> > >> > Obvious or not, you are not making things easy for reviewers. >> > >> >> Why should I make a NOT-ready-for-review patch easy for reviewers? >> What's the point here? I don't have the right to choose my patch NOT >> to be reviewed? Huh... >> > > Look, I only said : > > "Please send inline patches, otherwise its hard for us to review them > and give feedback." > > I gave a quite reasonable request, with a "Please" With hundreds of patches merged, I don't need to teach how to send a reviewable patch when it is ready, this one is just not ready for review. So your request is only valid for patches ready for review. Focus on the bug. It is not my first time to get surprised netdev people for some reason only talk when there is a patch, yet another reason I should send a patch even just for drawing attentions. -- 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
On Mon, 2015-05-04 at 15:44 -0700, Cong Wang wrote: > So your request is only valid for patches ready for review. Focus on > the bug. Yep, I was trying to, but couldn't reply on your patch, and this is not the first time it happens with you. Thus my reasonable request. Turns out you did not understand it and turned it into personal attack. Too bad. -- 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
From: Eric Dumazet <eric.dumazet@gmail.com> Date: Mon, 04 May 2015 16:00:04 -0700 > On Mon, 2015-05-04 at 15:44 -0700, Cong Wang wrote: > >> So your request is only valid for patches ready for review. Focus on >> the bug. > > Yep, I was trying to, but couldn't reply on your patch, and this is not > the first time it happens with you. > > Thus my reasonable request. +1 I totally agree with Eric on all of his counts, and I think you are being completely rediculous Cong. -- 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/bridge/br_stp_timer.c b/net/bridge/br_stp_timer.c index 4fcaa67750fda845ad0a180332c4cd96a9524086..7caf7fae2d5b8aa369b924e1c87a47c343fb8954 100644 --- a/net/bridge/br_stp_timer.c +++ b/net/bridge/br_stp_timer.c @@ -97,7 +97,9 @@ static void br_forward_delay_timer_expired(unsigned long arg) netif_carrier_on(br->dev); } br_log_state(p); + rcu_read_lock(); br_ifinfo_notify(RTM_NEWLINK, p); + rcu_read_unlock(); spin_unlock(&br->lock); }