On Wed, Dec 12, 2018 at 11:09:05AM +0200, Gal Pressman wrote:
> Introduce a 'flags' field to create address handle callback and add a
> flag that marks whether the callback is executed in an atomic context or
> not.
> This will allow drivers to wait for completion instead of polling for it
> when it is allowed.
>
> Signed-off-by: Gal Pressman <galpress@xxxxxxxxxx>
> ---
> drivers/infiniband/core/cm.c | 2 +-
> drivers/infiniband/core/sa_query.c | 3 ++-
> drivers/infiniband/core/verbs.c | 15 ++++++++++-----
> drivers/infiniband/hw/bnxt_re/ib_verbs.c | 1 +
> drivers/infiniband/hw/bnxt_re/ib_verbs.h | 1 +
> drivers/infiniband/hw/hfi1/mad.c | 2 +-
> drivers/infiniband/hw/hns/hns_roce_ah.c | 1 +
> drivers/infiniband/hw/hns/hns_roce_device.h | 1 +
> drivers/infiniband/hw/mlx4/ah.c | 4 ++--
> drivers/infiniband/hw/mlx4/mad.c | 4 ++--
> drivers/infiniband/hw/mlx4/mlx4_ib.h | 2 +-
> drivers/infiniband/hw/mlx5/ah.c | 2 +-
> drivers/infiniband/hw/mlx5/mlx5_ib.h | 2 +-
> drivers/infiniband/hw/mthca/mthca_mad.c | 2 +-
> drivers/infiniband/hw/mthca/mthca_provider.c | 1 +
> drivers/infiniband/hw/ocrdma/ocrdma_ah.c | 2 +-
> drivers/infiniband/hw/ocrdma/ocrdma_ah.h | 2 +-
> drivers/infiniband/hw/qedr/verbs.c | 2 +-
> drivers/infiniband/hw/qedr/verbs.h | 2 +-
> drivers/infiniband/hw/qib/qib_verbs.c | 2 +-
> drivers/infiniband/hw/usnic/usnic_ib_verbs.c | 1 +
> drivers/infiniband/hw/usnic/usnic_ib_verbs.h | 1 +
> drivers/infiniband/hw/vmw_pvrdma/pvrdma_verbs.c | 3 ++-
> drivers/infiniband/hw/vmw_pvrdma/pvrdma_verbs.h | 2 +-
> drivers/infiniband/sw/rdmavt/ah.c | 2 ++
> drivers/infiniband/sw/rdmavt/ah.h | 1 +
> drivers/infiniband/sw/rxe/rxe_verbs.c | 1 +
> drivers/infiniband/ulp/ipoib/ipoib_ib.c | 2 +-
> drivers/infiniband/ulp/opa_vnic/opa_vnic_vema.c | 2 +-
> include/rdma/ib_verbs.h | 10 +++++++++-
> 30 files changed, 52 insertions(+), 26 deletions(-)
Have you missed 'ib_uverbs_create_ah' ?
I ask this because you created new flag between uverbs and rdma layers
and it seems that you are not 100% consistent here where you pass this
flag from uverbs in error path of ib_uverbs_create_ah (patch 2/4) but
not in the valid path.
I'd expect you to pass this flag from ib_uverbs_create_ah to
rdma_create_user_ah as well.
>
> diff --git a/drivers/infiniband/core/cm.c b/drivers/infiniband/core/cm.c
> index edb2cb758be7..cf5b3c4314bb 100644
> --- a/drivers/infiniband/core/cm.c
> +++ b/drivers/infiniband/core/cm.c
> @@ -343,7 +343,7 @@ static int cm_alloc_msg(struct cm_id_private *cm_id_priv,
> ret = -ENODEV;
> goto out;
> }
> - ah = rdma_create_ah(mad_agent->qp->pd, &av->ah_attr);
> + ah = rdma_create_ah(mad_agent->qp->pd, &av->ah_attr, 0);
> if (IS_ERR(ah)) {
> ret = PTR_ERR(ah);
> goto out;
> diff --git a/drivers/infiniband/core/sa_query.c b/drivers/infiniband/core/sa_query.c
> index be5ba5e15496..fb96d9df1fdd 100644
> --- a/drivers/infiniband/core/sa_query.c
> +++ b/drivers/infiniband/core/sa_query.c
> @@ -2276,7 +2276,8 @@ static void update_sm_ah(struct work_struct *work)
> cpu_to_be64(IB_SA_WELL_KNOWN_GUID));
> }
>
> - new_ah->ah = rdma_create_ah(port->agent->qp->pd, &ah_attr);
> + new_ah->ah = rdma_create_ah(port->agent->qp->pd, &ah_attr,
> + RDMA_CREATE_AH_SLEEPABLE);
> if (IS_ERR(new_ah->ah)) {
> pr_warn("Couldn't create new SM AH\n");
> kfree(new_ah);
> diff --git a/drivers/infiniband/core/verbs.c b/drivers/infiniband/core/verbs.c
> index 178899e3ce73..6fd8179ce82e 100644
> --- a/drivers/infiniband/core/verbs.c
> +++ b/drivers/infiniband/core/verbs.c
> @@ -475,14 +475,17 @@ rdma_update_sgid_attr(struct rdma_ah_attr *ah_attr,
>
> static struct ib_ah *_rdma_create_ah(struct ib_pd *pd,
> struct rdma_ah_attr *ah_attr,
> + u32 flags,
> struct ib_udata *udata)
> {
Why you do not add the creation flags to rdma_ah_attr?
Something like
struct rdma_ah_attr {
[...]
u8 ah_flags;
u32 ah_creation_flags;
[...]
}
It is already set to 0 in ib_uverbs_create_ah.
You just need to make sure the rest of the none user path (such as mad)
also set this to 0.
> struct ib_ah *ah;
>
> + might_sleep_if(flags & RDMA_CREATE_AH_SLEEPABLE);
> +
> if (!pd->device->create_ah)
> return ERR_PTR(-EOPNOTSUPP);
>
> - ah = pd->device->create_ah(pd, ah_attr, udata);
> + ah = pd->device->create_ah(pd, ah_attr, flags, udata);
>
