Re: [openib-general] [PATCH 1/7] IB/ipath - performance improvements via mmap of queues

2006-08-23 Thread Roland Dreier 
Thanks, applied.

___
openib-general mailing list
openib-general@openib.org
http://openib.org/mailman/listinfo/openib-general

To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general

Re: [openib-general] [PATCH 1/7] IB/ipath - performance improvements via mmap of queues

2006-08-16 Thread Ralph Campbell 
On Wed, 2006-08-16 at 14:18 -0700, Roland Dreier wrote:
> Ralph> Was I misunderstanding your goal? The above works if you
> Ralph> are only trying to have one source code which can be
> Ralph> compiled to work with either version of libibverbs.
> 
> Yes, that's right.  I just want to be able to build against either
> version of libibverbs.  There are too many differences in structure
> layouts etc for things to work at runtime.
> 
>  - R.

OK. I guess we are in agreement.
A #define is fine with me.


___
openib-general mailing list
openib-general@openib.org
http://openib.org/mailman/listinfo/openib-general

To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general

Re: [openib-general] [PATCH 1/7] IB/ipath - performance improvements via mmap of queues

2006-08-16 Thread Roland Dreier 
Ralph> Was I misunderstanding your goal? The above works if you
Ralph> are only trying to have one source code which can be
Ralph> compiled to work with either version of libibverbs.

Yes, that's right.  I just want to be able to build against either
version of libibverbs.  There are too many differences in structure
layouts etc for things to work at runtime.

 - R.

___
openib-general mailing list
openib-general@openib.org
http://openib.org/mailman/listinfo/openib-general

To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general

Re: [openib-general] [PATCH 1/7] IB/ipath - performance improvements via mmap of queues

2006-08-16 Thread Ralph Campbell 
On Wed, 2006-08-16 at 13:56 -0700, Roland Dreier wrote:
> ralphc> A #define won't help the plug-in know what parameters to
> ralphc> pass, only a function name change will work if the
> ralphc> semantics change.
> 
> I don't follow:
> 
>   #ifdef OLD_FUNCTION_HAS_NEW_PARAMS
>   old_function(new_params);
>   #else
>   old_function(old_params);
>   #endif
> 
>  - R.

I thought your goal was to be able to have one binary libipathverbs
or libmthca which could be dlopen()'ed by libibverbs.so.1.0 or
libibverbs.so.2.0. The dlopen()'ed library needs to dynamically
check which libibverbs opened it and call old_function() with
the appropriate arguments. If it is compiled in, it will be wrong
for the other case.

Was I misunderstanding your goal? The above works if you are only
trying to have one source code which can be compiled to work with
either version of libibverbs.


___
openib-general mailing list
openib-general@openib.org
http://openib.org/mailman/listinfo/openib-general

To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general

Re: [openib-general] [PATCH 1/7] IB/ipath - performance improvements via mmap of queues

2006-08-16 Thread Roland Dreier 
ralphc> A #define won't help the plug-in know what parameters to
ralphc> pass, only a function name change will work if the
ralphc> semantics change.

I don't follow:

#ifdef OLD_FUNCTION_HAS_NEW_PARAMS
old_function(new_params);
#else
old_function(old_params);
#endif

 - R.

___
openib-general mailing list
openib-general@openib.org
http://openib.org/mailman/listinfo/openib-general

To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general

Re: [openib-general] [PATCH 1/7] IB/ipath - performance improvements via mmap of queues

2006-08-14 Thread ralphc 
>  >  int ibv_cmd_resize_cq(struct ibv_cq *cq, int cqe,
>  > -struct ibv_resize_cq *cmd, size_t cmd_size);
>  > +struct ibv_resize_cq *cmd, size_t cmd_size,
>  > +struct ibv_resize_cq_resp *resp, size_t resp_size);
>
> We can't make this change without a little more work -- as it stands
> this makes it impossible to have a low-level driver that works with
> both libibverbs 1.0 and 1.1, since there doesn't seem to be any
> autoconf way to check the number of parameters a function takes.

What we really need is a version number for the device library
plug-in to libibverbs.so interface verses the existing kernel
device driver to plug-in or application to libibverbs.so version.

> I see two ways forward: either at a new ibv_cmd_resize_cq_resp()
> function (as you did originally), or add something like
>
> #define IBV_CMD_RESIZE_CQ_HAS_RESP_PARAMS
>
> and test that in libmthca and libehca.
>
> Personally I lean towards the second solution, although neither is
> very elegant.
>
>  - R.

A #define won't help the plug-in know what parameters to pass,
only a function name change will work if the semantics change.

I can add another version argument to ibv_driver_init() if you
agree.  It seems to me that we have already made incompatible
changes by moving to libibverbs.so.2.  Couldn't we include this
as part of the transition?
Otherwise, I would vote for ibv_cmd_resize_cq_resp().



___
openib-general mailing list
openib-general@openib.org
http://openib.org/mailman/listinfo/openib-general

To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general

Re: [openib-general] [PATCH 1/7] IB/ipath - performance improvements via mmap of queues

2006-08-14 Thread Roland Dreier 
 >  int ibv_cmd_resize_cq(struct ibv_cq *cq, int cqe,
 > -  struct ibv_resize_cq *cmd, size_t cmd_size);
 > +  struct ibv_resize_cq *cmd, size_t cmd_size,
 > +  struct ibv_resize_cq_resp *resp, size_t resp_size);

We can't make this change without a little more work -- as it stands
this makes it impossible to have a low-level driver that works with
both libibverbs 1.0 and 1.1, since there doesn't seem to be any
autoconf way to check the number of parameters a function takes.

I see two ways forward: either at a new ibv_cmd_resize_cq_resp()
function (as you did originally), or add something like

#define IBV_CMD_RESIZE_CQ_HAS_RESP_PARAMS

and test that in libmthca and libehca.

Personally I lean towards the second solution, although neither is
very elegant.

 - R.

___
openib-general mailing list
openib-general@openib.org
http://openib.org/mailman/listinfo/openib-general

To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general

Re: [openib-general] [PATCH 1/7] IB/ipath - performance improvements via mmap of queues

2006-08-14 Thread Or Gerlitz 
>> I find it somehow hard to review your patch set as of two
>> reasons: (a) all the patches having the same subject line 

> a) I thought using the same subject line was the convention since
>it is essentially one patch.  I split it due to size and the
>fact that each patch has a different owner.

Nope, the convention is to have the subject line telling what it this 
patch role within the patch series very similarly to what you have 
stated in some beginning of most of the patches. Another useful practice 
is to have all the patches sent over the same thread.

> The bulk of the changes are to the InfiniPath kernel driver (ib_ipath)
> to support mmap'ing the CQ and receive queues (QP, SRQ) into
> the user level verbs library.

> The changes to the core IB were neccessary in order to allow
> additional information (i.e., the mmap offset) to be returned
> from the kernel driver to the user level verbs driver plugin.

thanks for the clarification.

Or.



___
openib-general mailing list
openib-general@openib.org
http://openib.org/mailman/listinfo/openib-general

To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general

Re: [openib-general] [PATCH 1/7] IB/ipath - performance improvements via mmap of queues

2006-08-13 Thread Michael S. Tsirkin 
Quoting r. [EMAIL PROTECTED] <[EMAIL PROTECTED]>:
> b) This is the format "svn diff" produces. I haven't had any
>complaints with it bwfore.

It's in the FAQ. Look here:
https://openib.org/tiki/tiki-index.php?page=OpenIBFAQ

anyway, git will generate the proper format for you.

-- 
MST

___
openib-general mailing list
openib-general@openib.org
http://openib.org/mailman/listinfo/openib-general

To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general

Re: [openib-general] [PATCH 1/7] IB/ipath - performance improvements via mmap of queues

2006-08-13 Thread ralphc 
> Ralph Campbell wrote:
>> The following patches update libibverbs, libmthca, libipathverbs,
>> and the kernel ib_core, ib_mthca, ib_ehca, and ib_ipath modules in
>> order to improve performance on QLogic InfiniPath HCAs.
>
> (With probably not being enough into the details of what you are
> changing) I find it somehow hard to review your patch set as of two
> reasons: (a) all the patches having the same subject line and (b) except
> for the seventh in the series, the patches were generated without the
> modified/new function/structure name (ie without the -p flag to diff).
>
> Or.

a) I thought using the same subject line was the convention since
   it is essentially one patch.  I split it due to size and the
   fact that each patch has a different owner.
b) This is the format "svn diff" produces. I haven't had any
   complaints with it bwfore.  I can resend them if you want.

The bulk of the changes are to the InfiniPath kernel driver (ib_ipath)
to support mmap'ing the CQ and receive queues (QP, SRQ) into
the user level verbs library.
The changes to the core IB were neccessary in order to allow
additional information (i.e., the mmap offset) to be returned
from the kernel driver to the user level verbs driver plugin.



___
openib-general mailing list
openib-general@openib.org
http://openib.org/mailman/listinfo/openib-general

To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general

Re: [openib-general] [PATCH 1/7] IB/ipath - performance improvements via mmap of queues

2006-08-13 Thread Or Gerlitz 
Ralph Campbell wrote:
> The following patches update libibverbs, libmthca, libipathverbs,
> and the kernel ib_core, ib_mthca, ib_ehca, and ib_ipath modules in
> order to improve performance on QLogic InfiniPath HCAs.

(With probably not being enough into the details of what you are 
changing) I find it somehow hard to review your patch set as of two 
reasons: (a) all the patches having the same subject line and (b) except 
for the seventh in the series, the patches were generated without the 
modified/new function/structure name (ie without the -p flag to diff).

Or.





___
openib-general mailing list
openib-general@openib.org
http://openib.org/mailman/listinfo/openib-general

To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general

[openib-general] [PATCH 1/7] IB/ipath - performance improvements via mmap of queues

2006-08-11 Thread Ralph Campbell 
The following patches update libibverbs, libmthca, libipathverbs,
and the kernel ib_core, ib_mthca, ib_ehca, and ib_ipath modules in
order to improve performance on QLogic InfiniPath HCAs.

The completion queue and receive queues are now mmap'ed into the
user's address space.  The kernel changes are compatible with
either the new or old user library and the new user library is
compatible with the old or new kernel driver.

The patches should be applied to the SVN trunk except for the
InfiniPath kernel driver patch (last one) which is applied to
the kernel git tree.

These patches have been posted earlier for review.
This posting should be considered ready for inclusion.
Allow the driver plug-in library to return additional data in the response
from ibv_cmd_resize_cq().

Signed-off-by: Ralph Campbell <[EMAIL PROTECTED]>

Index: src/userspace/libibverbs/include/infiniband/driver.h
===
--- src/userspace/libibverbs/include/infiniband/driver.h(revision 8843)
+++ src/userspace/libibverbs/include/infiniband/driver.h(working copy)
@@ -95,7 +95,8 @@
 int ibv_cmd_poll_cq(struct ibv_cq *cq, int ne, struct ibv_wc *wc);
 int ibv_cmd_req_notify_cq(struct ibv_cq *cq, int solicited_only);
 int ibv_cmd_resize_cq(struct ibv_cq *cq, int cqe,
- struct ibv_resize_cq *cmd, size_t cmd_size);
+ struct ibv_resize_cq *cmd, size_t cmd_size,
+ struct ibv_resize_cq_resp *resp, size_t resp_size);
 int ibv_cmd_destroy_cq(struct ibv_cq *cq);
 
 int ibv_cmd_create_srq(struct ibv_pd *pd,
Index: src/userspace/libibverbs/include/infiniband/kern-abi.h
===
--- src/userspace/libibverbs/include/infiniband/kern-abi.h  (revision 8843)
+++ src/userspace/libibverbs/include/infiniband/kern-abi.h  (working copy)
@@ -355,6 +355,8 @@
 
 struct ibv_resize_cq_resp {
__u32 cqe;
+   __u32 reserved;
+   __u64 driver_data[0];
 };
 
 struct ibv_destroy_cq {
Index: src/userspace/libibverbs/src/cmd.c
===
--- src/userspace/libibverbs/src/cmd.c  (revision 8843)
+++ src/userspace/libibverbs/src/cmd.c  (working copy)
@@ -368,18 +368,18 @@
 }
 
 int ibv_cmd_resize_cq(struct ibv_cq *cq, int cqe,
- struct ibv_resize_cq *cmd, size_t cmd_size)
+ struct ibv_resize_cq *cmd, size_t cmd_size,
+ struct ibv_resize_cq_resp *resp, size_t resp_size)
 {
-   struct ibv_resize_cq_resp resp;
 
-   IBV_INIT_CMD_RESP(cmd, cmd_size, RESIZE_CQ, &resp, sizeof resp);
+   IBV_INIT_CMD_RESP(cmd, cmd_size, RESIZE_CQ, resp, resp_size);
cmd->cq_handle = cq->handle;
cmd->cqe   = cqe;
 
if (write(cq->context->cmd_fd, cmd, cmd_size) != cmd_size)
return errno;
 
-   cq->cqe = resp.cqe;
+   cq->cqe = resp->cqe;
 
return 0;
 }



___
openib-general mailing list
openib-general@openib.org
http://openib.org/mailman/listinfo/openib-general

To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general