[ofa-general] Re: [PATCH] libmlx4: WQE shift calculation

[Eli Cohen]

On Tue, 2007-05-15 at 07:26 -0700, Roland Dreier wrote:
> > the code that calculates WQ size is quite different between kernel and
>  > user. I think that writing the code in a way that will allow to copy it
>  > as is between kernel and user is in place. Would like me to send such a
>  > patch?
First I should add the case that triggered his patch: the userspace code
calculated a smaller buffer size than the kernel code, which caused
get_user_pages() to fail since part of the buffer did not belong to the
process's address space.

> 
> Actually I've been thinking that perhaps we should let libmlx4 tell
> mlx4 in the kernel what WQE sizes it wants to use.  Otherwise it will
> probably be a pain if we want to use a small BB for SQs, etc.
As Mihcael said in a subsequent post, we still need this code both in
user and in kernel.


> 
>  >  	case IBV_QPT_RC:
>  > -		size += sizeof (struct mlx4_wqe_raddr_seg);
>  > +		size += sizeof (struct mlx4_wqe_raddr_seg) +
>  > +			sizeof (struct mlx4_wqe_atomic_seg);
>  >  		/*
>  >  		 * An atomic op will require an atomic segment, a
>  >  		 * remote address segment and one scatter entry.
> 
> This looks wrong.  Why do we have to allow for an atomic segment for
> normal operations?  The code that starts with the context above:

The kernel code in send_wqe_overhead() always adds atomic headers. Maybe
the fix should have gone there. Still I think we should have the same
code for this calculation.

> 
> 		/*
> 		 * An atomic op will require an atomic segment, a
> 		 * remote address segment and one scatter entry.
> 		 */
> 		if (size < (sizeof (struct mlx4_wqe_atomic_seg) +
> 			    sizeof (struct mlx4_wqe_raddr_seg) +
> 			    sizeof (struct mlx4_wqe_data_seg)))
> 			size = (sizeof (struct mlx4_wqe_atomic_seg) +
> 				sizeof (struct mlx4_wqe_raddr_seg) +
> 				sizeof (struct mlx4_wqe_data_seg));
> 
> seems to take into account leaving space for atomic operations.
> 
>  - R.