Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Bart Van Assche <bvanassche@acm.org>
To:     "Martin K . Petersen" <martin.petersen@oracle.com>
Cc:     linux-scsi@vger.kernel.org, Bart Van Assche <bvanassche@acm.org>,
        Luis Chamberlain <mcgrof@kernel.org>,
        Christoph Hellwig <hch@lst.de>, Ming Lei <ming.lei@redhat.com>,
        Hannes Reinecke <hare@suse.de>,
        John Garry <john.garry@huawei.com>,
        Mike Christie <michael.christie@oracle.com>,
        Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        linux-modules@vger.kernel.org, linux-kernel@vger.kernel.org,
        "James E.J. Bottomley" <jejb@linux.ibm.com>,
        Tejun Heo <tj@kernel.org>
Subject: [PATCH v5 6/7] module: Improve support for asynchronous module exit code
Date:   Wed, 14 Sep 2022 15:56:20 -0700
Message-Id: <20220914225621.415631-7-bvanassche@acm.org>
In-Reply-To: <20220914225621.415631-1-bvanassche@acm.org>
References: <20220914225621.415631-1-bvanassche@acm.org>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

Some kernel modules call device_del() from their module exit code and
schedule asynchronous work from inside the .release callback without waiting
until that callback has finished. As an example, many SCSI LLD drivers call
scsi_remove_host() from their module exit code. scsi_remove_host() may
invoke scsi_device_dev_release_usercontext() asynchronously.
scsi_device_dev_release_usercontext() uses the host template pointer and
that pointer usually exists in static storage in the SCSI LLD. Support
using the module reference count to keep the module around until
asynchronous module exiting has completed by waiting in the delete_module()
system call until the module reference count drops to zero.

The following debug patch has been used to make the new wait_event()
call wait:

diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index 8be8e08fb67d..fead694ff95a 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -14,6 +14,7 @@
 #include <linux/device.h>
 #include <linux/pm_runtime.h>
 #include <linux/bsg.h>
+#include <linux/delay.h>

 #include <scsi/scsi.h>
 #include <scsi/scsi_device.h>
@@ -518,6 +519,7 @@ static void scsi_device_dev_release_usercontext(struct work_struct *work)

 	if (parent)
 		put_device(parent);
+	msleep(100);
 	module_put(mod);
 }

diff --git a/kernel/module/main.c b/kernel/module/main.c
index a271126d7d59..0bf75ec3f5a8 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -756,8 +756,10 @@ SYSCALL_DEFINE2(delete_module, const char __user *, name_user,
 	 * unloading is not forced, wait for the module reference count to drop
 	 * to zero again.
 	 */
-	if (!forced)
+	if (!forced) {
+		WARN_ON_ONCE(atomic_read(&mod->refcnt));
 		wait_event(mod->refcnt_wq, atomic_read(&mod->refcnt) == 0);
+	}
 	blocking_notifier_call_chain(&module_notify_list,
 				     MODULE_STATE_GOING, mod);
 	klp_module_going(mod);
diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index aeea9731ef80..f021625f2caa 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -3355,7 +3355,7 @@ int schedule_on_each_cpu(work_func_t func)
  */
 int execute_in_process_context(work_func_t fn, struct execute_work *ew)
 {
-	if (!in_interrupt()) {
+	if (false && !in_interrupt()) {
 		fn(&ew->work);
 		return 0;
 	}

Cc: Luis Chamberlain <mcgrof@kernel.org>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Ming Lei <ming.lei@redhat.com>
Cc: Hannes Reinecke <hare@suse.de>
Cc: John Garry <john.garry@huawei.com>
Cc: Mike Christie <michael.christie@oracle.com>
Cc: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: linux-modules@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 include/linux/module.h |  1 +
 kernel/module/main.c   | 10 ++++++++++
 2 files changed, 11 insertions(+)

diff --git a/include/linux/module.h b/include/linux/module.h
index 518296ea7f73..3a77d2bd4198 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -533,6 +533,7 @@ struct module {
 	/* Destruction function. */
 	void (*exit)(void);
 
+	wait_queue_head_t refcnt_wq;
 	atomic_t refcnt;
 #endif
 
diff --git a/kernel/module/main.c b/kernel/module/main.c
index a4e4d84b6f4e..a271126d7d59 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -550,6 +550,7 @@ static int module_unload_init(struct module *mod)
 
 	/* Hold reference count during initialization. */
 	atomic_inc(&mod->refcnt);
+	init_waitqueue_head(&mod->refcnt_wq);
 
 	return 0;
 }
@@ -750,6 +751,13 @@ SYSCALL_DEFINE2(delete_module, const char __user *, name_user,
 	/* Final destruction now no one is using it. */
 	if (mod->exit != NULL)
 		mod->exit();
+	/*
+	 * If the module reference count was increased by mod->exit() and if
+	 * unloading is not forced, wait for the module reference count to drop
+	 * to zero again.
+	 */
+	if (!forced)
+		wait_event(mod->refcnt_wq, atomic_read(&mod->refcnt) == 0);
 	blocking_notifier_call_chain(&module_notify_list,
 				     MODULE_STATE_GOING, mod);
 	klp_module_going(mod);
@@ -854,6 +862,8 @@ void module_put(struct module *module)
 		WARN_ON(ret < 0);	/* Failed to put refcount */
 		trace_module_put(module, _RET_IP_);
 		preempt_enable();
+		if (ret == 0)
+			wake_up(&module->refcnt_wq);
 	}
 }
 EXPORT_SYMBOL(module_put);