Re: [RFC] split struct ib_send_wr
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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] split struct ib_send_wr
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. -- Doug Ledford dledf...@redhat.com GPG KeyID: 0E572FDD signature.asc Description: OpenPGP digital signature
Re: [RFC] split struct ib_send_wr
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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] split struct ib_send_wr
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 jguntho...@obsidianresearch.com Jason -- To unsubscribe from this list: send the line unsubscribe linux-rdma in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] split struct ib_send_wr
On Aug 13, 2015, at 9:04 AM, Christoph Hellwig h...@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.le...@oracle.com -- Chuck Lever -- To unsubscribe from this list: send the line unsubscribe linux-rdma in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] split struct ib_send_wr
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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] split struct ib_send_wr
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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] split struct ib_send_wr
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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] split struct ib_send_wr
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. -- Doug Ledford dledf...@redhat.com GPG KeyID: 0E572FDD signature.asc Description: OpenPGP digital signature
Re: [RFC] split struct ib_send_wr
On Aug 12, 2015, at 6:45 PM, Doug Ledford dledf...@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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] split struct ib_send_wr
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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] split struct ib_send_wr
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 sa...@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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] split struct ib_send_wr
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.vanass...@sandisk.com -- To unsubscribe from this list: send the line unsubscribe linux-rdma in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] split struct ib_send_wr
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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] split struct ib_send_wr
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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] split struct ib_send_wr
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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] split struct ib_send_wr
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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] split struct ib_send_wr
On Aug 7, 2015, at 2:36 AM, Christoph Hellwig h...@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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] split struct ib_send_wr
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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] split struct ib_send_wr
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 sw...@opengridcomputing.com -- To unsubscribe from this list: send the line unsubscribe linux-rdma in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] split struct ib_send_wr
On Wed, Aug 05, 2015 at 10:40:08PM -0600, Jason Gunthorpe wrote: Any numbers on the struct size reduction? sizeof(struct ib_send_wr) (old): 96 sizeof(struct ib_send_wr): 48 sizeof(struct ib_rdma_wr): 64 sizeof(struct ib_atomic_wr): 96 sizeof(struct ib_ud_wr): 88 sizeof(struct ib_fast_reg_wr): 88 sizeof(struct ib_bind_mw_wr): 96 sizeof(struct ib_sig_handover_wr): 80 sizeof(struct ib_fastreg_wr) (Sagi): 64 So the commonly used send and rdma WRs are drastically redueces. FR currently doesn't look much better, but the replacement WR for it in Sagi's series will be in a simila ballpark. Note that we can remove an additional 4 bytes from each of them by removing the unused xrc_remote_srq_num value from struct ib_send_wr. -- To unsubscribe from this list: send the line unsubscribe linux-rdma in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] split struct ib_send_wr
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 h...@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 intpage_shift;
- unsigned intpage_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 intpage_shift;
+ unsigned intpage_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
Re: [RFC] split struct ib_send_wr
On Aug 6, 2015, at 1:44 PM, Steve Wise sw...@opengridcomputing.com wrote: -Original Message- From: linux-rdma-ow...@vger.kernel.org [mailto:linux-rdma-ow...@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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] split struct ib_send_wr
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 intpage_shift;
- unsigned intpage_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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
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. -- To unsubscribe from this list: send the line unsubscribe linux-rdma in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] split struct ib_send_wr
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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [RFC] split struct ib_send_wr
-Original Message- From: linux-rdma-ow...@vger.kernel.org [mailto:linux-rdma-ow...@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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] split struct ib_send_wr
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 intpage_shift;
- unsigned intpage_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 intpage_shift;
+ unsigned intpage_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
Re: [RFC] split struct ib_send_wr
On Tue, Aug 04, 2015 at 07:34:47AM -0700, Christoph Hellwig wrote: Hi all, please take a look at my RFC patch here: http://git.infradead.org/users/hch/scsi.git/commitdiff/751774250b71da83a26ba8584cff70f5e7bb7b1e the commit contains my explanation, but apparently the patch is too large for the list limit and didn't make it through. This looks really interesting, I'll try to take a closer look after I return from some vaction next week.. Any numbers on the struct size reduction? Jason -- To unsubscribe from this list: send the line unsubscribe linux-rdma in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] split struct ib_send_wr
On Tue, Aug 04, 2015 at 04:07:42PM +, Hefty, Sean wrote:
This looks like a reasonable start. It may help with feedback if you
could just post the changes to ib_verbs.h.
Not sure it's all that useful, but here we go:
diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
index 0940051..666b571 100644
--- a/include/rdma/ib_verbs.h
+++ b/include/rdma/ib_verbs.h
@@ -1100,55 +1100,96 @@ 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 intpage_shift;
- unsigned intpage_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 intpage_shift;
+ unsigned intpage_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
Re: [RFC] split struct ib_send_wr
On 08/04/2015 09:29 AM, Christoph Hellwig wrote:
On Tue, Aug 04, 2015 at 04:07:42PM +, Hefty, Sean wrote:
This looks like a reasonable start. It may help with feedback if you
could just post the changes to ib_verbs.h.
Not sure it's all that useful, but here we go:
diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
[ ... ]
struct ib_recv_wr {
+ struct ib_send_wr wr;
struct ib_recv_wr *next;
u64 wr_id;
struct ib_sge *sg_list;
Hello Christoph,
This part of the patch surprised me ?
Bart.
--
To unsubscribe from this list: send the line unsubscribe linux-rdma in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [RFC] split struct ib_send_wr
please take a look at my RFC patch here: http://git.infradead.org/users/hch/scsi.git/commitdiff/751774250b71d a83a26ba8584cff70f5e7bb7b1e the commit contains my explanation, but apparently the patch is too large for the list limit and didn't make it through. This looks like a reasonable start. It may help with feedback if you could just post the changes to ib_verbs.h. -- To unsubscribe from this list: send the line unsubscribe linux-rdma in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] split struct ib_send_wr
On Tue, Aug 04, 2015 at 09:36:49AM -0700, Bart Van Assche wrote:
diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
[ ... ]
struct ib_recv_wr {
+struct ib_send_wr wr;
struct ib_recv_wr *next;
u64 wr_id;
struct ib_sge *sg_list;
Hello Christoph,
This part of the patch surprised me ?
It should. It's a stupid cut paste error, the new member isn't used
at all. I'll fix it up.
--
To unsubscribe from this list: send the line unsubscribe linux-rdma in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
