Re: crash in 4.14-rc1 with IPoIB

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Photo]     [Yosemite News]     [Yosemite Photos]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux