From patchwork Thu Jul 26 03:40:14 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jason Gunthorpe X-Patchwork-Id: 10545137 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 9AA12139A for ; Thu, 26 Jul 2018 03:40:29 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 84A742ACA5 for ; Thu, 26 Jul 2018 03:40:29 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 6C03B2A66A; Thu, 26 Jul 2018 03:40:29 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.8 required=2.0 tests=BAYES_00,DKIM_SIGNED, MAILING_LIST_MULTI,RCVD_IN_DNSWL_HI,T_DKIM_INVALID autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 696CA2A66A for ; Thu, 26 Jul 2018 03:40:28 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726375AbeGZEzL (ORCPT ); Thu, 26 Jul 2018 00:55:11 -0400 Received: from mail-io0-f194.google.com ([209.85.223.194]:41169 "EHLO mail-io0-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726072AbeGZEzL (ORCPT ); Thu, 26 Jul 2018 00:55:11 -0400 Received: by mail-io0-f194.google.com with SMTP id q9-v6so239294ioj.8 for ; Wed, 25 Jul 2018 20:40:25 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ziepe.ca; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references; bh=TxZucZeQmJD57bJyO4T7aTEgXk9bbr4LFqLxf04f9W4=; b=M3EsJ/zmcmPaxfa47p0SmqKc5TV70Leo3szkSK+rzxrH7gxdqItTK3fFWmU5Pl1nbS 45VkGbpwYtQ3tvZbQtzjMigY/OiP83/Fre8gt5mHvOUCvvLwRXBKqXDPywDE2k77c98M zZkijFZsUpYM3q3oAsnU5XSacreLiv6WfEXQgPfTRuM+PKfBmmZkZzN06Gabkqw2fD9k OMHfP0jal3bgRfcm8PzFxsvdbRSj9Y76z+p+rIKfiTRoK7mhf9e8YK1dwII1ntfSzdDj W2T0fxtZBj3kmmVNIzFqpkRfjcGP+ILGu1koSmakghcLD9Nskv5CNaNt1dlgTbjrbuNM jWYw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=TxZucZeQmJD57bJyO4T7aTEgXk9bbr4LFqLxf04f9W4=; b=k28EZJA5tM+am33JXr3Vzu0Pmbm0wdF7cKCv36oNeyPP2Ae+OQg0cHc+ZaJcupJgJm j7bNAtDYmbScxmM979Q4tscE29xs/wMd3oMepk7eKxCh0B3qHbF3RDMD7DVLcG4cPy4I 0A98PCmZkVh1kRSYak6pEbKloRV8NdBiUp8/ZYFfp5PJ7zaJB8XRxsD0EpesCl6vI1K5 kWpBVDWDkEe8UmsabpwqoiyGE1+3alEEiPXV3eJ6tdsbTISD7jaBJia3C2z8v6N85KMi THsHle1rJ2ACefFJ6lxQJQY0yghD1Bf1lXlqAx1tCjGy6ifd2zK4GMhIh3yj+Wcn/z4l bIIQ== X-Gm-Message-State: AOUpUlE8V0UQB7t7HXE/9jHQigzngyRy5434511vHDdEWcWAnN05WxNJ tUCaR95xD/toByN7jy94TjkJ5f2Zd8gOHg== X-Google-Smtp-Source: AAOMgpeKCFxeSdw6YmtJHuaVjLKKXQeusSt9P5vjhuQO6eCw9JZLpl8ysygtWlrO9UvIB7RexOghxQ== X-Received: by 2002:a6b:4e04:: with SMTP id c4-v6mr174919iob.19.1532576425288; Wed, 25 Jul 2018 20:40:25 -0700 (PDT) Received: from ziepe.ca (S010614cc2056d97f.ed.shawcable.net. [174.3.196.123]) by smtp.gmail.com with ESMTPSA id x17-v6sm294250ith.24.2018.07.25.20.40.24 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 25 Jul 2018 20:40:24 -0700 (PDT) Received: from jgg by mlx.ziepe.ca with local (Exim 4.86_2) (envelope-from ) id 1fiX8Z-0001Wh-LW; Wed, 25 Jul 2018 21:40:23 -0600 From: Jason Gunthorpe To: linux-rdma@vger.kernel.org, Leon Romanovsky , "Guy Levi(SW)" , Yishai Hadas , "Ruhl, Michael J" Cc: Jason Gunthorpe Subject: [PATCH 05/11] IB/uverbs: Allow RDMA_REMOVE_DESTROY to work concurrently with disassociate Date: Wed, 25 Jul 2018 21:40:14 -0600 Message-Id: <20180726034020.5583-6-jgg@ziepe.ca> X-Mailer: git-send-email 2.18.0 In-Reply-To: <20180726034020.5583-1-jgg@ziepe.ca> References: <20180726034020.5583-1-jgg@ziepe.ca> Sender: linux-rdma-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-rdma@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP From: Jason Gunthorpe After all the recent structural changes this is now straightfoward, hoist the hw_destroy_rwsem up out of rdma_destroy_explicit and wrap it around the uobject write lock as well as the destroy. This is necessary as obtaining a write lock concurrently with uverbs_destroy_ufile_hw() will cause malfunction. After this change none of the destroy callbacks require the disassociate_srcu lock to be correct. This requires introducing a new lookup mode, UVERBS_LOOKUP_DESTROY as the IOCTL interface needs to hold an unlocked kref until all command verification is completed. Signed-off-by: Jason Gunthorpe --- drivers/infiniband/core/rdma_core.c | 71 ++++++++++++++++++-------- drivers/infiniband/core/rdma_core.h | 2 + drivers/infiniband/core/uverbs_ioctl.c | 7 ++- include/rdma/uverbs_types.h | 7 ++- 4 files changed, 63 insertions(+), 24 deletions(-) diff --git a/drivers/infiniband/core/rdma_core.c b/drivers/infiniband/core/rdma_core.c index 435dbe8ef2a28d..81d668abe18e45 100644 --- a/drivers/infiniband/core/rdma_core.c +++ b/drivers/infiniband/core/rdma_core.c @@ -127,8 +127,10 @@ static int uverbs_try_lock_object(struct ib_uobject *uobj, return __atomic_add_unless(&uobj->usecnt, 1, -1) == -1 ? -EBUSY : 0; case UVERBS_LOOKUP_WRITE: - /* lock is either WRITE or DESTROY - should be exclusive */ + /* lock is exclusive */ return atomic_cmpxchg(&uobj->usecnt, 0, -1) == 0 ? 0 : -EBUSY; + case UVERBS_LOOKUP_DESTROY: + return 0; } return 0; } @@ -144,6 +146,8 @@ static void assert_uverbs_usecnt(struct ib_uobject *uobj, case UVERBS_LOOKUP_WRITE: WARN_ON(atomic_read(&uobj->usecnt) != -1); break; + case UVERBS_LOOKUP_DESTROY: + break; } #endif } @@ -227,6 +231,35 @@ static int uverbs_destroy_uobject(struct ib_uobject *uobj, return 0; } +/* + * 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 @@ -238,13 +271,13 @@ struct ib_uobject *__uobj_get_destroy(const struct uverbs_obj_type *type, struct ib_uobject *uobj; int ret; - uobj = rdma_lookup_get_uobject(type, ufile, id, UVERBS_LOOKUP_WRITE); + uobj = rdma_lookup_get_uobject(type, ufile, id, UVERBS_LOOKUP_DESTROY); if (IS_ERR(uobj)) return uobj; - ret = rdma_explicit_destroy(uobj); + ret = uobj_destroy(uobj); if (ret) { - rdma_lookup_put_uobject(uobj, UVERBS_LOOKUP_WRITE); + rdma_lookup_put_uobject(uobj, UVERBS_LOOKUP_DESTROY); return ERR_PTR(ret); } @@ -265,6 +298,11 @@ int __uobj_perform_destroy(const struct uverbs_obj_type *type, u32 id, 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; } @@ -534,23 +572,6 @@ static int __must_check remove_commit_fd_uobject(struct ib_uobject *uobj, return 0; } -int rdma_explicit_destroy(struct ib_uobject *uobject) -{ - int ret; - struct ib_uverbs_file *ufile = uobject->ufile; - - /* Cleanup is running. Calling this should have been impossible */ - if (!down_read_trylock(&ufile->hw_destroy_rwsem)) { - WARN(true, "ib_uverbs: Cleanup is running while removing an uobject\n"); - return 0; - } - - ret = uverbs_destroy_uobject(uobject, RDMA_REMOVE_DESTROY); - - up_read(&ufile->hw_destroy_rwsem); - return ret; -} - static int alloc_commit_idr_uobject(struct ib_uobject *uobj) { struct ib_uverbs_file *ufile = uobj->ufile; @@ -686,6 +707,8 @@ void rdma_lookup_put_uobject(struct ib_uobject *uobj, case UVERBS_LOOKUP_WRITE: atomic_set(&uobj->usecnt, 0); break; + case UVERBS_LOOKUP_DESTROY: + break; } /* Pairs with the kref obtained by type->lookup_get */ @@ -911,6 +934,9 @@ uverbs_get_uobject_from_file(const struct uverbs_obj_type *type_attrs, return rdma_lookup_get_uobject(type_attrs, ufile, id, UVERBS_LOOKUP_READ); case UVERBS_ACCESS_DESTROY: + /* Actual destruction is done inside uverbs_handle_method */ + return rdma_lookup_get_uobject(type_attrs, ufile, id, + UVERBS_LOOKUP_DESTROY); case UVERBS_ACCESS_WRITE: return rdma_lookup_get_uobject(type_attrs, ufile, id, UVERBS_LOOKUP_WRITE); @@ -942,7 +968,8 @@ int uverbs_finalize_object(struct ib_uobject *uobj, rdma_lookup_put_uobject(uobj, UVERBS_LOOKUP_WRITE); break; case UVERBS_ACCESS_DESTROY: - rdma_lookup_put_uobject(uobj, UVERBS_LOOKUP_WRITE); + if (uobj) + rdma_lookup_put_uobject(uobj, UVERBS_LOOKUP_DESTROY); break; case UVERBS_ACCESS_NEW: if (commit) diff --git a/drivers/infiniband/core/rdma_core.h b/drivers/infiniband/core/rdma_core.h index a736b46d18e34c..e4d8b985c31135 100644 --- a/drivers/infiniband/core/rdma_core.h +++ b/drivers/infiniband/core/rdma_core.h @@ -52,6 +52,8 @@ const struct uverbs_method_spec *uverbs_get_method(const struct uverbs_object_sp void uverbs_destroy_ufile_hw(struct ib_uverbs_file *ufile, enum rdma_remove_reason reason); +int uobj_destroy(struct ib_uobject *uobj); + /* * uverbs_uobject_get is called in order to increase the reference count on * an uobject. This is useful when a handler wants to keep the uobject's memory diff --git a/drivers/infiniband/core/uverbs_ioctl.c b/drivers/infiniband/core/uverbs_ioctl.c index 703710085b5beb..204130ee1cbe59 100644 --- a/drivers/infiniband/core/uverbs_ioctl.c +++ b/drivers/infiniband/core/uverbs_ioctl.c @@ -347,13 +347,18 @@ static int uverbs_handle_method(struct ib_uverbs_attr __user *uattr_ptr, * not get to manipulate the HW objects. */ if (destroy_attr) { - ret = rdma_explicit_destroy(destroy_attr->uobject); + ret = uobj_destroy(destroy_attr->uobject); if (ret) goto cleanup; } ret = method_spec->handler(ibdev, ufile, attr_bundle); + if (destroy_attr) { + uobj_put_destroy(destroy_attr->uobject); + destroy_attr->uobject = NULL; + } + cleanup: finalize_ret = uverbs_finalize_attrs(attr_bundle, method_spec->attr_buckets, diff --git a/include/rdma/uverbs_types.h b/include/rdma/uverbs_types.h index 0676672dbbb995..f64f413cecac22 100644 --- a/include/rdma/uverbs_types.h +++ b/include/rdma/uverbs_types.h @@ -41,6 +41,12 @@ struct uverbs_obj_type; enum rdma_lookup_mode { UVERBS_LOOKUP_READ, UVERBS_LOOKUP_WRITE, + /* + * Destroy is like LOOKUP_WRITE, except that the uobject is not + * locked. uobj_destroy is used to convert a LOOKUP_DESTROY lock into + * a LOOKUP_WRITE lock. + */ + UVERBS_LOOKUP_DESTROY, }; /* @@ -129,7 +135,6 @@ struct ib_uobject *rdma_alloc_begin_uobject(const struct uverbs_obj_type *type, struct ib_uverbs_file *ufile); void rdma_alloc_abort_uobject(struct ib_uobject *uobj); int __must_check rdma_alloc_commit_uobject(struct ib_uobject *uobj); -int rdma_explicit_destroy(struct ib_uobject *uobject); struct uverbs_obj_fd_type { /*