Leon, The only intention I had with that note was to try to help with the process of screening patches to maintain and improve the quality of common rdma code. As it was taken as an insult I apologize for that. Never had in mind. I should have scripted it better. Perhaps, put the note through the checkpatch :) And yes, as a member of rdma list I accept my share of blame for missing that bug. Thanks, Alex. P.S. My apologies to the community for the spam. > On Sat, Sep 23, 2017 at 07:20:53PM +0000, Estrin, Alex wrote: > > > > Hello, > > > > > > > > One minor note regarding the original commit 523633359224 > > > > that broke the core. > > > > It seem it was let through without trivial validation, > > > > otherwise it wouldn't pass the checkpatch. > > > > > > Can you be more specific? Are you referring to "WARNING: line over 80 > > > characters" or to something else? If yes, I feel really bad for you and > > > your workplace. > > Please don't be. Keep doing a great job at your workplace, I will do the same at > mine. > > > > > Readability is a first priority for the submitted code. > > I can agree with you on that, considering easy readable submitted code > > does not introduce a trivial bugs. > > It will be very helpful to everyone if you stop to throw claims without any actual > support. > 1. Doug allows enough time to respond on the patches and neither you and > neither your > colleagues didn't see such "trivial bug" back then. > 2. It fixed another "trivial bug" introduced by your colleague which > broke RoCE (one of the most popular fabric in the stack) and we didn't > cry other the internet about it. > > Before you are rushing to reply me, please consult with Denny, he can > give you a short update on how hard the recent OPA changes in AH and > LIDs broke the stack and RoCE/IB devices. > > > > > > ➜ linux-rdma git:(rdma-rc) git fp -1 523633359224 -o /tmp/ > > > /tmp/0001-IB-core-Fix-the-validations-of-a-multicast-LID-in-at.patch > > > ➜ linux-rdma git:(rdma-rc) ./scripts/checkpatch.pl --strict /tmp/0001-IB-core- > Fix- > > > the-validations-of-a-multicast-LID-in-at.patch > > > WARNING: line over 80 characters > > > #45: FILE: drivers/infiniband/core/verbs.c:1584: > > > + if (qp->device->get_link_layer(qp->device, attr.port_num) != > > > > > > total: 0 errors, 1 warnings, 0 checks, 62 lines checked > > > > > > NOTE: For some of the reported defects, checkpatch may be able to > > > mechanically convert to the typical style using --fix or --fix-inplace. > > > > > > /tmp/0001-IB-core-Fix-the-validations-of-a-multicast-LID-in-at.patch has > style > > > problems, please review. > > > > > > NOTE: If any of the errors are false positives, please report > > > them to the maintainer, see CHECKPATCH in MAINTAINERS. > > > > > > > > > > > > > > Thanks, > > > > Alex. > > > > > > > > > On Fri, Sep 22, 2017 at 06:42:41PM -0400, Doug Ledford wrote: > > > > > > On Fri, 2017-09-22 at 15:17 -0600, Jason Gunthorpe wrote: > > > > > > > On Fri, Sep 22, 2017 at 05:06:26PM -0400, Doug Ledford wrote: > > > > > > > > > > > > > > > Sure, I get that, but I was already out on PTO on the 30th. What > > > > > > > > sucks > > > > > > > > is that it landed right after I was out. But I plan to have the > > > > > > > > pull > > > > > > > > request in before EOB today, so the difference between the 20th > and > > > > > > > > today is neglible. Especially since lots of people doing QA > > > > > > > > testing > > > > > > > > prefer to take -rc tags, in that case, the difference is non- > > > > > > > > existent. > > > > > > > > > > > > > > My thinking was that people should test -rc, > > > > > > > > > > > > Great, with you here... > > > > > > > > > > > > > but if they have problems > > > > > > > they could grab your for-rc branch and check if their issue is > > > > > > > already > > > > > > > fixed.. > > > > > > > > > > > > They can do this too... > > > > > > > > > > > > But if that still doesn't resolve their problem, a quick check of the > > > > > > mailing list contents isn't out of the question either. In that case, > > > > > > they would have found the solution to their problem. But, when you > get > > > > > > right down to it, only one person reported it in addition to the > > > > > > original poster, so either other people saw the original post and > > > > > > compensated in their own testing, or (the more likely I think), most > > > > > > people don't start testing -rcs until after -rc2. > > > > > > > > > > I don't know about other people, but our testing of -rc starts on -rc1 > > > > > and we are not waiting for -rc2. From my observe of netdev, they also > > > > > start to test -rc immediately. > > > > > > > > > > Otherwise, what is the point of the week between -rc1 and -rc2? > > > > > > > > > > > Which is why I try to set -rc2 as a milestone for several purposes. > > > > > > For getting in the bulk of the known fixes, but also as a branching > > > > > > point for for-next. > > > > > > > > > > > > -- > > > > > > Doug Ledford <dledford@xxxxxxxxxx> > > > > > > GPG KeyID: B826A3330E572FDD > > > > > > Key fingerprint = AE6B 1BDA 122B 23B4 265B 1274 B826 A333 > 0E57 > > > 2FDD > > > > > > > > > > -- > > > > To unsubscribe from this list: send the line "unsubscribe linux-rdma" in > > > > the body of a message to majordomo@xxxxxxxxxxxxxxx > > > > More majordomo info at http://vger.kernel.org/majordomo-info.html ��.n��������+%������w��{.n�����{���fk��ܨ}���Ơz�j:+v�����w����ޙ��&�)ߡ�a����z�ޗ���ݢj��w�f
