Hi all,
On Mon, 6 Aug 2018 14:53:31 +1000 Stephen Rothwell <s...@canb.auug.org.au>
wrote:
>
> Today's linux-next merge of the tip tree got a conflict in:
>
> drivers/infiniband/core/rdma_core.c
>
> between commit:
>
> 9867f5c6695f ("IB/uverbs: Convert 'bool exclusive' into an enum")
>
> from the rdma tree and commit:
>
> bfc18e389c7a ("atomics/treewide: Rename __atomic_add_unless() =>
> atomic_fetch_add_unless()")
>
> from the tip tree.
>
> I fixed it up (see below) and can carry the fix as necessary. This
> is now fixed as far as linux-next is concerned, but any non trivial
> conflicts should be mentioned to your upstream maintainer when your tree
> is submitted for merging. You may also want to consider cooperating
> with the maintainer of the conflicting tree to minimise any particularly
> complex conflicts.
>
> --
> Cheers,
> Stephen Rothwell
>
> diff --cc drivers/infiniband/core/rdma_core.c
> index 4235b9ddc2ad,475910ffbcb6..000000000000
> --- a/drivers/infiniband/core/rdma_core.c
> +++ b/drivers/infiniband/core/rdma_core.c
> @@@ -122,206 -120,19 +122,206 @@@ static int uverbs_try_lock_object(struc
> * concurrently, setting the counter to zero is enough for releasing
> * this lock.
> */
> - if (!exclusive)
> + switch (mode) {
> + case UVERBS_LOOKUP_READ:
> - return __atomic_add_unless(&uobj->usecnt, 1, -1) == -1 ?
> + return atomic_fetch_add_unless(&uobj->usecnt, 1, -1) == -1 ?
> -EBUSY : 0;
> + case UVERBS_LOOKUP_WRITE:
> + /* lock is exclusive */
> + return atomic_cmpxchg(&uobj->usecnt, 0, -1) == 0 ? 0 : -EBUSY;
> + case UVERBS_LOOKUP_DESTROY:
> + return 0;
> + }
> + return 0;
> +}
> +
> +static void assert_uverbs_usecnt(struct ib_uobject *uobj,
> + enum rdma_lookup_mode mode)
> +{
> +#ifdef CONFIG_LOCKDEP
> + switch (mode) {
> + case UVERBS_LOOKUP_READ:
> + WARN_ON(atomic_read(&uobj->usecnt) <= 0);
> + break;
> + case UVERBS_LOOKUP_WRITE:
> + WARN_ON(atomic_read(&uobj->usecnt) != -1);
> + break;
> + case UVERBS_LOOKUP_DESTROY:
> + break;
> + }
> +#endif
> +}
> +
> +/*
> + * This must be called with the hw_destroy_rwsem locked for read or write,
> + * also the uobject itself must be locked for write.
> + *
> + * Upon return the HW object is guaranteed to be destroyed.
> + *
> + * For RDMA_REMOVE_ABORT, the hw_destroy_rwsem is not required to be held,
> + * however the type's allocat_commit function cannot have been called and
> the
> + * uobject cannot be on the uobjects_lists
> + *
> + * For RDMA_REMOVE_DESTROY the caller shold be holding a kref (eg via
> + * rdma_lookup_get_uobject) and the object is left in a state where the
> caller
> + * needs to call rdma_lookup_put_uobject.
> + *
> + * For all other destroy modes this function internally unlocks the uobject
> + * and consumes the kref on the uobj.
> + */
> +static int uverbs_destroy_uobject(struct ib_uobject *uobj,
> + enum rdma_remove_reason reason)
> +{
> + struct ib_uverbs_file *ufile = uobj->ufile;
> + unsigned long flags;
> + int ret;
> +
> + lockdep_assert_held(&ufile->hw_destroy_rwsem);
> + assert_uverbs_usecnt(uobj, UVERBS_LOOKUP_WRITE);
> +
> + if (uobj->object) {
> + ret = uobj->type->type_class->destroy_hw(uobj, reason);
> + if (ret) {
> + if (ib_is_destroy_retryable(ret, reason, uobj))
> + return ret;
> +
> + /* Nothing to be done, dangle the memory and move on */
> + WARN(true,
> + "ib_uverbs: failed to remove uobject id %d, driver
> err=%d",
> + uobj->id, ret);
> + }
> +
> + uobj->object = NULL;
> + }
> +
> + if (reason == RDMA_REMOVE_ABORT) {
> + WARN_ON(!list_empty(&uobj->list));
> + WARN_ON(!uobj->context);
> + uobj->type->type_class->alloc_abort(uobj);
> + }
> +
> + uobj->context = NULL;
> +
> + /*
> + * For DESTROY the usecnt is held write locked, the caller is expected
> + * to put it unlock and put the object when done with it. Only DESTROY
> + * can remove the IDR handle.
> + */
> + if (reason != RDMA_REMOVE_DESTROY)
> + atomic_set(&uobj->usecnt, 0);
> + else
> + uobj->type->type_class->remove_handle(uobj);
> +
> + if (!list_empty(&uobj->list)) {
> + spin_lock_irqsave(&ufile->uobjects_lock, flags);
> + list_del_init(&uobj->list);
> + spin_unlock_irqrestore(&ufile->uobjects_lock, flags);
> +
> + /*
> + * Pairs with the get in rdma_alloc_commit_uobject(), could
> + * destroy uobj.
> + */
> + uverbs_uobject_put(uobj);
> + }
>
> - /* lock is either WRITE or DESTROY - should be exclusive */
> - return atomic_cmpxchg(&uobj->usecnt, 0, -1) == 0 ? 0 : -EBUSY;
> + /*
> + * When aborting the stack kref remains owned by the core code, and is
> + * not transferred into the type. Pairs with the get in alloc_uobj
> + */
> + if (reason == RDMA_REMOVE_ABORT)
> + uverbs_uobject_put(uobj);
> +
> + return 0;
> }
>
> -static struct ib_uobject *alloc_uobj(struct ib_ucontext *context,
> +/*
> + * This calls uverbs_destroy_uobject() using the RDMA_REMOVE_DESTROY
> + * sequence. It should only be used from command callbacks. On success the
> + * caller must pair this with rdma_lookup_put_uobject(LOOKUP_WRITE). This
> + * version requires the caller to have already obtained an
> + * LOOKUP_DESTROY uobject kref.
> + */
> +int uobj_destroy(struct ib_uobject *uobj)
> +{
> + struct ib_uverbs_file *ufile = uobj->ufile;
> + int ret;
> +
> + down_read(&ufile->hw_destroy_rwsem);
> +
> + ret = uverbs_try_lock_object(uobj, UVERBS_LOOKUP_WRITE);
> + if (ret)
> + goto out_unlock;
> +
> + ret = uverbs_destroy_uobject(uobj, RDMA_REMOVE_DESTROY);
> + if (ret) {
> + atomic_set(&uobj->usecnt, 0);
> + goto out_unlock;
> + }
> +
> +out_unlock:
> + up_read(&ufile->hw_destroy_rwsem);
> + return ret;
> +}
> +
> +/*
> + * uobj_get_destroy destroys the HW object and returns a handle to the uobj
> + * with a NULL object pointer. The caller must pair this with
> + * uverbs_put_destroy.
> + */
> +struct ib_uobject *__uobj_get_destroy(const struct uverbs_obj_type *type,
> + u32 id, struct ib_uverbs_file *ufile)
> +{
> + struct ib_uobject *uobj;
> + int ret;
> +
> + uobj = rdma_lookup_get_uobject(type, ufile, id, UVERBS_LOOKUP_DESTROY);
> + if (IS_ERR(uobj))
> + return uobj;
> +
> + ret = uobj_destroy(uobj);
> + if (ret) {
> + rdma_lookup_put_uobject(uobj, UVERBS_LOOKUP_DESTROY);
> + return ERR_PTR(ret);
> + }
> +
> + return uobj;
> +}
> +
> +/*
> + * Does both uobj_get_destroy() and uobj_put_destroy(). Returns success_res
> + * on success (negative errno on failure). For use by callers that do not
> need
> + * the uobj.
> + */
> +int __uobj_perform_destroy(const struct uverbs_obj_type *type, u32 id,
> + struct ib_uverbs_file *ufile, int success_res)
> +{
> + struct ib_uobject *uobj;
> +
> + uobj = __uobj_get_destroy(type, id, ufile);
> + if (IS_ERR(uobj))
> + return PTR_ERR(uobj);
> +
> + /*
> + * FIXME: After destroy this is not safe. We no longer hold the rwsem
> + * so disassociation could have completed and unloaded the module that
> + * backs the uobj->type pointer.
> + */
> + rdma_lookup_put_uobject(uobj, UVERBS_LOOKUP_WRITE);
> + return success_res;
> +}
> +
> +/* alloc_uobj must be undone by uverbs_destroy_uobject() */
> +static struct ib_uobject *alloc_uobj(struct ib_uverbs_file *ufile,
> const struct uverbs_obj_type *type)
> {
> - struct ib_uobject *uobj = kzalloc(type->obj_size, GFP_KERNEL);
> + struct ib_uobject *uobj;
> + struct ib_ucontext *ucontext;
> +
> + ucontext = ib_uverbs_get_ucontext(ufile);
> + if (IS_ERR(ucontext))
> + return ERR_CAST(ucontext);
>
> + uobj = kzalloc(type->obj_size, GFP_KERNEL);
> if (!uobj)
> return ERR_PTR(-ENOMEM);
> /*This is now a conflict between Linus' tree and the rdma tree. -- Cheers, Stephen Rothwell
pgppxVoQAMR56.pgp
Description: OpenPGP digital signature
