On 1/10/22 20:22, yangx.jy@xxxxxxxxxxx wrote:
> On 2022/1/11 9:41, Xiao Yang wrote:
>> The expression "cons == ((qp->cur_index + 1) % q->index_mask)" mistakenly
>> assumes the queue is full when qp->cur_index is equal to "maximum - 1"
>> (maximum is correct).
> Hi All,
>
> The commit message seems inappropriate so I will resend this patch.
>
> Best Regards,
> Xiao Yang
>> For example:
>> If cons and qp->cur_index are 0 and q->index_mask is 1, check_qp_queue_full()
>> reports ENOSPC.
>>
>> Fixes: 1a894ca10105 ("Providers/rxe: Implement ibv_create_qp_ex verb")
>> Signed-off-by: Xiao Yang <yangx.jy@xxxxxxxxxxx>
>> ---
>> providers/rxe/rxe_queue.h | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/providers/rxe/rxe_queue.h b/providers/rxe/rxe_queue.h
>> index 6de8140c..708e76ac 100644
>> --- a/providers/rxe/rxe_queue.h
>> +++ b/providers/rxe/rxe_queue.h
>> @@ -205,7 +205,7 @@ static inline int check_qp_queue_full(struct rxe_qp *qp)
>> if (qp->err)
>> goto err;
>>
>> - if (cons == ((qp->cur_index + 1) % q->index_mask))
>> + if (cons == ((qp->cur_index + 1) & q->index_mask))
>> qp->err = ENOSPC;
>> err:
>> return qp->err;
The way these queues work they can only hold 2^n - 1 entries. The reason for this is
to distinguish empty and full. If you let the last entry be written then producer would advance to equal consumer which is the initial empty condition. So a queue which can hold 1 entry has to have two slots (n=1) but can only hold one entry. It begins with
prod = cons = 0 and is empty and is *not* full
Adding one entry gives
slot[0] = value, prod = 1, cons = 0 and is full and not empty
After reading this value
slot[0] = old value, prod = cons = 1 and queue is empty again.
Writing a new value
slot[1] = new value, slot[0] = old value, prod = 0 and cons = 1 and queue is full again.
The expression full = (cons == ((qp->cur_index + 1) % q->index_mask)) is correct.
Please do not make this change.
Bob
