Re: [PATCH rdma-core 2/4] mlx5: Implement ibv_query_qp_data_in_order() verb

[Jason Gunthorpe <jgg@nvidia.com>]

On Thu, Jun 10, 2021 at 08:50:59AM +0300, Leon Romanovsky wrote:
> On Wed, Jun 09, 2021 at 06:59:30PM +0300, Yishai Hadas wrote:
> > From: Patrisious Haddad <phaddad@nvidia.com>
> > 
> > Implement the ibv_query_qp_data_in_order() verb by using DEVX to read
> > from firmware the 'in_order_data' capability.
> > 
> > Signed-off-by: Patrisious Haddad <phaddad@nvidia.com>
> > Reviewed-by: Maor Gottlieb <maorg@nvidia.com>
> > Signed-off-by: Yishai Hadas <yishaih@nvidia.com>
> >  providers/mlx5/mlx5.c     |  1 +
> >  providers/mlx5/mlx5.h     |  3 +++
> >  providers/mlx5/mlx5_ifc.h | 39 +++++++++++++++++++++++++++++++--
> >  providers/mlx5/verbs.c    | 55 +++++++++++++++++++++++++++++++++++++++++++++++
> >  4 files changed, 96 insertions(+), 2 deletions(-)
> 
> <...>
> 
> > +int mlx5_query_qp_data_in_order(struct ibv_qp *qp, enum ibv_wr_opcode op,
> > +				uint32_t flags)
> > +{
> > +	uint32_t in_qp[DEVX_ST_SZ_DW(query_qp_in)] = {};
> > +	uint32_t out_qp[DEVX_ST_SZ_DW(query_qp_out)] = {};
> > +	struct mlx5_context *mctx = to_mctx(qp->context);
> > +	struct mlx5_qp *mqp = to_mqp(qp);
> > +	int ret;
> > +
> > +/* Currently this API is only supported for x86 architectures since most
> > + * non-x86 platforms are known to be OOO and need to do a per-platform study.
> > + */
> > +#if !defined(__i386__) && !defined(__x86_64__)
> > +	return 0;
> > +#endif
> 
> Does it compile without warnings/errors on such platforms?
> You have "return 0;" in the middle of function, so the right thing to do
> it is to write with "#if ..." over function or inside like below, as
> long as "#else" exists.
> 
> int mlx5_query_qp_data_in_order(struct ibv_qp *qp, enum ibv_wr_opcode op,
> 				uint32_t flags)
> {
> #if !defined(__i386__) && !defined(__x86_64__)
> 	/* Currently this API is only supported for x86 architectures since most
> 	 * non-x86 platforms are known to be OOO and need to do a per-platform study.
> 	 */
> 	 return 0;
> #else
> .....
> #endif

We should probably put the above in the core code anyhow

Jason