[openib-general] moving IBM eHCA Device Driver to openib.org

Roland Dreier rolandd at cisco.com
Fri Oct 21 14:58:04 PDT 2005


Here are a few quick notes based on spending a little while skimming
through some of the ehca source code:

ehca_asm.h:
    this stuff should be in include/asm and mostly already is:
        asm_sync_mem is just the standard mb() from <asm/system.h>.
        mftb() is get_tb() from <asm/time.h>
        so you just need to add prefetch_zero to include/asm-ppc64

ehca_classes.c:
    why have ehca_module_new?  it seems to be used to allocate a
    single instance of a struct that should just be module-global variables.

ehca_classes_pSeries.h:
    Why have EHCA_MEMPAGESIZE and EHCA_MEMPAGESIZE_MASK?  Can they be
    different from PAGE_SIZE and PAGE_MASK?

    Get rid of typedefs of struct hcp_eq_handle -- just use structs
    directly in code.

    Can struct hcp_modify_qp_control_block declaration be made readable?

ehca_common.h:
    why have typedef of ehca_redcode_t?  Just use long directly.

    ntohd() seems to duplicate be64_to_cpu() except without proper
    __be64 annotation -- why is it needed?

ehca_irq.c:
    in ehca_comp_event_callback(), what protects against the CQ being
    destroyed out from under you?

ehca_kernel.h:
    ehca_sleep() and ehca_msleep() duplicate msleep_interruptible().

    why is assert() needed instead of just using BUG_ON()

    why have MIN() and MAX() instad of just using min()/max()?

    ehca_kv_to_g() looks rather horrible -- why are you working with
    kernel virtual addresses at all?

    ehca_kr_to_g() looks like it is a substitute for dma_map_single()
    Why not use the correct DMA API instead?

ehca_main.c:
    ehca_module_exit() -- I don't see where ehca_wq is destroyed.

    ehca_module_exit() -- what prevents ehca_poll_eq() from running
        after the module text is gone?  You don't wait for the kthread
        to actually stop, and it might be in the middle of the sleep.



More information about the general mailing list