> -----Original Message----- > From: Jason Gunthorpe <jgg@xxxxxxxx> > Sent: Tuesday, December 01, 2020 4:39 PM > To: Xiong, Jianxin <jianxin.xiong@xxxxxxxxx> > Cc: linux-rdma@xxxxxxxxxxxxxxx; dri-devel@xxxxxxxxxxxxxxxxxxxxx; Doug Ledford <dledford@xxxxxxxxxx>; Leon Romanovsky > <leon@xxxxxxxxxx>; Sumit Semwal <sumit.semwal@xxxxxxxxxx>; Christian Koenig <christian.koenig@xxxxxxx>; Vetter, Daniel > <daniel.vetter@xxxxxxxxx> > Subject: Re: [PATCH rdma-core v3 4/6] pyverbs: Add dma-buf based MR support > > On Mon, Nov 30, 2020 at 05:53:39PM +0000, Xiong, Jianxin wrote: > > > From: Jason Gunthorpe <jgg@xxxxxxxx> > > > Sent: Monday, November 30, 2020 8:08 AM > > > To: Xiong, Jianxin <jianxin.xiong@xxxxxxxxx> > > > Cc: linux-rdma@xxxxxxxxxxxxxxx; dri-devel@xxxxxxxxxxxxxxxxxxxxx; > > > Doug Ledford <dledford@xxxxxxxxxx>; Leon Romanovsky > > > <leon@xxxxxxxxxx>; Sumit Semwal <sumit.semwal@xxxxxxxxxx>; Christian > > > Koenig <christian.koenig@xxxxxxx>; Vetter, Daniel > > > <daniel.vetter@xxxxxxxxx> > > > Subject: Re: [PATCH rdma-core v3 4/6] pyverbs: Add dma-buf based MR > > > support > > > > > > On Fri, Nov 27, 2020 at 12:55:41PM -0800, Jianxin Xiong wrote: > > > > > > > > +function(rdma_multifile_module PY_MODULE MODULE_NAME > > > > +LINKER_FLAGS) > > > > > > I think just replace rdma_cython_module with this? No good reason I can see to have two APIs? > > > > rdma_cython_module can handle many modules, but this one is for a single module. > > If you agree, I can merge the two by slightly tweaking the logic: each > > module starts with a .pyx file, followed by 0 or more .c and .h files. > > Then have rdma_cython_module call some rdam_single_cython_module() multiple times that has this code below? Mostly like that. Here is an outline: function(build_one_module PY_MODULE MODULE_NAME ALL_CFILES) string(REGEX_REPLACE "\\.so$" "" SONAME ${MODULE_NAME}${CMAKE_PYTHON_SO_SUFFIX}") add_library(......) set_target_properties(......) target_link_libraries(......) install(......) endfunction() function(rdma_cython_module .......) foreach(SRC_FILE ${ARGN}) ...... # commands to parse file name If (${EXT} STREQAL ".pyx") If (ALL_CFILES AND MODULE_NAME) build_one_module(${PY_MODUE} ${MODULE_NAME} ${ALL_CFILES}) set(ALL_CFILES "") set(MODULE_NAME "") endif() ...... # commands to convert .pyx to .c set(ALL_CFILES "${ALL_CFILES};${CFILE}") elseif (${EXT} STREQAL ".c") ...... set(ALL_CFILES "${ALL_CFILES};${CFILE}") else() continue() endif() endforeach() If (ALL_CFILES AND MODULE_NAME) build_one_module(${PY_MODULE} ${MODULE_NAME} ${ALL_CFILES}) endif() endfunction() > > > > Here too? You probably don't need to specify h files at all, at > > > worst they should only be used with publish_internal_headers > > > > Without the .h link, the compiler fail to find the header file (both > > dmabuf_alloc.c and the generated "dmabuf.c" contain #include > > "dmabuf_alloc.h"). > > Header files are made 'cross module' using the "publish_internal_headers" command > > But we could also hack in a -I directive to fix up the "" include for the cython outupt.. > > But it should not be handled here in the cython module command Sure. That can be fixed. > > Jason