Re: [PATCH for-next v4] Subject: RDMA/rxe: Handle zero length rdma

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 09/02/2023 16:14, Matsuda, Daisuke/松田 大輔 wrote:
> On Wed, Feb 8, 2023 5:41 PM Li, Zhijian/李 智坚 wrote:
>>
>> On 02/02/2023 12:42, Bob Pearson wrote:
>>> +/* resolve the packet rkey to qp->resp.mr or set qp->resp.mr to NULL
>>> + * if an invalid rkey is received or the rdma length is zero. For middle
>>> + * or last packets use the stored value of mr.
>>> + */
>>>    static enum resp_states check_rkey(struct rxe_qp *qp,
>>>    				   struct rxe_pkt_info *pkt)
>>>    {
>>> @@ -473,10 +487,12 @@ static enum resp_states check_rkey(struct rxe_qp *qp,
>>>    		return RESPST_EXECUTE;
>>>    	}
>>>
>>> -	/* A zero-byte op is not required to set an addr or rkey. See C9-88 */
>>> +	/* A zero-byte read or write op is not required to
>>> +	 * set an addr or rkey. See C9-88
>>> +	 */
>>>    	if ((pkt->mask & RXE_READ_OR_WRITE_MASK) &&
>>> -	    (pkt->mask & RXE_RETH_MASK) &&
>>> -	    reth_len(pkt) == 0) {
>>> +	    (pkt->mask & RXE_RETH_MASK) && reth_len(pkt) == 0) {
>>> +		qp->resp.mr = NULL;
>>
>> You are making sure 'qp->resp.mr = NULL', but I doubt the previous qp->resp.mr will leak after this assignment when its
>> value is not NULL.
> 
> I do not see what you mean by " the previous qp->resp.mr ".
> As far as I understand, 'qp->resp.mr' is set to NULL in cleanup()
> before responder completes its work, so it is not supposed to be
> reused unlike 'res->read.rkey' being retained for multi-packet Read
> responses. Could you elaborate on your view?

IMO every 'qp->resp.mr' assignment will take a mr reference, so we should drop the this reference if we want to assign it a new mr again.


Theoretically, it should be changed to something like
if (qp->resp.mr) {
	rxe_put(qp->resp.mr)
         qp->resp.mr = NULL;
}




>>
>>>    		return RESPST_EXECUTE;
>>>    	}
>>>
>>> @@ -555,6 +571,7 @@ static enum resp_states check_rkey(struct rxe_qp *qp,
>>>    	return RESPST_EXECUTE;
>>>
>>>    err:
>>> +	qp->resp.mr = NULL;

ditto

Thanks
Zhijian

>>>    	if (mr)




[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Photo]     [Yosemite News]     [Yosemite Photos]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux