[openib-general] [ANNOUNCE] GSI Implementation Candidate

Hal Rosenstock halr at voltaire.com
Mon Aug 2 08:30:34 PDT 2004


Roland Dreier wrote:
> A few quick comments based on starting to read the code:

I added a TODO list as:
https://openib.org/svn/gen2/branches/openib-candidate/src/linux-kernel/infin
iband/access/TODO
with your comments.

>  - Makefile should use standard kbuild rather your own rules.  It
>    doesn't seem like it can even build a 2.6 .ko module.
>  - Need to get rid of your spinlock wrappers -- not just for style
>    reasons, as usual the wrappers are buggy.
>  - Need to remove all the #if 0/#if 1 (or replace with #ifdef
>    SUITABLE_PREPROC_SYMBOL) -- however #ifdefs in .c files should be
>    avoided if at all possible.
>  - Static limit on number of ports/HCAs supported doesn't look good
> to me.
>  - VD_ENTERFUNC() etc. debugging code needs to be removed
>  - all printk()s need appropriate KERN_ levels.
>  - all /proc files should be moved to sysfs
>  - Shouldn't hard-code P_Key index ... needs to be settable by
> consumer
>  - Need some way to send mads with GRH

I agree in general with this but:
There is no requirement to send with GRH (only to receive with GRH)
(until multisubnet is supported which is currently an incomplete aspect of
IBA).
I have not added this one into the TODO (at least yet)...

>  - ib_reg_mr() function has been removed from the API, and registering
>    memory in the data path doesn't look good to me -- you should do
>    ib_reg_phys_mr() once to cover all of lowmem, and then just do
>    pci_map_single()/pci_unmap_single() in the data path.
>  - gsi_post_send_mad() looks buggy to me -- where is addr_hndl_attr
>    filled in?

Good catch. I missed that in the port to ib_verbs :-(

-- Hal




More information about the general mailing list