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

[Devesh Sharma <devesh.s.sharma@oracle.com>]

> -----Original Message-----
> From: yangx.jy@fujitsu.com <yangx.jy@fujitsu.com>
> Sent: Tuesday, January 11, 2022 2:41 PM
> To: Devesh Sharma <devesh.s.sharma@oracle.com>
> Cc: rpearsonhpe@gmail.com; leon@kernel.org; linux-rdma@vger.kernel.org
> Subject: [External] : Re: [PATCH rdma-core RESEND] providers/rxe: Replace
> '%' with '&' in check_qp_queue_full()
> 
> On 2022/1/11 12:12, Devesh Sharma wrote:
> >
> >> -----Original Message-----
> >> From: Xiao Yang<yangx.jy@fujitsu.com>
> >> Sent: Tuesday, January 11, 2022 7:54 AM
> >> To: rpearsonhpe@gmail.com; leon@kernel.org
> >> Cc: linux-rdma@vger.kernel.org; Xiao Yang<yangx.jy@fujitsu.com>
> >> Subject: [PATCH rdma-core RESEND] providers/rxe: Replace '%' with '&'
> >> in
> >> check_qp_queue_full()
> >>
> >> The expression "cons == ((qp->cur_index + 1) % q->index_mask)"
> >> mistakenly assumes the queue is full when the number of entires is
> >> equal to "maximum
> >> - 1"
> >> (maximum is correct).
> >>
> >> 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@fujitsu.com>
> >> ---
> >>   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))
> > Are you sure that index_mask would always be aligned with 2^X?
> Hi Deves,
> 
> I think it is.  index_mask is alwasy set to 2^X -1 in kernel:

Cool looks good, other than comments from Leon.
Reviewed-By: Devesh Sharma <devesh.s.sharma@oracle.com>

> ----------------------------------------------------------------
> struct rxe_queue *rxe_queue_init(struct rxe_dev *rxe, int *num_elem,
>                          unsigned int elem_size, enum queue_type type) {
>      ...
>      num_slots = *num_elem + 1;
>      num_slots = roundup_pow_of_two(num_slots);
>      q->index_mask = num_slots - 1;
>      ...
> }
> ----------------------------------------------------------------
> 
> Best Regards,
> Xiao Yang
> >>   		qp->err = ENOSPC;
> >>   err:
> >>   	return qp->err;
> >> --
> >> 2.25.1
> >>
> >>