> On Dec 1, 2015, at 8:38 AM, Tom Talpey <tom@xxxxxxxxxx> wrote:
>
> On 11/30/2015 5:25 PM, Chuck Lever wrote:
>> To support the server-side of an NFSv4.1 backchannel on RDMA
>> connections, add a transport class that enables backward
>> direction messages on an existing forward channel connection.
>>
>
>> +static void *
>> +xprt_rdma_bc_allocate(struct rpc_task *task, size_t size)
>> +{
>> + struct rpc_rqst *rqst = task->tk_rqstp;
>> + struct svc_rdma_op_ctxt *ctxt;
>> + struct svcxprt_rdma *rdma;
>> + struct svc_xprt *sxprt;
>> + struct page *page;
>> +
>> + if (size > PAGE_SIZE) {
>> + WARN_ONCE(1, "failed to handle buffer allocation (size %zu)\n",
>> + size);
>
> You may want to add more context to that rather cryptic string, at
> least the function name.
>
> Also, it's not exactly "failed to handle", it's an invalid size. Why
> would this ever happen? Why even log it?
>
>
>
>> +static int
>> +rpcrdma_bc_send_request(struct svcxprt_rdma *rdma, struct rpc_rqst *rqst)
>> +{
> ...
>> +
>> +drop_connection:
>> + dprintk("Failed to send backchannel call\n");
>
> Ditto on the prefix / function context.
>
> And also...
>
>> + dprintk("%s: sending request with xid: %08x\n",
>> + __func__, be32_to_cpu(rqst->rq_xid));
> ...
>> + dprintk("RPC: %s: xprt %p\n", __func__, xprt);
>
> The format strings for many of the dprintk's are somewhat inconsistent.
> Some start with "RPC", some with the function name, and some (in other
> patches of this series) with "svcrdma". Confusing, and perhaps hard to
> pick out of the log.
They do follow a convention: “RPC: “ is used on the client
side when there is no rpc_task::tk_pid available. “svcrdma” is
used on the server everywhere.
The dprintk changes here were a bit cursory, so I will go back and
review them more carefully.
--
Chuck Lever
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
