All of lore.kernel.org
 help / color / mirror / Atom feed
* vmw_pvrdma: Add SRQ support - broke rdma-core
@ 2017-11-14  8:51 Leon Romanovsky
       [not found] ` <20171114085143.GS18825-U/DQcQFIOTAAJjI8aNfphQ@public.gmane.org>
  0 siblings, 1 reply; 11+ messages in thread
From: Leon Romanovsky @ 2017-11-14  8:51 UTC (permalink / raw)
  To: Bryan Tan; +Cc: RDMA mailing list, Doug Ledford, Jason Gunthorpe

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

The commit 25f07300bfb7 ("vmw_pvrdma: Add SRQ support") causes to build
failures in Travis CI [1] and on my system too [2].

Please fix.

Thanks

[1] https://travis-ci.org/linux-rdma/rdma-core/builds/301651144
[2]

-- Missing Optional Items:
--  Valgrind memcheck.h NOT enabled
--  Valgrind drd.h NOT enabled
-- Configuring done
-- Generating done
-- Build files have been written to: /home/leonro/src/rdma-core/build
[1/81] Linking C shared module lib/libhfi1verbs-rdmav16.so
[2/81] Building C object providers/vmw_pvrdma/CMakeFiles/vmw_pvrdma-rdmav16.dir/pvrdma_main.c.o
FAILED: providers/vmw_pvrdma/CMakeFiles/vmw_pvrdma-rdmav16.dir/pvrdma_main.c.o
/usr/lib/ccache/bin/cc -Dvmw_pvrdma_rdmav16_EXPORTS -Iinclude -I/usr/include/libnl3 -Wall -Wextra -Wno-sign-compare -Wno-unused-parameter -Wmissing-prototypes -Wmissing-declarations -Wwrite-strings -Wformat=2 -Wshadow -Wstrict-prototypes -Wold-style-definition -Wredundant-decls -O2 -g  -fPIC   -std=gnu11 -MD -MT providers/vmw_pvrdma/CMakeFiles/vmw_pvrdma-rdmav16.dir/pvrdma_main.c.o -MF providers/vmw_pvrdma/CMakeFiles/vmw_pvrdma-rdmav16.dir/pvrdma_main.c.o.d -o providers/vmw_pvrdma/CMakeFiles/vmw_pvrdma-rdmav16.dir/pvrdma_main.c.o   -c ../providers/vmw_pvrdma/pvrdma_main.c
In file included from ../providers/vmw_pvrdma/pvrdma_main.c:46:0:
../providers/vmw_pvrdma/pvrdma.h: In function ‘pvrdma_write_uar_srq’:
../providers/vmw_pvrdma/pvrdma.h:244:20: error: ‘PVRDMA_UAR_SRQ_OFFSET’ undeclared (first use in this function); did you mean ‘PVRDMA_UAR_CQ_OFFSET’?
  *(__le32 *)(uar + PVRDMA_UAR_SRQ_OFFSET) = htole32(value);
                    ^~~~~~~~~~~~~~~~~~~~~
                    PVRDMA_UAR_CQ_OFFSET
../providers/vmw_pvrdma/pvrdma.h:244:20: note: each undeclared identifier is reported only once for each function it appears in
[3/81] Building C object providers/vmw_pvrdma/CMakeFiles/vmw_pvrdma-rdmav16.dir/qp.c.o
FAILED: providers/vmw_pvrdma/CMakeFiles/vmw_pvrdma-rdmav16.dir/qp.c.o
/usr/lib/ccache/bin/cc -Dvmw_pvrdma_rdmav16_EXPORTS -Iinclude -I/usr/include/libnl3 -Wall -Wextra -Wno-sign-compare -Wno-unused-parameter -Wmissing-prototypes -Wmissing-declarations -Wwrite-strings -Wformat=2 -Wshadow -Wstrict-prototypes -Wold-style-definition -Wredundant-decls -O2 -g  -fPIC   -std=gnu11 -MD -MT providers/vmw_pvrdma/CMakeFiles/vmw_pvrdma-rdmav16.dir/qp.c.o -MF providers/vmw_pvrdma/CMakeFiles/vmw_pvrdma-rdmav16.dir/qp.c.o.d -o providers/vmw_pvrdma/CMakeFiles/vmw_pvrdma-rdmav16.dir/qp.c.o   -c ../providers/vmw_pvrdma/qp.c
In file included from ../providers/vmw_pvrdma/qp.c:48:0:
../providers/vmw_pvrdma/pvrdma.h: In function ‘pvrdma_write_uar_srq’:
../providers/vmw_pvrdma/pvrdma.h:244:20: error: ‘PVRDMA_UAR_SRQ_OFFSET’ undeclared (first use in this function); did you mean ‘PVRDMA_UAR_CQ_OFFSET’?
  *(__le32 *)(uar + PVRDMA_UAR_SRQ_OFFSET) = htole32(value);
                    ^~~~~~~~~~~~~~~~~~~~~
                    PVRDMA_UAR_CQ_OFFSET
../providers/vmw_pvrdma/pvrdma.h:244:20: note: each undeclared identifier is reported only once for each function it appears in
../providers/vmw_pvrdma/qp.c: In function ‘pvrdma_create_srq’:
../providers/vmw_pvrdma/qp.c:141:12: error: ‘struct pvrdma_create_srq’ has no member named ‘buf_size’; did you mean ‘buf_addr’?
  cmd.udata.buf_size = srq->buf.length;
            ^~~~~~~~
            buf_addr
../providers/vmw_pvrdma/qp.c: In function ‘pvrdma_post_srq_recv’:
../providers/vmw_pvrdma/qp.c:704:10: error: ‘PVRDMA_UAR_SRQ_RECV’ undeclared (first use in this function); did you mean ‘PVRDMA_UAR_QP_RECV’?
          PVRDMA_UAR_SRQ_RECV | srq->srqn);
          ^~~~~~~~~~~~~~~~~~~
          PVRDMA_UAR_QP_RECV
[4/81] Building C object providers/vmw_pvrdma/CMakeFiles/vmw_pvrdma-rdmav16.dir/verbs.c.o
FAILED: providers/vmw_pvrdma/CMakeFiles/vmw_pvrdma-rdmav16.dir/verbs.c.o
/usr/lib/ccache/bin/cc -Dvmw_pvrdma_rdmav16_EXPORTS -Iinclude -I/usr/include/libnl3 -Wall -Wextra -Wno-sign-compare -Wno-unused-parameter -Wmissing-prototypes -Wmissing-declarations -Wwrite-strings -Wformat=2 -Wshadow -Wstrict-prototypes -Wold-style-definition -Wredundant-decls -O2 -g  -fPIC   -std=gnu11 -MD -MT providers/vmw_pvrdma/CMakeFiles/vmw_pvrdma-rdmav16.dir/verbs.c.o -MF providers/vmw_pvrdma/CMakeFiles/vmw_pvrdma-rdmav16.dir/verbs.c.o.d -o providers/vmw_pvrdma/CMakeFiles/vmw_pvrdma-rdmav16.dir/verbs.c.o   -c ../providers/vmw_pvrdma/verbs.c
In file included from ../providers/vmw_pvrdma/verbs.c:47:0:
../providers/vmw_pvrdma/pvrdma.h: In function ‘pvrdma_write_uar_srq’:
../providers/vmw_pvrdma/pvrdma.h:244:20: error: ‘PVRDMA_UAR_SRQ_OFFSET’ undeclared (first use in this function); did you mean ‘PVRDMA_UAR_CQ_OFFSET’?
  *(__le32 *)(uar + PVRDMA_UAR_SRQ_OFFSET) = htole32(value);
                    ^~~~~~~~~~~~~~~~~~~~~
                    PVRDMA_UAR_CQ_OFFSET
../providers/vmw_pvrdma/pvrdma.h:244:20: note: each undeclared identifier is reported only once for each function it appears in
[5/81] Building C object providers/vmw_pvrdma/CMakeFiles/vmw_pvrdma-rdmav16.dir/cq.c.o
FAILED: providers/vmw_pvrdma/CMakeFiles/vmw_pvrdma-rdmav16.dir/cq.c.o
/usr/lib/ccache/bin/cc -Dvmw_pvrdma_rdmav16_EXPORTS -Iinclude -I/usr/include/libnl3 -Wall -Wextra -Wno-sign-compare -Wno-unused-parameter -Wmissing-prototypes -Wmissing-declarations -Wwrite-strings -Wformat=2 -Wshadow -Wstrict-prototypes -Wold-style-definition -Wredundant-decls -O2 -g  -fPIC   -std=gnu11 -MD -MT providers/vmw_pvrdma/CMakeFiles/vmw_pvrdma-rdmav16.dir/cq.c.o -MF providers/vmw_pvrdma/CMakeFiles/vmw_pvrdma-rdmav16.dir/cq.c.o.d -o providers/vmw_pvrdma/CMakeFiles/vmw_pvrdma-rdmav16.dir/cq.c.o   -c ../providers/vmw_pvrdma/cq.c
In file included from ../providers/vmw_pvrdma/cq.c:48:0:
../providers/vmw_pvrdma/pvrdma.h: In function ‘pvrdma_write_uar_srq’:
../providers/vmw_pvrdma/pvrdma.h:244:20: error: ‘PVRDMA_UAR_SRQ_OFFSET’ undeclared (first use in this function); did you mean ‘PVRDMA_UAR_CQ_OFFSET’?
  *(__le32 *)(uar + PVRDMA_UAR_SRQ_OFFSET) = htole32(value);
                    ^~~~~~~~~~~~~~~~~~~~~
                    PVRDMA_UAR_CQ_OFFSET
../providers/vmw_pvrdma/pvrdma.h:244:20: note: each undeclared identifier is reported only once for each function it appears in
[6/81] Building C object providers/ipathverbs/CMakeFiles/ipathverbs-rdmav16.dir/ipathverbs.c.o
[7/81] Building C object providers/ipathverbs/CMakeFiles/ipathverbs-rdmav16.dir/verbs.c.o
ninja: build stopped: subcommand failed.
Thanks

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: vmw_pvrdma: Add SRQ support - broke rdma-core
       [not found] ` <20171114085143.GS18825-U/DQcQFIOTAAJjI8aNfphQ@public.gmane.org>
@ 2017-11-14 18:27   ` Leon Romanovsky
       [not found]     ` <20171114182720.GC18825-U/DQcQFIOTAAJjI8aNfphQ@public.gmane.org>
  0 siblings, 1 reply; 11+ messages in thread
From: Leon Romanovsky @ 2017-11-14 18:27 UTC (permalink / raw)
  To: Bryan Tan
  Cc: RDMA mailing list, Doug Ledford, Jason Gunthorpe, Benjamin Drung

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

On Tue, Nov 14, 2017 at 10:51:43AM +0200, Leon Romanovsky wrote:
> The commit 25f07300bfb7 ("vmw_pvrdma: Add SRQ support") causes to build
> failures in Travis CI [1] and on my system too [2].
>
> Please fix.

I reverted the commit, please fix compilation error and resubmit.

Commit: 32bf4efb8605 ("Revert "vmw_pvrdma: Add SRQ support" because it causes build failures")
Thanks

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: vmw_pvrdma: Add SRQ support - broke rdma-core
       [not found]     ` <20171114182720.GC18825-U/DQcQFIOTAAJjI8aNfphQ@public.gmane.org>
@ 2017-11-14 19:00       ` Leon Romanovsky
       [not found]         ` <20171114190029.GI18825-U/DQcQFIOTAAJjI8aNfphQ@public.gmane.org>
  0 siblings, 1 reply; 11+ messages in thread
From: Leon Romanovsky @ 2017-11-14 19:00 UTC (permalink / raw)
  To: Bryan Tan
  Cc: RDMA mailing list, Doug Ledford, Jason Gunthorpe, Benjamin Drung

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

On Tue, Nov 14, 2017 at 08:27:20PM +0200, Leon Romanovsky wrote:
> On Tue, Nov 14, 2017 at 10:51:43AM +0200, Leon Romanovsky wrote:
> > The commit 25f07300bfb7 ("vmw_pvrdma: Add SRQ support") causes to build
> > failures in Travis CI [1] and on my system too [2].
> >
> > Please fix.
>
> I reverted the commit, please fix compilation error and resubmit.
>
> Commit: 32bf4efb8605 ("Revert "vmw_pvrdma: Add SRQ support" because it causes build failures")

Just as a pointer, it fails on the systems with kernel's include/uapi/rdma/vmw_pvrdma-abi.h
file available, so fixup is not applied but without your definition yet.

> Thanks



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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: vmw_pvrdma: Add SRQ support - broke rdma-core
       [not found]         ` <20171114190029.GI18825-U/DQcQFIOTAAJjI8aNfphQ@public.gmane.org>
@ 2017-11-14 19:25           ` Bryan Tan
       [not found]             ` <20171114192510.GA6083-qXbCdz4EeRo1jLI2hToXVI42T8aCTgcwy4vvyvUx+exJXi8ZT2ovy+oDBWuYMCC/JZORHMmSJCU@public.gmane.org>
  0 siblings, 1 reply; 11+ messages in thread
From: Bryan Tan @ 2017-11-14 19:25 UTC (permalink / raw)
  To: Leon Romanovsky
  Cc: RDMA mailing list, Doug Ledford, Jason Gunthorpe, Benjamin Drung

On Tue, Nov 14, 2017 at 09:00:29PM +0200, Leon Romanovsky wrote:
> On Tue, Nov 14, 2017 at 08:27:20PM +0200, Leon Romanovsky wrote:
> > On Tue, Nov 14, 2017 at 10:51:43AM +0200, Leon Romanovsky wrote:
> > > The commit 25f07300bfb7 ("vmw_pvrdma: Add SRQ support") causes to build
> > > failures in Travis CI [1] and on my system too [2].
> > >
> > > Please fix.
> >
> > I reverted the commit, please fix compilation error and resubmit.
> >
> > Commit: 32bf4efb8605 ("Revert "vmw_pvrdma: Add SRQ support" because it causes build failures")
> 
> Just as a pointer, it fails on the systems with kernel's include/uapi/rdma/vmw_pvrdma-abi.h
> file available, so fixup is not applied but without your definition yet.

Thanks for the notice, I should have tried building on a kernel without
the updated ABI header. We had a question though, what would be the best
approach to fix this? We didn't bump up the ABI version after discussion
on the pull request. Is there a way for the fixup to be included if
there is an ABI version mismatch, or if a macro is not defined (in this
case, PVRDMA_UAR_SRQ_OFFSET)? I thought that we'd use the fixup over the
ABI header exposed by the kernel, but my assumption there was wrong.

> > Thanks
> 
> 


--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: vmw_pvrdma: Add SRQ support - broke rdma-core
       [not found]             ` <20171114192510.GA6083-qXbCdz4EeRo1jLI2hToXVI42T8aCTgcwy4vvyvUx+exJXi8ZT2ovy+oDBWuYMCC/JZORHMmSJCU@public.gmane.org>
@ 2017-11-14 19:28               ` Jason Gunthorpe
       [not found]                 ` <20171114192858.GM4263-uk2M96/98Pc@public.gmane.org>
  0 siblings, 1 reply; 11+ messages in thread
From: Jason Gunthorpe @ 2017-11-14 19:28 UTC (permalink / raw)
  To: Bryan Tan
  Cc: Leon Romanovsky, RDMA mailing list, Doug Ledford, Benjamin Drung

On Tue, Nov 14, 2017 at 11:25:12AM -0800, Bryan Tan wrote:
> On Tue, Nov 14, 2017 at 09:00:29PM +0200, Leon Romanovsky wrote:
> > On Tue, Nov 14, 2017 at 08:27:20PM +0200, Leon Romanovsky wrote:
> > > On Tue, Nov 14, 2017 at 10:51:43AM +0200, Leon Romanovsky wrote:
> > > > The commit 25f07300bfb7 ("vmw_pvrdma: Add SRQ support") causes to build
> > > > failures in Travis CI [1] and on my system too [2].
> > > >
> > > > Please fix.
> > >
> > > I reverted the commit, please fix compilation error and resubmit.
> > >
> > > Commit: 32bf4efb8605 ("Revert "vmw_pvrdma: Add SRQ support" because it causes build failures")
> > 
> > Just as a pointer, it fails on the systems with kernel's include/uapi/rdma/vmw_pvrdma-abi.h
> > file available, so fixup is not applied but without your definition yet.
> 
> Thanks for the notice, I should have tried building on a kernel without
> the updated ABI header. We had a question though, what would be the best
> approach to fix this?

The cmake scheme always uses the distro header if the distro header
passes the 'test'. Till now the test for pvrdma.h was existance only.

Now that you depend on new stuff, you have to update the test to see
if the header is new enough.

> We didn't bump up the ABI version after discussion on the pull
> request.

Correct.

> there is an ABI version mismatch, or if a macro is not defined (in this
> case, PVRDMA_UAR_SRQ_OFFSET)? I thought that we'd use the fixup over the
> ABI header exposed by the kernel, but my assumption there was wrong.

Look in buildlib/RDMA_LinuxHeaders.cmake and copy something like the
rdma/rdma_netlink.h line, but test for PVRDMA_UAR_SRQ_OFFSET.

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: vmw_pvrdma: Add SRQ support - broke rdma-core
       [not found]                 ` <20171114192858.GM4263-uk2M96/98Pc@public.gmane.org>
@ 2017-11-15  2:49                   ` Bryan Tan
       [not found]                     ` <20171115024924.GA23574-qXbCdz4EeRo1jLI2hToXVI42T8aCTgcwy4vvyvUx+exJXi8ZT2ovy+oDBWuYMCC/JZORHMmSJCU@public.gmane.org>
  0 siblings, 1 reply; 11+ messages in thread
From: Bryan Tan @ 2017-11-15  2:49 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: Leon Romanovsky, RDMA mailing list, Doug Ledford, Benjamin Drung

On Tue, Nov 14, 2017 at 12:28:58PM -0700, Jason Gunthorpe wrote:
> On Tue, Nov 14, 2017 at 11:25:12AM -0800, Bryan Tan wrote:
> > On Tue, Nov 14, 2017 at 09:00:29PM +0200, Leon Romanovsky wrote:
> > > On Tue, Nov 14, 2017 at 08:27:20PM +0200, Leon Romanovsky wrote:
> > > > On Tue, Nov 14, 2017 at 10:51:43AM +0200, Leon Romanovsky wrote:
> > > > > The commit 25f07300bfb7 ("vmw_pvrdma: Add SRQ support") causes to build
> > > > > failures in Travis CI [1] and on my system too [2].
> > > > >
> > > > > Please fix.
> > > >
> > > > I reverted the commit, please fix compilation error and resubmit.
> > > >
> > > > Commit: 32bf4efb8605 ("Revert "vmw_pvrdma: Add SRQ support" because it causes build failures")
> > > 
> > > Just as a pointer, it fails on the systems with kernel's include/uapi/rdma/vmw_pvrdma-abi.h
> > > file available, so fixup is not applied but without your definition yet.
> > 
> > Thanks for the notice, I should have tried building on a kernel without
> > the updated ABI header. We had a question though, what would be the best
> > approach to fix this?
> 
> The cmake scheme always uses the distro header if the distro header
> passes the 'test'. Till now the test for pvrdma.h was existance only.
> 
> Now that you depend on new stuff, you have to update the test to see
> if the header is new enough.
> 
> > We didn't bump up the ABI version after discussion on the pull
> > request.
> 
> Correct.
> 
> > there is an ABI version mismatch, or if a macro is not defined (in this
> > case, PVRDMA_UAR_SRQ_OFFSET)? I thought that we'd use the fixup over the
> > ABI header exposed by the kernel, but my assumption there was wrong.
> 
> Look in buildlib/RDMA_LinuxHeaders.cmake and copy something like the
> rdma/rdma_netlink.h line, but test for PVRDMA_UAR_SRQ_OFFSET.

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.

> 
> Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: vmw_pvrdma: Add SRQ support - broke rdma-core
       [not found]                     ` <20171115024924.GA23574-qXbCdz4EeRo1jLI2hToXVI42T8aCTgcwy4vvyvUx+exJXi8ZT2ovy+oDBWuYMCC/JZORHMmSJCU@public.gmane.org>
@ 2017-11-15  3:15                       ` Jason Gunthorpe
       [not found]                         ` <20171115031526.GJ25894-uk2M96/98Pc@public.gmane.org>
  0 siblings, 1 reply; 11+ messages in thread
From: Jason Gunthorpe @ 2017-11-15  3:15 UTC (permalink / raw)
  To: Bryan Tan
  Cc: Leon Romanovsky, RDMA mailing list, Doug Ledford, Benjamin Drung

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.

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: vmw_pvrdma: Add SRQ support - broke rdma-core
       [not found]                         ` <20171115031526.GJ25894-uk2M96/98Pc@public.gmane.org>
@ 2017-11-15 22:10                           ` Bryan Tan
       [not found]                             ` <20171115221052.GA16946-qXbCdz4EeRo1jLI2hToXVI42T8aCTgcwy4vvyvUx+exJXi8ZT2ovy+oDBWuYMCC/JZORHMmSJCU@public.gmane.org>
  0 siblings, 1 reply; 11+ messages in thread
From: Bryan Tan @ 2017-11-15 22:10 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: Leon Romanovsky, RDMA mailing list, Doug Ledford, Benjamin Drung

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.

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
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: vmw_pvrdma: Add SRQ support - broke rdma-core
       [not found]                             ` <20171115221052.GA16946-qXbCdz4EeRo1jLI2hToXVI42T8aCTgcwy4vvyvUx+exJXi8ZT2ovy+oDBWuYMCC/JZORHMmSJCU@public.gmane.org>
@ 2017-11-16  5:21                               ` Leon Romanovsky
       [not found]                                 ` <20171116052119.GF18825-U/DQcQFIOTAAJjI8aNfphQ@public.gmane.org>
  0 siblings, 1 reply; 11+ messages in thread
From: Leon Romanovsky @ 2017-11-16  5:21 UTC (permalink / raw)
  To: Bryan Tan
  Cc: Jason Gunthorpe, RDMA mailing list, Doug Ledford, Benjamin Drung

[-- 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 --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: vmw_pvrdma: Add SRQ support - broke rdma-core
       [not found]                                 ` <20171116052119.GF18825-U/DQcQFIOTAAJjI8aNfphQ@public.gmane.org>
@ 2017-11-16 18:54                                   ` Bryan Tan
       [not found]                                     ` <20171116185450.GB16946-qXbCdz4EeRo1jLI2hToXVI42T8aCTgcwy4vvyvUx+exJXi8ZT2ovy+oDBWuYMCC/JZORHMmSJCU@public.gmane.org>
  0 siblings, 1 reply; 11+ messages in thread
From: Bryan Tan @ 2017-11-16 18:54 UTC (permalink / raw)
  To: Leon Romanovsky
  Cc: Jason Gunthorpe, RDMA mailing list, Doug Ledford, Benjamin Drung

On Thu, Nov 16, 2017 at 07:21:19AM +0200, Leon Romanovsky wrote:
> 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.

I've updated the PR after taking in Jason's suggestion of swapping the
order of the patches that I missed, and ran travis builds on both
commits. The travis build on the PR is failing, but it looks like the
top of tree travis build is failing as well.

> 
> >
> > 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


--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: vmw_pvrdma: Add SRQ support - broke rdma-core
       [not found]                                     ` <20171116185450.GB16946-qXbCdz4EeRo1jLI2hToXVI42T8aCTgcwy4vvyvUx+exJXi8ZT2ovy+oDBWuYMCC/JZORHMmSJCU@public.gmane.org>
@ 2017-11-17 23:37                                       ` Jason Gunthorpe
  0 siblings, 0 replies; 11+ messages in thread
From: Jason Gunthorpe @ 2017-11-17 23:37 UTC (permalink / raw)
  To: Bryan Tan
  Cc: Leon Romanovsky, RDMA mailing list, Doug Ledford, Benjamin Drung

On Thu, Nov 16, 2017 at 10:54:51AM -0800, Bryan Tan wrote:

> I've updated the PR after taking in Jason's suggestion of swapping the
> order of the patches that I missed, and ran travis builds on both
> commits. The travis build on the PR is failing, but it looks like the
> top of tree travis build is failing as well.

Please move the test update to the prior patch, I left a note on
github to be clear.

Travis should be fixed right away.

Please force push the change and we should be able to merge it!

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2017-11-17 23:37 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-14  8:51 vmw_pvrdma: Add SRQ support - broke rdma-core Leon Romanovsky
     [not found] ` <20171114085143.GS18825-U/DQcQFIOTAAJjI8aNfphQ@public.gmane.org>
2017-11-14 18:27   ` Leon Romanovsky
     [not found]     ` <20171114182720.GC18825-U/DQcQFIOTAAJjI8aNfphQ@public.gmane.org>
2017-11-14 19:00       ` Leon Romanovsky
     [not found]         ` <20171114190029.GI18825-U/DQcQFIOTAAJjI8aNfphQ@public.gmane.org>
2017-11-14 19:25           ` Bryan Tan
     [not found]             ` <20171114192510.GA6083-qXbCdz4EeRo1jLI2hToXVI42T8aCTgcwy4vvyvUx+exJXi8ZT2ovy+oDBWuYMCC/JZORHMmSJCU@public.gmane.org>
2017-11-14 19:28               ` Jason Gunthorpe
     [not found]                 ` <20171114192858.GM4263-uk2M96/98Pc@public.gmane.org>
2017-11-15  2:49                   ` Bryan Tan
     [not found]                     ` <20171115024924.GA23574-qXbCdz4EeRo1jLI2hToXVI42T8aCTgcwy4vvyvUx+exJXi8ZT2ovy+oDBWuYMCC/JZORHMmSJCU@public.gmane.org>
2017-11-15  3:15                       ` Jason Gunthorpe
     [not found]                         ` <20171115031526.GJ25894-uk2M96/98Pc@public.gmane.org>
2017-11-15 22:10                           ` Bryan Tan
     [not found]                             ` <20171115221052.GA16946-qXbCdz4EeRo1jLI2hToXVI42T8aCTgcwy4vvyvUx+exJXi8ZT2ovy+oDBWuYMCC/JZORHMmSJCU@public.gmane.org>
2017-11-16  5:21                               ` Leon Romanovsky
     [not found]                                 ` <20171116052119.GF18825-U/DQcQFIOTAAJjI8aNfphQ@public.gmane.org>
2017-11-16 18:54                                   ` Bryan Tan
     [not found]                                     ` <20171116185450.GB16946-qXbCdz4EeRo1jLI2hToXVI42T8aCTgcwy4vvyvUx+exJXi8ZT2ovy+oDBWuYMCC/JZORHMmSJCU@public.gmane.org>
2017-11-17 23:37                                       ` Jason Gunthorpe

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.