Re: vmw_pvrdma: Add SRQ support - broke rdma-core

[Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>]

[-- Attachment #1: Type: text/plain, Size: 1776 bytes --]

On Wed, Nov 15, 2017 at 02:10:54PM -0800, Bryan Tan wrote:
> On Tue, Nov 14, 2017 at 08:15:26PM -0700, Jason Gunthorpe wrote:
> > On Tue, Nov 14, 2017 at 06:49:25PM -0800, Bryan Tan wrote:
> >
> > > Thanks for your suggestion, Jason. I've opened up a new pull request
> > > that includes the original commit for SRQ support that has been
> > > reverted, along with the one line change you suggested. Let me know if
> > > you prefer something different. I've run the travis build on kernels
> > > both with and without the new ABI header as well.
> >
> > You should swap the order of the two patches and move the change of
> > buildlib/fixup-include/rdma-vmw_pvrdma-abi.h to the other patch.
> >
> > Then the description of the abi.h patch would just be:
> >
> > 'Update rdma-core to the latest kernel pvrdma-abi.h header'
> >
> > Then everything is more self contained.
> >
> > You can do this and force push a update to your PR.
>
> Got it, I've done a force push with the new commits. There were two
> other items that were in the kernel ABI header and not in the
> fixup-include header, so I've also fixed that.
>
> Unfortunately, I also missed adding the macro I am testing for in
> RDMA_LinuxHeaders.cmake in the kernel ABI header ): so I sent out a
> patch to fix that. This should not be a problem, as the test for
> PVRDMA_UAR_SRQ_OFFSET will make sure we use the header from
> fixup-include for now, until the kernel patch is accepted.

No, it is a problem.
You sent two patches, while the PR passes the compilation on my machine,
the first patch alone doesn't.

Every patch should be standalone.

Please fix.

>
> Going forward, I'll copy the ABI header file to make sure they are
> synced up and I don't make this mistake again.
>
> Thanks!
> Bryan
>
> >
> > Jason

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]