Re: [PATCH 7/8] xprtrdma: Split the completion queue

Sagi Grimberg
2014-04-16T14:12:30Z

On 4/16/2014 4:30 PM, Steve Wise wrote:
> On 4/16/2014 7:48 AM, Sagi Grimberg wrote:
>> On 4/15/2014 1:23 AM, Chuck Lever wrote:
>>> The current CQ handler uses the ib_wc.opcode field to distinguish
>>> between event types. However, the contents of that field are not
>>> reliable if the completion status is not IB_WC_SUCCESS.
>>>
>>> When an error completion occurs on a send event, the CQ handler
>>> schedules a tasklet with something that is not a struct rpcrdma_rep.
>>> This is never correct behavior, and sometimes it results in a panic.
>>>
>>> To resolve this issue, split the completion queue into a send CQ and
>>> a receive CQ. The send CQ handler now handles only struct rpcrdma_mw
>>> wr_id's, and the receive CQ handler now handles only struct
>>> rpcrdma_rep wr_id's.
>>
>> Hey Chuck,
>>
>> So 2 suggestions related (although not directly) to this one.
>>
>> 1. I recommend suppressing Fastreg completions - no one cares that
>> they succeeded.
>>
>
> Not true. The nfsrdma client uses frmrs across re-connects for the
> same mount and needs to know at any point in time if a frmr is
> registered or invalid. So completions of both fastreg and invalidate
> need to be signaled. See:
>
> commit 5c635e09cec0feeeb310968e51dad01040244851
> Author: Tom Tucker <tom-/Yg/VP3ZvrM@public.gmane.org>
> Date: Wed Feb 9 19:45:34 2011 +0000
>
> RPCRDMA: Fix FRMR registration/invalidate handling.
>

Hmm, But if either FASTREG or LINV failed the QP will go to error state
and you *will* get the error wc (with a rain of FLUSH errors).
AFAICT it is safe to assume that it succeeded as long as you don't get
error completions.
Moreover, FASTREG on top of FASTREG are not allowed indeed, but AFAIK
LINV on top of LINV are allowed.
It is OK to just always do LINV+FASTREG post-list each registration and
this way no need to account for successful completions.

Cheers,
Sagi.