2015-05-04 19:17:18

by Chuck Lever III

[permalink] [raw]
Subject: RFC: combine xprtrdma and svcrdma

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





2015-05-05 12:03:37

by Tom Talpey

[permalink] [raw]
Subject: Re: RFC: combine xprtrdma and svcrdma

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.


2015-05-05 17:32:07

by Christoph Hellwig

[permalink] [raw]
Subject: Re: RFC: combine xprtrdma and svcrdma

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.

2015-05-05 18:15:18

by Chuck Lever III

[permalink] [raw]
Subject: Re: RFC: combine xprtrdma and svcrdma


On May 5, 2015, at 1:32 PM, Christoph Hellwig <[email protected]> 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




2015-05-05 19:12:31

by Christoph Hellwig

[permalink] [raw]
Subject: Re: RFC: combine xprtrdma and svcrdma

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.