| Message ID | 20150806162438.GA27608@infradead.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
On 8/6/2015 7:24 PM, Christoph Hellwig wrote: > I've pushed out a new version. Updates: > > - the ib_recv_wr change Bart notices has been fixed. > - iser and isert have been converted Thanks Christoph! > - the handling of the embedded WR in the qib software queue entry > has been fixed. > > Which means we're basically done now and the patch could use > broader testing. pulling it now... > > The full patch will be too much for the list again, so here is the > git commit: > > http://git.infradead.org/users/hch/scsi.git/commitdiff/a0027ed00fc3ae2686d8a843a724b50597115a71 > > ib_vers.h diff below: > > diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h > index 0940051..2f2efdd 100644 > --- a/include/rdma/ib_verbs.h > +++ b/include/rdma/ib_verbs.h > @@ -1100,54 +1100,94 @@ struct ib_send_wr { > __be32 imm_data; > u32 invalidate_rkey; > } ex; > - union { > - struct { > - u64 remote_addr; > - u32 rkey; > - } rdma; > - struct { > - u64 remote_addr; > - u64 compare_add; > - u64 swap; > - u64 compare_add_mask; > - u64 swap_mask; > - u32 rkey; > - } atomic; > - struct { > - struct ib_ah *ah; > - void *header; > - int hlen; > - int mss; > - u32 remote_qpn; > - u32 remote_qkey; > - u16 pkey_index; /* valid for GSI only */ > - u8 port_num; /* valid for DR SMPs on switch only */ > - } ud; > - struct { > - u64 iova_start; > - struct ib_fast_reg_page_list *page_list; > - unsigned int page_shift; > - unsigned int page_list_len; > - u32 length; > - int access_flags; > - u32 rkey; > - } fast_reg; > - struct { > - struct ib_mw *mw; > - /* The new rkey for the memory window. */ > - u32 rkey; > - struct ib_mw_bind_info bind_info; > - } bind_mw; > - struct { > - struct ib_sig_attrs *sig_attrs; > - struct ib_mr *sig_mr; > - int access_flags; > - struct ib_sge *prot; > - } sig_handover; > - } wr; > u32 xrc_remote_srq_num; /* XRC TGT QPs only */ I agree that this is a shame to keep in here for everyone to carry... The only driver I've seen supporting XRC is mlx5 with no consumers. If people are reluctant to remove it, you can put it in ib_xrc_send_wr or something... -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 8/6/2015 11:24 AM, Christoph Hellwig wrote: > I've pushed out a new version. Updates: > > - the ib_recv_wr change Bart notices has been fixed. > - iser and isert have been converted > - the handling of the embedded WR in the qib software queue entry > has been fixed. > > Which means we're basically done now and the patch could use > broader testing. > > The full patch will be too much for the list again, so here is the > git commit: > > http://git.infradead.org/users/hch/scsi.git/commitdiff/a0027ed00fc3ae2686d8a843a724b50597115a71 > Hey Christoph, You missed amso1100 (and probably ipath) that have been moved to drivers/staging... CC [M] drivers/staging/amso1100/c2_qp.o drivers/staging/amso1100/c2_qp.c: In function âc2_post_sendâ: drivers/staging/amso1100/c2_qp.c:863: error: âstruct ib_send_wrâ has no member named âwrâ I'll disable them from my config so I can test your code on cxgb4, but I wanted to let you know... Steve. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Aug 06, 2015 at 12:04:32PM -0500, Steve Wise wrote: > You missed amso1100 (and probably ipath) that have been moved to > drivers/staging... Driver/staging isn't considered in tree for global API change perspective, so I didn't bother with all these staging drivers. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Do you see value in dividing ib_ud _wr into ib_ud_wr and ib_ud_gsi_wr to save 4 bytes? On Thu, Aug 6, 2015 at 9:54 PM, Christoph Hellwig <hch@infradead.org> wrote: > I've pushed out a new version. Updates: > > - the ib_recv_wr change Bart notices has been fixed. > - iser and isert have been converted > - the handling of the embedded WR in the qib software queue entry > has been fixed. > > Which means we're basically done now and the patch could use > broader testing. > > The full patch will be too much for the list again, so here is the > git commit: > > http://git.infradead.org/users/hch/scsi.git/commitdiff/a0027ed00fc3ae2686d8a843a724b50597115a71 > > ib_vers.h diff below: > > diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h > index 0940051..2f2efdd 100644 > --- a/include/rdma/ib_verbs.h > +++ b/include/rdma/ib_verbs.h > @@ -1100,54 +1100,94 @@ struct ib_send_wr { > __be32 imm_data; > u32 invalidate_rkey; > } ex; > - union { > - struct { > - u64 remote_addr; > - u32 rkey; > - } rdma; > - struct { > - u64 remote_addr; > - u64 compare_add; > - u64 swap; > - u64 compare_add_mask; > - u64 swap_mask; > - u32 rkey; > - } atomic; > - struct { > - struct ib_ah *ah; > - void *header; > - int hlen; > - int mss; > - u32 remote_qpn; > - u32 remote_qkey; > - u16 pkey_index; /* valid for GSI only */ > - u8 port_num; /* valid for DR SMPs on switch only */ > - } ud; > - struct { > - u64 iova_start; > - struct ib_fast_reg_page_list *page_list; > - unsigned int page_shift; > - unsigned int page_list_len; > - u32 length; > - int access_flags; > - u32 rkey; > - } fast_reg; > - struct { > - struct ib_mw *mw; > - /* The new rkey for the memory window. */ > - u32 rkey; > - struct ib_mw_bind_info bind_info; > - } bind_mw; > - struct { > - struct ib_sig_attrs *sig_attrs; > - struct ib_mr *sig_mr; > - int access_flags; > - struct ib_sge *prot; > - } sig_handover; > - } wr; > u32 xrc_remote_srq_num; /* XRC TGT QPs only */ > }; > > +struct ib_rdma_wr { > + struct ib_send_wr wr; > + u64 remote_addr; > + u32 rkey; > +}; > + > +static inline struct ib_rdma_wr *rdma_wr(struct ib_send_wr *wr) > +{ > + return container_of(wr, struct ib_rdma_wr, wr); > +} > + > +struct ib_atomic_wr { > + struct ib_send_wr wr; > + u64 remote_addr; > + u64 compare_add; > + u64 swap; > + u64 compare_add_mask; > + u64 swap_mask; > + u32 rkey; > +}; > + > +static inline struct ib_atomic_wr *atomic_wr(struct ib_send_wr *wr) > +{ > + return container_of(wr, struct ib_atomic_wr, wr); > +} > + > +struct ib_ud_wr { > + struct ib_send_wr wr; > + struct ib_ah *ah; > + void *header; > + int hlen; > + int mss; > + u32 remote_qpn; > + u32 remote_qkey; > + u16 pkey_index; /* valid for GSI only */ > + u8 port_num; /* valid for DR SMPs on switch only */ > +}; > + > +static inline struct ib_ud_wr *ud_wr(struct ib_send_wr *wr) > +{ > + return container_of(wr, struct ib_ud_wr, wr); > +} > + > +struct ib_fast_reg_wr { > + struct ib_send_wr wr; > + u64 iova_start; > + struct ib_fast_reg_page_list *page_list; > + unsigned int page_shift; > + unsigned int page_list_len; > + u32 length; > + int access_flags; > + u32 rkey; > +}; > + > +static inline struct ib_fast_reg_wr *fast_reg_wr(struct ib_send_wr *wr) > +{ > + return container_of(wr, struct ib_fast_reg_wr, wr); > +} > + > +struct ib_bind_mw_wr { > + struct ib_send_wr wr; > + struct ib_mw *mw; > + /* The new rkey for the memory window. */ > + u32 rkey; > + struct ib_mw_bind_info bind_info; > +}; > + > +static inline struct ib_bind_mw_wr *bind_mw_wr(struct ib_send_wr *wr) > +{ > + return container_of(wr, struct ib_bind_mw_wr, wr); > +} > + > +struct ib_sig_handover_wr { > + struct ib_send_wr wr; > + struct ib_sig_attrs *sig_attrs; > + struct ib_mr *sig_mr; > + int access_flags; > + struct ib_sge *prot; > +}; > + > +static inline struct ib_sig_handover_wr *sig_handover_wr(struct ib_send_wr *wr) > +{ > + return container_of(wr, struct ib_sig_handover_wr, wr); > +} > + > struct ib_recv_wr { > struct ib_recv_wr *next; > u64 wr_id; > -- > To unsubscribe from this list: send the line "unsubscribe linux-rdma" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> -----Original Message----- > From: linux-rdma-owner@vger.kernel.org [mailto:linux-rdma-owner@vger.kernel.org] On Behalf Of Christoph Hellwig > Sent: Thursday, August 06, 2015 12:32 PM > To: Steve Wise > Cc: Christoph Hellwig; linux-rdma@vger.kernel.org; Sagi Grimberg > Subject: Re: [RFC] split struct ib_send_wr > > On Thu, Aug 06, 2015 at 12:04:32PM -0500, Steve Wise wrote: > > You missed amso1100 (and probably ipath) that have been moved to > > drivers/staging... > > Driver/staging isn't considered in tree for global API change > perspective, so I didn't bother with all these staging drivers. The kbuild test bot will probably catch this. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Aug 6, 2015, at 1:44 PM, Steve Wise <swise@opengridcomputing.com> wrote: > > >> -----Original Message----- >> From: linux-rdma-owner@vger.kernel.org [mailto:linux-rdma-owner@vger.kernel.org] On Behalf Of Christoph Hellwig >> Sent: Thursday, August 06, 2015 12:32 PM >> To: Steve Wise >> Cc: Christoph Hellwig; linux-rdma@vger.kernel.org; Sagi Grimberg >> Subject: Re: [RFC] split struct ib_send_wr >> >> On Thu, Aug 06, 2015 at 12:04:32PM -0500, Steve Wise wrote: >>> You missed amso1100 (and probably ipath) that have been moved to >>> drivers/staging... >> >> Driver/staging isn't considered in tree for global API change >> perspective, so I didn't bother with all these staging drivers. > > The kbuild test bot will probably catch this. Wondering if this means we'll have to drop ib_reg_phys_mr() removal until Lustre gets around to removing their call sites from the staging tree. -- Chuck Lever -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Aug 06, 2015 at 11:08:45PM +0530, Parav Pandit wrote: > Do you see value in dividing ib_ud _wr into ib_ud_wr and ib_ud_gsi_wr > to save 4 bytes? For now I just wanted to split along the lines of the existing unions. From looking at the various drivers splitting the GSI path might not be a bad idea, but it's not a priority for me. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Aug 06, 2015 at 12:44:42PM -0500, Steve Wise wrote: > > Driver/staging isn't considered in tree for global API change > > perspective, so I didn't bother with all these staging drivers. > > The kbuild test bot will probably catch this. It already did catch it for my tree, which is expected. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Aug 06, 2015 at 01:58:45PM -0400, Chuck Lever wrote: > Wondering if this means we'll have to drop ib_reg_phys_mr() > removal until Lustre gets around to removing their call sites > from the staging tree. Why? Just because the buildbot catches it? -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Aug 06, 2015 at 07:46:44PM +0300, Sagi Grimberg wrote: > I agree that this is a shame to keep in here for everyone to carry... > The only driver I've seen supporting XRC is mlx5 with no consumers. > > If people are reluctant to remove it, you can put it in ib_xrc_send_wr > or something... If'll send a patch to remove it, and then we can introduce ib_xrc_send_wr once users show up. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Aug 7, 2015, at 2:36 AM, Christoph Hellwig <hch@infradead.org> wrote: > On Thu, Aug 06, 2015 at 01:58:45PM -0400, Chuck Lever wrote: >> Wondering if this means we'll have to drop ib_reg_phys_mr() >> removal until Lustre gets around to removing their call sites >> from the staging tree. > > Why? Just because the buildbot catches it? If bot barking doesn't bother anyone, then I'll keep the removal patch. For some such a complaint might be grounds for rejecting the patch. -- Chuck Lever -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Aug 07, 2015 at 10:17:18AM -0400, Chuck Lever wrote: > If bot barking doesn't bother anyone, then I'll keep the removal patch. > For some such a complaint might be grounds for rejecting the patch. If it's (a) in tree proper and (b) not one of the rare false positives I would consider it a reason for rejection as well. But this is the staging tree we're talking about. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 8/6/2015 11:24 AM, Christoph Hellwig wrote: > I've pushed out a new version. Updates: > > - the ib_recv_wr change Bart notices has been fixed. > - iser and isert have been converted > - the handling of the embedded WR in the qib software queue entry > has been fixed. > > Which means we're basically done now and the patch could use > broader testing. > > The full patch will be too much for the list again, so here is the > git commit: > > http://git.infradead.org/users/hch/scsi.git/commitdiff/a0027ed00fc3ae2686d8a843a724b50597115a71 > This tests ok over iwarp/cxgb4 using NFSRDMA and user mode RDMA apps. Tested-by: Steve Wise <swise@opengridcomputing.com> -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 08/06/2015 09:24 AM, Christoph Hellwig wrote: > I've pushed out a new version. Updates: > > - the ib_recv_wr change Bart notices has been fixed. > - iser and isert have been converted > - the handling of the embedded WR in the qib software queue entry > has been fixed. > > Which means we're basically done now and the patch could use > broader testing. > > The full patch will be too much for the list again, so here is the > git commit: > > http://git.infradead.org/users/hch/scsi.git/commitdiff/a0027ed00fc3ae2686d8a843a724b50597115a71 > > ib_vers.h diff below: > [ ... ] For the SRP initiator and target changes: Reviewed-by: Bart Van Assche <bart.vanassche@sandisk.com> -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 8/6/2015 7:24 PM, Christoph Hellwig wrote: > I've pushed out a new version. Updates: > > - the ib_recv_wr change Bart notices has been fixed. > - iser and isert have been converted > - the handling of the embedded WR in the qib software queue entry > has been fixed. > > Which means we're basically done now and the patch could use > broader testing. This went very smooth with iser, isert and ipoib... You can add: Tested-by: Sagi Grimberg <sagig@mellanox.com> Just a nit that I've noticed, in mlx4 set_fmr_seg params are not aligned to the parenthesis (maybe in other locations too but I haven't noticed such...) -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 08/07/2015 10:19 AM, Christoph Hellwig wrote: > On Fri, Aug 07, 2015 at 10:17:18AM -0400, Chuck Lever wrote: >> If bot barking doesn't bother anyone, then I'll keep the removal patch. >> For some such a complaint might be grounds for rejecting the patch. > > If it's (a) in tree proper and (b) not one of the rare false positives I > would consider it a reason for rejection as well. But this is the > staging tree we're talking about. I want to put my $.02 in on this. The staging tree has meant different things over time. In particular, it didn't used to be a place where "to be removed" drivers went to hang out for a few releases before finally being removed entirely. I suspect this policy of not touching staging drivers with tree wide API changes pre-dates the policy of using staging as an intermediate step in removal. It would seem to me that saying we are going to remove a driver in 4.6 and moving it to staging for that purpose, and then immediately breaking it so it doesn't compile, is not compatible with the goals of orderly device driver removal (namely: alerting people to the upcoming change, waiting a reasonable period of time for objections/feedback, and then removing the driver if no one brings forth a case for it to stay in the kernel). It effectively becomes an immediate removal. For that reason, I can't say that I agree with this policy of skipping staging drivers for API updates, at least as it applies to drivers that were in the tree proper and are in staging now as part of their orderly removal process. I can't say as I really agree with the policy for drivers coming in through staging either unless the authors of the driver are allowing it to languish and not pursuing their TODO list. In any case, I don't expect to have stuff from the RDMA core in staging for an extended amount of time. But right now there is, and for the way I'm using the staging area, *I* care if your patch breaks the drivers that are there.
> On Aug 12, 2015, at 6:45 PM, Doug Ledford <dledford@redhat.com> wrote: > >> On 08/07/2015 10:19 AM, Christoph Hellwig wrote: >>> On Fri, Aug 07, 2015 at 10:17:18AM -0400, Chuck Lever wrote: >>> If bot barking doesn't bother anyone, then I'll keep the removal patch. >>> For some such a complaint might be grounds for rejecting the patch. >> >> If it's (a) in tree proper and (b) not one of the rare false positives I >> would consider it a reason for rejection as well. But this is the >> staging tree we're talking about. > > I want to put my $.02 in on this. The staging tree has meant different > things over time. In particular, it didn't used to be a place where "to > be removed" drivers went to hang out for a few releases before finally > being removed entirely. I suspect this policy of not touching staging > drivers with tree wide API changes pre-dates the policy of using staging > as an intermediate step in removal. It would seem to me that saying we > are going to remove a driver in 4.6 and moving it to staging for that > purpose, and then immediately breaking it so it doesn't compile, is not > compatible with the goals of orderly device driver removal (namely: > alerting people to the upcoming change, waiting a reasonable period of > time for objections/feedback, and then removing the driver if no one > brings forth a case for it to stay in the kernel). It effectively > becomes an immediate removal. For that reason, I can't say that I agree > with this policy of skipping staging drivers for API updates, at least > as it applies to drivers that were in the tree proper and are in staging > now as part of their orderly removal process. > > I can't say as I really agree with the policy for drivers coming in > through staging either unless the authors of the driver are allowing it > to languish and not pursuing their TODO list. > > In any case, I don't expect to have stuff from the RDMA core in staging > for an extended amount of time. But right now there is, and for the way > I'm using the staging area, *I* care if your patch breaks the drivers > that are there. That makes sense, but you already Acked the change that breaks Lustre, and it's going in through the NFS tree. Are you changing that to a NAK?-- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Aug 12, 2015 at 07:24:44PM -0700, Chuck Lever wrote: > That makes sense, but you already Acked the change that breaks Lustre, > and it's going in through the NFS tree. Are you changing that to a NAK? It seems like Doug was mostly concened about to be removed drivers. I defintively refuse to fix Lustre for anything I tough because it's such a giant mess with uses just about every major subsystem in an incorrect way. Doug: was your mail a request to fix up the two de-staged drivers? I'm happy to do that if you're fine with the patch in general. amso1100 should be trivial anyway, while ipath is a mess, just like the new intel driver with the third copy of the soft ib stack. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 08/13/2015 01:54 AM, Christoph Hellwig wrote: > On Wed, Aug 12, 2015 at 07:24:44PM -0700, Chuck Lever wrote: >> That makes sense, but you already Acked the change that breaks Lustre, >> and it's going in through the NFS tree. Are you changing that to a NAK? No. Lustre fits in my "languishing in the staging tree" category. > It seems like Doug was mostly concened about to be removed drivers. > I defintively refuse to fix Lustre for anything I tough because it's > such a giant mess with uses just about every major subsystem in > an incorrect way. > > Doug: was your mail a request to fix up the two de-staged drivers? > I'm happy to do that if you're fine with the patch in general. amso1100 > should be trivial anyway, while ipath is a mess, just like the new intel > driver with the third copy of the soft ib stack. Correct.
On Wed, Aug 12, 2015 at 08:24:49PM +0300, Sagi Grimberg wrote: > Just a nit that I've noticed, in mlx4 set_fmr_seg params are not > aligned to the parenthesis (maybe in other locations too but I haven't > noticed such...) This is just using a normal two tab indent for continued function parameters.. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Aug 13, 2015 at 09:07:14AM -0400, Doug Ledford wrote: > > Doug: was your mail a request to fix up the two de-staged drivers? > > I'm happy to do that if you're fine with the patch in general. amso1100 > > should be trivial anyway, while ipath is a mess, just like the new intel > > driver with the third copy of the soft ib stack. > > Correct. http://git.infradead.org/users/hch/rdma.git/commitdiff/efb2b0f21645b9caabcce955481ab6966e52ad90 contains the updates for ipath and amso1100, as well as the reviewed-by and tested-by tags. Note that for now I've skipped the new intel hfi1 driver as updating two of the soft ib codebases already was tiresome enough. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Aug 13, 2015, at 9:04 AM, Christoph Hellwig <hch@infradead.org> wrote: > On Thu, Aug 13, 2015 at 09:07:14AM -0400, Doug Ledford wrote: >>> Doug: was your mail a request to fix up the two de-staged drivers? >>> I'm happy to do that if you're fine with the patch in general. amso1100 >>> should be trivial anyway, while ipath is a mess, just like the new intel >>> driver with the third copy of the soft ib stack. >> >> Correct. > > http://git.infradead.org/users/hch/rdma.git/commitdiff/efb2b0f21645b9caabcce955481ab6966e52ad90 > > contains the updates for ipath and amso1100, as well as the reviewed-by > and tested-by tags. > > Note that for now I've skipped the new intel hfi1 driver as updating > two of the soft ib codebases already was tiresome enough. This looks like a straightforward mechanical change. For the hunks under net/sunrpc/xprtrdma/ : Reviewed-by: Chuck Lever <chuck.lever@oracle.com> -- Chuck Lever -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Aug 13, 2015 at 09:04:39AM -0700, Christoph Hellwig wrote: > On Thu, Aug 13, 2015 at 09:07:14AM -0400, Doug Ledford wrote: > > > Doug: was your mail a request to fix up the two de-staged drivers? > > > I'm happy to do that if you're fine with the patch in general. amso1100 > > > should be trivial anyway, while ipath is a mess, just like the new intel > > > driver with the third copy of the soft ib stack. > > > > Correct. > > http://git.infradead.org/users/hch/rdma.git/commitdiff/efb2b0f21645b9caabcce955481ab6966e52ad90 > > contains the updates for ipath and amso1100, as well as the reviewed-by > and tested-by tags. The uverbs change needs to drop/move the original kmalloc: next = kmalloc(ALIGN(sizeof *next, sizeof (struct ib_sge)) + user_wr->num_sge * sizeof (struct ib_sge), GFP_KERNEL); It looks like it is leaking that allocation right now. Every path replaces next with the result of alloc_mr.. Noticed a couple of trailing whitespaces too.. Jason -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Aug 13, 2015 at 11:22:34AM -0600, Jason Gunthorpe wrote: > The uverbs change needs to drop/move the original kmalloc: > > next = kmalloc(ALIGN(sizeof *next, sizeof (struct ib_sge)) + > user_wr->num_sge * sizeof (struct ib_sge), > GFP_KERNEL); > > It looks like it is leaking that allocation right now. Every path > replaces next with the result of alloc_mr.. Thanks. It should be come and indeed was in my first version. Not sure how it sneaked in during a rebase. > Noticed a couple of trailing whitespaces too.. checkpatch found two of them, which I've fixed now. New version at: http://git.infradead.org/users/hch/rdma.git/commitdiff/5d7e6fa563dae32d4b6f63e29e3795717a545f11 -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Aug 13, 2015 at 10:53:54AM -0700, Christoph Hellwig wrote: > http://git.infradead.org/users/hch/rdma.git/commitdiff/5d7e6fa563dae32d4b6f63e29e3795717a545f11 For the core bits: Reviewed-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> Jason -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Aug 13, 2015 at 09:04:39AM -0700, Christoph Hellwig wrote: > > > I'm happy to do that if you're fine with the patch in general. amso1100 > > > should be trivial anyway, while ipath is a mess, just like the new intel > > > driver with the third copy of the soft ib stack. > > > > Correct. > > http://git.infradead.org/users/hch/rdma.git/commitdiff/efb2b0f21645b9caabcce955481ab6966e52ad90 > > contains the updates for ipath and amso1100, as well as the reviewed-by > and tested-by tags. > > Note that for now I've skipped the new intel hfi1 driver as updating > two of the soft ib codebases already was tiresome enough. Doug, is this good enough for merging? http://git.infradead.org/users/hch/rdma.git/shortlog/refs/heads/wr-cleanup is my always uptodate branch, it has collected all the reviews and tested-by tag I got on the list. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" 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/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h index 0940051..2f2efdd 100644 --- a/include/rdma/ib_verbs.h +++ b/include/rdma/ib_verbs.h @@ -1100,54 +1100,94 @@ struct ib_send_wr { __be32 imm_data; u32 invalidate_rkey; } ex; - union { - struct { - u64 remote_addr; - u32 rkey; - } rdma; - struct { - u64 remote_addr; - u64 compare_add; - u64 swap; - u64 compare_add_mask; - u64 swap_mask; - u32 rkey; - } atomic; - struct { - struct ib_ah *ah; - void *header; - int hlen; - int mss; - u32 remote_qpn; - u32 remote_qkey; - u16 pkey_index; /* valid for GSI only */ - u8 port_num; /* valid for DR SMPs on switch only */ - } ud; - struct { - u64 iova_start; - struct ib_fast_reg_page_list *page_list; - unsigned int page_shift; - unsigned int page_list_len; - u32 length; - int access_flags; - u32 rkey; - } fast_reg; - struct { - struct ib_mw *mw; - /* The new rkey for the memory window. */ - u32 rkey; - struct ib_mw_bind_info bind_info; - } bind_mw; - struct { - struct ib_sig_attrs *sig_attrs; - struct ib_mr *sig_mr; - int access_flags; - struct ib_sge *prot; - } sig_handover; - } wr; u32 xrc_remote_srq_num; /* XRC TGT QPs only */ }; +struct ib_rdma_wr { + struct ib_send_wr wr; + u64 remote_addr; + u32 rkey; +}; + +static inline struct ib_rdma_wr *rdma_wr(struct ib_send_wr *wr) +{ + return container_of(wr, struct ib_rdma_wr, wr); +} + +struct ib_atomic_wr { + struct ib_send_wr wr; + u64 remote_addr; + u64 compare_add; + u64 swap; + u64 compare_add_mask; + u64 swap_mask; + u32 rkey; +}; + +static inline struct ib_atomic_wr *atomic_wr(struct ib_send_wr *wr) +{ + return container_of(wr, struct ib_atomic_wr, wr); +} + +struct ib_ud_wr { + struct ib_send_wr wr; + struct ib_ah *ah; + void *header; + int hlen; + int mss; + u32 remote_qpn; + u32 remote_qkey; + u16 pkey_index; /* valid for GSI only */ + u8 port_num; /* valid for DR SMPs on switch only */ +}; + +static inline struct ib_ud_wr *ud_wr(struct ib_send_wr *wr) +{ + return container_of(wr, struct ib_ud_wr, wr); +} + +struct ib_fast_reg_wr { + struct ib_send_wr wr; + u64 iova_start; + struct ib_fast_reg_page_list *page_list; + unsigned int page_shift; + unsigned int page_list_len; + u32 length; + int access_flags; + u32 rkey; +}; + +static inline struct ib_fast_reg_wr *fast_reg_wr(struct ib_send_wr *wr) +{ + return container_of(wr, struct ib_fast_reg_wr, wr); +} + +struct ib_bind_mw_wr { + struct ib_send_wr wr; + struct ib_mw *mw; + /* The new rkey for the memory window. */ + u32 rkey; + struct ib_mw_bind_info bind_info; +}; + +static inline struct ib_bind_mw_wr *bind_mw_wr(struct ib_send_wr *wr) +{ + return container_of(wr, struct ib_bind_mw_wr, wr); +} + +struct ib_sig_handover_wr { + struct ib_send_wr wr; + struct ib_sig_attrs *sig_attrs; + struct ib_mr *sig_mr; + int access_flags; + struct ib_sge *prot; +}; + +static inline struct ib_sig_handover_wr *sig_handover_wr(struct ib_send_wr *wr) +{ + return container_of(wr, struct ib_sig_handover_wr, wr); +} + struct ib_recv_wr { struct ib_recv_wr *next; u64 wr_id;