From patchwork Mon May 20 12:50:54 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: James Simmons X-Patchwork-Id: 10951041 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 CCEAE17E0 for ; Mon, 20 May 2019 12:52:17 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id BBE59284ED for ; Mon, 20 May 2019 12:52:17 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id B07072882D; Mon, 20 May 2019 12:52:17 +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=-2.9 required=2.0 tests=BAYES_00,MAILING_LIST_MULTI, RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.1 Received: from pdx1-mailman02.dreamhost.com (pdx1-mailman02.dreamhost.com [64.90.62.194]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 5A96C28820 for ; Mon, 20 May 2019 12:52:17 +0000 (UTC) Received: from pdx1-mailman02.dreamhost.com (localhost [IPv6:::1]) by pdx1-mailman02.dreamhost.com (Postfix) with ESMTP id 6A2A921F957; Mon, 20 May 2019 05:51:50 -0700 (PDT) X-Original-To: lustre-devel@lists.lustre.org Delivered-To: lustre-devel-lustre.org@pdx1-mailman02.dreamhost.com Received: from smtp4.ccs.ornl.gov (smtp4.ccs.ornl.gov [160.91.203.40]) by pdx1-mailman02.dreamhost.com (Postfix) with ESMTP id 295D221E022 for ; Mon, 20 May 2019 05:51:19 -0700 (PDT) Received: from star.ccs.ornl.gov (star.ccs.ornl.gov [160.91.202.134]) by smtp4.ccs.ornl.gov (Postfix) with ESMTP id 6981C1005199; Mon, 20 May 2019 08:51:13 -0400 (EDT) Received: by star.ccs.ornl.gov (Postfix, from userid 2004) id 6810C2B7; Mon, 20 May 2019 08:51:13 -0400 (EDT) From: James Simmons To: Andreas Dilger , Oleg Drokin , NeilBrown Date: Mon, 20 May 2019 08:50:54 -0400 Message-Id: <1558356671-29599-13-git-send-email-jsimmons@infradead.org> X-Mailer: git-send-email 1.8.3.1 In-Reply-To: <1558356671-29599-1-git-send-email-jsimmons@infradead.org> References: <1558356671-29599-1-git-send-email-jsimmons@infradead.org> Subject: [lustre-devel] [PATCH v2 12/29] lustre: obdclass: fix module load locking. X-BeenThere: lustre-devel@lists.lustre.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: "For discussing Lustre software development." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Lustre Development List MIME-Version: 1.0 Errors-To: lustre-devel-bounces@lists.lustre.org Sender: "lustre-devel" X-Virus-Scanned: ClamAV using ClamSMTP From: NeilBrown Safe module loading requires that we try_module_get() in a context where the module cannot be unloaded, typically protected by a spinlock that module-unload has to take. This doesn't currently happen in class_get_type(). As free_module() calls synchronize_rcu() between calling the exit function and freeing the module, we can use rcu_read_lock() to check if the exit function has been called, and try_module_get() if it hasn't. We must also check the return status of try_module_get(). Reviewed-by: James Simmons Signed-off-by: NeilBrown --- fs/lustre/obdclass/genops.c | 25 ++++++++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/fs/lustre/obdclass/genops.c b/fs/lustre/obdclass/genops.c index 6d33414..00181e3 100644 --- a/fs/lustre/obdclass/genops.c +++ b/fs/lustre/obdclass/genops.c @@ -100,20 +100,31 @@ static struct obd_type *class_get_type(const char *name) { struct obd_type *type; + rcu_read_lock(); type = class_search_type(name); if (!type) { const char *modname = name; + rcu_read_unlock(); if (!request_module("%s", modname)) { CDEBUG(D_INFO, "Loaded module '%s'\n", modname); - type = class_search_type(name); } else { LCONSOLE_ERROR_MSG(0x158, "Can't load module '%s'\n", modname); } + rcu_read_lock(); + type = class_search_type(name); } if (type) { - if (try_module_get(type->typ_dt_ops->owner)) { + /* + * Holding rcu_read_lock() matches the synchronize_rcu() call + * in free_module() and ensures that if type->typ_dt_ops is + * not yet NULL, then the module won't be freed until after + * we rcu_read_unlock(). + */ + const struct obd_ops *dt_ops = READ_ONCE(type->typ_dt_ops); + + if (dt_ops && try_module_get(dt_ops->owner)) { atomic_inc(&type->typ_refcnt); /* class_search_type() returned a counted reference, * but we don't need that count any more as @@ -125,6 +136,7 @@ static struct obd_type *class_get_type(const char *name) type = NULL; } } + rcu_read_unlock(); return type; } @@ -209,11 +221,18 @@ int class_unregister_type(const char *name) return -EINVAL; } + /* + * Ensure that class_get_type doesn't try to get the module + * as it could be freed before the obd_type is released. + * synchronize_rcu() will be called before the module + * is freed. + */ + type->typ_dt_ops = NULL; + if (atomic_read(&type->typ_refcnt)) { CERROR("type %s has refcount (%d)\n", name, atomic_read(&type->typ_refcnt)); /* This is a bad situation, let's make the best of it */ /* Remove ops, but leave the name for debugging */ - type->typ_dt_ops = NULL; type->typ_md_ops = NULL; kobject_put(&type->typ_kobj); return -EBUSY;