Re: [PATCH rdma-next 1/6] IB/uverbs: Allow CQ moderation with modify CQ

[Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>]

[-- Attachment #1: Type: text/plain, Size: 3112 bytes --]

On Thu, Oct 19, 2017 at 08:35:19AM +0300, Leon Romanovsky wrote:
> From: Yonatan Cohen <yonatanc-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>
> Uverbs support in modify_cq for CQ moderation only.
> Gives ability to change cq_max_count and cq_period.
> CQ moderation enhance performance by moderating the number
> of cookies needed to create an event instead of application
> having to suffer from event per cookie.
> To achieve CQ moderation the application needs to set cq_max_count
> and cq_period.
> cq_max_count - defines the number of cookies
>                needed to create an event.
> cq_period - defines the timeout (micro seconds) between last
>             event and a new one that will occur even if
> 	    cq_max_count was not satisfied
>
> Signed-off-by: Yonatan Cohen <yonatanc-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> Reviewed-by: Majd Dibbiny <majd-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> ---
>  drivers/infiniband/core/uverbs.h      |  1 +
>  drivers/infiniband/core/uverbs_cmd.c  | 36 +++++++++++++++++++++++++++++++++++
>  drivers/infiniband/core/uverbs_main.c |  1 +
>  include/rdma/ib_verbs.h               |  4 ++++
>  include/uapi/rdma/ib_user_verbs.h     | 11 ++++++++++-
>  5 files changed, 52 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/infiniband/core/uverbs.h b/drivers/infiniband/core/uverbs.h
> index ee2739ae4305..deccefb71a6b 100644
> --- a/drivers/infiniband/core/uverbs.h
> +++ b/drivers/infiniband/core/uverbs.h
> @@ -306,5 +306,6 @@ IB_UVERBS_DECLARE_EX_CMD(destroy_wq);
>  IB_UVERBS_DECLARE_EX_CMD(create_rwq_ind_table);
>  IB_UVERBS_DECLARE_EX_CMD(destroy_rwq_ind_table);
>  IB_UVERBS_DECLARE_EX_CMD(modify_qp);
> +IB_UVERBS_DECLARE_EX_CMD(modify_cq);
>
>  #endif /* UVERBS_H */
> diff --git a/drivers/infiniband/core/uverbs_cmd.c b/drivers/infiniband/core/uverbs_cmd.c
> index ab29a0327831..c7819e5540f1 100644
> --- a/drivers/infiniband/core/uverbs_cmd.c
> +++ b/drivers/infiniband/core/uverbs_cmd.c
> @@ -3856,3 +3856,39 @@ int ib_uverbs_ex_query_device(struct ib_uverbs_file *file,
>  	err = ib_copy_to_udata(ucore, &resp, resp.response_length);
>  	return err;
>  }
> +
> +int ib_uverbs_ex_modify_cq(struct ib_uverbs_file *file,
> +			   struct ib_device *ib_dev,
> +			   struct ib_udata *ucore,
> +			   struct ib_udata *uhw)
> +{
> +	struct ib_uverbs_ex_modify_cq cmd = {};
> +	struct ib_cq *cq;
> +	int ret;
> +
> +	/* sanity checks */
> +	if (ucore->inlen > sizeof(cmd) &&
> +	    !ib_is_udata_cleared(ucore, sizeof(cmd),
> +				 ucore->inlen - sizeof(cmd)))
> +		return -EOPNOTSUPP;
> +
> +	ret = ib_copy_from_udata(&cmd, ucore, min(sizeof(cmd), ucore->inlen));
> +	if (ret)
> +		return ret;
> +
> +	if (cmd.attr_mask != IB_CQ_MODERATE)
> +		return -EINVAL;
> +
> +	if (!cmd.comp_mask)
> +		return -EINVAL;

No way that it worked, comp_mask is never set, so it is supposed to be
zero and it is always returning EINVAL.

Additionally, the "attr_mask" is already carrying the same information as
"comp_mask" will handle, hence the "comp_mask" should be dropped.

Thanks

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]