| Message ID | 4e7574d7-960f-9f92-e92f-630287f1903f@talpey.com (mailing list archive) |
|---|---|
| State | Rejected |
| Delegated to: | Jason Gunthorpe |
| Headers | show |
| Series | Add missing ib_uverbs dependency from SoftiWARP | expand |
On Wed, Sep 07, 2022 at 09:45:07AM -0400, Tom Talpey wrote: > When loading the siw module, ib_uverbs is needed so that consumers may > access it. However, siw references only inline functions in ib_uverbs.h, > so the kernel linker can not detect this, and the module is not loaded > automatically. Add a module dependency to ensure ib_uverbs is present. No, that is not how things work. Modern rdma-core will auto-load uverbs when something uses it, we shouldn't have things like this. Jason
> -----Original Message----- > From: Jason Gunthorpe <jgg@nvidia.com> > Sent: Wednesday, 7 September 2022 15:51 > To: Tom Talpey <tom@talpey.com> > Cc: Leon Romanovsky <leonro@nvidia.com>; Bernard Metzler > <BMT@zurich.ibm.com>; linux-rdma@vger.kernel.org > Subject: [EXTERNAL] Re: [PATCH] Add missing ib_uverbs dependency from > SoftiWARP > > On Wed, Sep 07, 2022 at 09:45:07AM -0400, Tom Talpey wrote: > > When loading the siw module, ib_uverbs is needed so that consumers may > > access it. However, siw references only inline functions in > ib_uverbs.h, > > so the kernel linker can not detect this, and the module is not loaded > > automatically. Add a module dependency to ensure ib_uverbs is present. > > No, that is not how things work. > > Modern rdma-core will auto-load uverbs when something uses it, we > shouldn't have things like this. Hmmm, if e.g. siw references ib_copy_to_udata(), it uses ib_uverbs functionality, but the kernel build mechanics do not bring in ib_uverbs.ko dependency, since ib_copy_to_udata() is just an inline defined in ib_uverbs.h. It seems drivers depend on using at least one 'real' symbol out of that .ko. Maybe we shall not put functions into header files? Thanks Bernard.
On 9/7/2022 9:51 AM, Jason Gunthorpe wrote: > On Wed, Sep 07, 2022 at 09:45:07AM -0400, Tom Talpey wrote: >> When loading the siw module, ib_uverbs is needed so that consumers may >> access it. However, siw references only inline functions in ib_uverbs.h, >> so the kernel linker can not detect this, and the module is not loaded >> automatically. Add a module dependency to ensure ib_uverbs is present. > > No, that is not how things work. > > Modern rdma-core will auto-load uverbs when something uses it, we > shouldn't have things like this. But, it doesn't, for the reason stated - siw only consumes inline functions from ib_uverbs.h, and the kernel linker has no clue. You can test it easily, just load siw on a laptop without any other RDMA provider. The ib_uverbs module will not be loaded, and the siw provider won't be seen, rping -s will run but peers cannot connect for example. My workaround has been to add a modprobe.d file, but it's a hack and very non-obvious. Is there a simpler way to allow ib_uverbs to be auto-loaded for siw? Tom.
On Wed, Sep 07, 2022 at 09:57:43AM -0400, Tom Talpey wrote: > You can test it easily, just load siw on a laptop without any > other RDMA provider. The ib_uverbs module will not be loaded, > and the siw provider won't be seen, rping -s will run but peers > cannot connect for example. Perhaps there is something funky with rping, it works fine in simpler cases: $ rdma link show link siw0/1 state ACTIVE physical_state LINK_UP netdev enp0s31f6 $ sudo rmmod ib_uverbs $ build/bin/ibv_devinfo hca_id: siw0 transport: iWARP (1) fw_ver: 0.0.0 node_guid: 7686:e2ff:fe28:63fc sys_image_guid: 7486:e228:63fc:0000 vendor_id: 0x626d74 vendor_part_id: 1 hw_ver: 0x0 phys_port_cnt: 1 port: 1 state: PORT_ACTIVE (4) max_mtu: 1024 (3) active_mtu: 1024 (3) sm_lid: 0 port_lid: 0 port_lmc: 0x00 link_layer: Ethernet $ lsmod | grep -i uverb ib_uverbs 163840 0 ib_core 393216 2 ib_uverbs,siw Jason
On 9/7/2022 10:06 AM, Jason Gunthorpe wrote: > On Wed, Sep 07, 2022 at 09:57:43AM -0400, Tom Talpey wrote: > >> You can test it easily, just load siw on a laptop without any >> other RDMA provider. The ib_uverbs module will not be loaded, >> and the siw provider won't be seen, rping -s will run but peers >> cannot connect for example. > > Perhaps there is something funky with rping, it works fine in simpler cases: > > $ rdma link show > link siw0/1 state ACTIVE physical_state LINK_UP netdev enp0s31f6 > $ sudo rmmod ib_uverbs > $ build/bin/ibv_devinfo > hca_id: siw0 > transport: iWARP (1) > fw_ver: 0.0.0 > node_guid: 7686:e2ff:fe28:63fc > sys_image_guid: 7486:e228:63fc:0000 > vendor_id: 0x626d74 > vendor_part_id: 1 > hw_ver: 0x0 > phys_port_cnt: 1 > port: 1 > state: PORT_ACTIVE (4) > max_mtu: 1024 (3) > active_mtu: 1024 (3) > sm_lid: 0 > port_lid: 0 > port_lmc: 0x00 > link_layer: Ethernet > $ lsmod | grep -i uverb > ib_uverbs 163840 0 > ib_core 393216 2 ib_uverbs,siw That's odd - your ib_uverbs "Used by" list is empty. Did some module dependency cache force-load it? On my system, it doesn't load at all. With the proposed siw softdep, it does. Tom. > > Jason > >
On Wed, Sep 07, 2022 at 11:24:19AM -0400, Tom Talpey wrote: > On 9/7/2022 10:06 AM, Jason Gunthorpe wrote: > > On Wed, Sep 07, 2022 at 09:57:43AM -0400, Tom Talpey wrote: > > > > > You can test it easily, just load siw on a laptop without any > > > other RDMA provider. The ib_uverbs module will not be loaded, > > > and the siw provider won't be seen, rping -s will run but peers > > > cannot connect for example. > > > > Perhaps there is something funky with rping, it works fine in simpler cases: > > > > $ rdma link show > > link siw0/1 state ACTIVE physical_state LINK_UP netdev enp0s31f6 > > $ sudo rmmod ib_uverbs > > $ build/bin/ibv_devinfo > > hca_id: siw0 > > transport: iWARP (1) > > fw_ver: 0.0.0 > > node_guid: 7686:e2ff:fe28:63fc > > sys_image_guid: 7486:e228:63fc:0000 > > vendor_id: 0x626d74 > > vendor_part_id: 1 > > hw_ver: 0x0 > > phys_port_cnt: 1 > > port: 1 > > state: PORT_ACTIVE (4) > > max_mtu: 1024 (3) > > active_mtu: 1024 (3) > > sm_lid: 0 > > port_lid: 0 > > port_lmc: 0x00 > > link_layer: Ethernet > > $ lsmod | grep -i uverb > > ib_uverbs 163840 0 > > ib_core 393216 2 ib_uverbs,siw > > > That's odd - your ib_uverbs "Used by" list is empty. Did some > module dependency cache force-load it? On my system, it doesn't > load at all. With the proposed siw softdep, it does. As I said, modern rdma-core auto-loads it on its own. There is no module dependency causing it to load. Jason
On 9/7/2022 12:48 PM, Jason Gunthorpe wrote: > On Wed, Sep 07, 2022 at 11:24:19AM -0400, Tom Talpey wrote: >> That's odd - your ib_uverbs "Used by" list is empty. Did some >> module dependency cache force-load it? On my system, it doesn't >> load at all. With the proposed siw softdep, it does. > > As I said, modern rdma-core auto-loads it on its own. There is no > module dependency causing it to load. Ok. I just packed up my test machines for shipping to the SDC IOLab event next week, so I'll re-test then, and try to figure out why the patch did change behavior. BTW, there I'll be working on the kernel SMB3 server (ksmbd) RDMA support, and testing over siw and rxe. Tom.
diff --git a/drivers/infiniband/sw/siw/siw_main.c b/drivers/infiniband/sw/siw/siw_main.c index dacc174604bf..372b37b18bac 100644 --- a/drivers/infiniband/sw/siw/siw_main.c +++ b/drivers/infiniband/sw/siw/siw_main.c @@ -628,3 +628,4 @@ module_init(siw_init_module); module_exit(siw_exit_module); MODULE_ALIAS_RDMA_LINK("siw"); +MODULE_SOFTDEP("ib_uverbs");
When loading the siw module, ib_uverbs is needed so that consumers may access it. However, siw references only inline functions in ib_uverbs.h, so the kernel linker can not detect this, and the module is not loaded automatically. Add a module dependency to ensure ib_uverbs is present. Signed-off-by: Tom Talpey <tom@talpey.com> --- drivers/infiniband/sw/siw/siw_main.c | 1 + 1 file changed, 1 insertion(+)