Fwd: Re: [rdma-core PATCH] providers/rxe: Replace '%' with '&' in check_qp_queue_full()

["yangx.jy@fujitsu.com" <yangx.jy@fujitsu.com>]

CC: linux-rdma mail list

On 2022/1/14 4:51, Bob Pearson wrote:
> 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
Hi Bob,

Thanks for your detailed example.
> 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.
The issue happens here, let's look at the "writing a new value" step.
We think queue is full again when prod = 0 and cons = 1, but this 
expression thinks the queue is not full.
cons == ((qp->cur_index + 1) % q->index_mask))
<1> == <0 + 1>     % <1>

In addition, this expression thinks the queue is full wrongly when we 
create an empty two-slots queue (i.e. prod = 0 and cons = 0).
cons == ((qp->cur_index + 1) % q->index_mask))
<0> == <0 + 1>     % <1>

The following steps can expose the issue:
-------------------------------------------
attr_ex.cap.max_send_wr = attr_ex.cap.max_recv_wr = 1;
...
ret = rdma_create_qp_ex(id, &attr_ex);
...
ibv_wr_start(qpx);
ibv_wr_rdma_atomic_write(qpx,...);  --> check_qp_queue_full() returns 
ENOSPC wrongly.
ibv_wr_complete(qpx);
-------------------------------------------
Please see my example[1] for details.

[1] 
https://github.com/yangx-jy/rdma-core/commit/257c6672dc147e0881525e469a49d933f1bc9183

I think the commit message misled you.

Best Regards,
Xiao Yang
> Please do not make this change.
>
> Bob