RFC: combine xprtrdma and svcrdma

RFC: combine xprtrdma and svcrdma

[Chuck Lever]

Hi-

I’ve been experimenting with adding bi-directional RPC/RDMA support
on both the client and server side. The problem is that both modules
need to be loaded before the backchannel transports are registered
and can be used by the upper layers.

If I add a couple of request_module() call sites I get this:

> WARNING: Module /lib/modules/4.1.0-rc2-00011-g1460752/kernel/net/sunrpc/xprtrdma/xprtrdma.ko ignored, due to loop
> WARNING: Loop detected: /lib/modules/4.1.0-rc2-00011-g1460752/kernel/net/sunrpc/xprtrdma/svcrdma.ko needs xprtrdma.ko which needs svcrdma.ko again!
> WARNING: Module /lib/modules/4.1.0-rc2-00011-g1460752/kernel/net/sunrpc/xprtrdma/svcrdma.ko ignored, due to loop
> Installing kernel boot image ...
> Constructing initramdisk ...
> ERROR: modinfo: could not find module svcrdma


This isn’t a problem for TCP because both client and server side
TCP socket support are built into the sunrpc.ko module. The client and
server RDMA transport support are in separate modules.

A straightforward way to address this would be to combine xprtrdma.ko
with svcrdma.ko. Any thoughts on this approach?

--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com

Re: RFC: combine xprtrdma and svcrdma

[Tom Talpey]

On 5/4/2015 3:17 PM, Chuck Lever wrote:
> Hi-
>
> I’ve been experimenting with adding bi-directional RPC/RDMA support
> on both the client and server side. The problem is that both modules
> need to be loaded before the backchannel transports are registered
> and can be used by the upper layers.
>
> If I add a couple of request_module() call sites I get this:
>
>> WARNING: Module /lib/modules/4.1.0-rc2-00011-g1460752/kernel/net/sunrpc/xprtrdma/xprtrdma.ko ignored, due to loop
>> WARNING: Loop detected: /lib/modules/4.1.0-rc2-00011-g1460752/kernel/net/sunrpc/xprtrdma/svcrdma.ko needs xprtrdma.ko which needs svcrdma.ko again!
>> WARNING: Module /lib/modules/4.1.0-rc2-00011-g1460752/kernel/net/sunrpc/xprtrdma/svcrdma.ko ignored, due to loop
>> Installing kernel boot image ...
>> Constructing initramdisk ...
>> ERROR: modinfo: could not find module svcrdma
>
>
> This isn’t a problem for TCP because both client and server side
> TCP socket support are built into the sunrpc.ko module. The client and
> server RDMA transport support are in separate modules.
>
> A straightforward way to address this would be to combine xprtrdma.ko
> with svcrdma.ko. Any thoughts on this approach?

I think it's long overdue, go for it. In the end, it will reduce
overall code.

Tom.

Re: RFC: combine xprtrdma and svcrdma

[Christoph Hellwig]

On Mon, May 04, 2015 at 03:17:25PM -0400, Chuck Lever wrote:
> This isn?t a problem for TCP because both client and server side
> TCP socket support are built into the sunrpc.ko module. The client and
> server RDMA transport support are in separate modules.

A little offtopic, but at least the code structure is a nightmare for
TCP as well where we have a file like bc_svc.c which only has a single
function (bc_send), that happens to be called from backchannel-specific code
in svc.c just to call into a function in clnt.c that is entirely
backchannel-specific as well.  Of course due to the lack of separate
modules that at least doesn't cause problems for users.

Re: RFC: combine xprtrdma and svcrdma

[Chuck Lever]

On May 5, 2015, at 1:32 PM, Christoph Hellwig <hch@infradead.org> wrote:

> On Mon, May 04, 2015 at 03:17:25PM -0400, Chuck Lever wrote:
>> This isn?t a problem for TCP because both client and server side
>> TCP socket support are built into the sunrpc.ko module. The client and
>> server RDMA transport support are in separate modules.
> 
> A little offtopic, but at least the code structure is a nightmare for
> TCP as well where we have a file like bc_svc.c which only has a single
> function (bc_send), that happens to be called from backchannel-specific code
> in svc.c just to call into a function in clnt.c that is entirely
> backchannel-specific as well.  Of course due to the lack of separate
> modules that at least doesn't cause problems for users.

bc_svc.c struck me as left over from a prototype.

It would be reasonable to move bc_send into xprtsock.c (or some other
convenient file) and then pass its address into the server code via
a virtual function (eg. as an rpc_xprt op).

I’ve been scratching my head wondering why we have still have
CONFIG_SUNRPC_BACKCHANNEL, which is not exposed in menuconfig. Is it
time to remove it?

--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com

Re: RFC: combine xprtrdma and svcrdma

[Christoph Hellwig]

On Tue, May 05, 2015 at 02:15:07PM -0400, Chuck Lever wrote:
> bc_svc.c struck me as left over from a prototype.
> 
> It would be reasonable to move bc_send into xprtsock.c (or some other
> convenient file) and then pass its address into the server code via
> a virtual function (eg. as an rpc_xprt op).

Given that they always sit in the same module direct calls should just
work fine.

> I?ve been scratching my head wondering why we have still have
> CONFIG_SUNRPC_BACKCHANNEL, which is not exposed in menuconfig. Is it
> time to remove it?

It's selected by CONFIG_NFS_V4_1, and thus isn't build if you don't
select 4.1 support.