target-devel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Mike Christie <mchristi@redhat.com>
To: martin.petersen@oracle.com, jejb@linux.vnet.ibm.com,
	linux-scsi@vger.kernel.org, target-devel@vger.kernel.org,
	nab@linux-iscsi.org
Cc: Mike Christie <mchristi@redhat.com>
Subject: [PATCH 07/19] tcmu: fix unmap thread race
Date: Mon, 30 Oct 2017 03:44:27 +0000	[thread overview]
Message-ID: <1509335079-5276-8-git-send-email-mchristi@redhat.com> (raw)
In-Reply-To: <1509335079-5276-1-git-send-email-mchristi@redhat.com>

If the unmap thread has already run find_free_blocks
but not yet run prepare_to_wait when a wake_up(&unmap_wait)
call is done, the unmap thread is going to miss the wake
call. Instead of adding checks for if new waiters were added
this just has use a work queue which will run us again
in this type of case.

Signed-off-by: Mike Christie <mchristi@redhat.com>
---
 drivers/target/target_core_user.c | 43 +++++++++------------------------------
 1 file changed, 10 insertions(+), 33 deletions(-)

diff --git a/drivers/target/target_core_user.c b/drivers/target/target_core_user.c
index 15b54fd..14d9b79 100644
--- a/drivers/target/target_core_user.c
+++ b/drivers/target/target_core_user.c
@@ -32,7 +32,7 @@
 #include <linux/highmem.h>
 #include <linux/configfs.h>
 #include <linux/mutex.h>
-#include <linux/kthread.h>
+#include <linux/workqueue.h>
 #include <net/genetlink.h>
 #include <scsi/scsi_common.h>
 #include <scsi/scsi_proto.h>
@@ -176,12 +176,11 @@ struct tcmu_cmd {
 	unsigned long flags;
 };
 
-static struct task_struct *unmap_thread;
-static wait_queue_head_t unmap_wait;
 static DEFINE_MUTEX(root_udev_mutex);
 static LIST_HEAD(root_udev);
 
 static atomic_t global_db_count = ATOMIC_INIT(0);
+static struct work_struct tcmu_unmap_work;
 
 static struct kmem_cache *tcmu_cmd_cache;
 
@@ -389,8 +388,7 @@ static bool tcmu_get_empty_blocks(struct tcmu_dev *udev,
 
 err:
 	udev->waiting_global = true;
-	/* Try to wake up the unmap thread */
-	wake_up(&unmap_wait);
+	schedule_work(&tcmu_unmap_work);
 	return false;
 }
 
@@ -1063,8 +1061,7 @@ static void tcmu_device_timedout(unsigned long data)
 	idr_for_each(&udev->commands, tcmu_check_expired_cmd, NULL);
 	spin_unlock_irqrestore(&udev->commands_lock, flags);
 
-	/* Try to wake up the ummap thread */
-	wake_up(&unmap_wait);
+	schedule_work(&tcmu_unmap_work);
 
 	/*
 	 * We don't need to wakeup threads on wait_cmdr since they have their
@@ -2042,23 +2039,10 @@ static void run_cmdr_queues(void)
 	mutex_unlock(&root_udev_mutex);
 }
 
-static int unmap_thread_fn(void *data)
+static void tcmu_unmap_work_fn(struct work_struct *work)
 {
-	while (!kthread_should_stop()) {
-		DEFINE_WAIT(__wait);
-
-		prepare_to_wait(&unmap_wait, &__wait, TASK_INTERRUPTIBLE);
-		schedule();
-		finish_wait(&unmap_wait, &__wait);
-
-		if (kthread_should_stop())
-			break;
-
-		find_free_blocks();
-		run_cmdr_queues();
-	}
-
-	return 0;
+	find_free_blocks();
+	run_cmdr_queues();
 }
 
 static int __init tcmu_module_init(void)
@@ -2067,6 +2051,8 @@ static int __init tcmu_module_init(void)
 
 	BUILD_BUG_ON((sizeof(struct tcmu_cmd_entry) % TCMU_OP_ALIGN_SIZE) != 0);
 
+	INIT_WORK(&tcmu_unmap_work, tcmu_unmap_work_fn);
+
 	tcmu_cmd_cache = kmem_cache_create("tcmu_cmd_cache",
 				sizeof(struct tcmu_cmd),
 				__alignof__(struct tcmu_cmd),
@@ -2112,17 +2098,8 @@ static int __init tcmu_module_init(void)
 	if (ret)
 		goto out_attrs;
 
-	init_waitqueue_head(&unmap_wait);
-	unmap_thread = kthread_run(unmap_thread_fn, NULL, "tcmu_unmap");
-	if (IS_ERR(unmap_thread)) {
-		ret = PTR_ERR(unmap_thread);
-		goto out_unreg_transport;
-	}
-
 	return 0;
 
-out_unreg_transport:
-	target_backend_unregister(&tcmu_ops);
 out_attrs:
 	kfree(tcmu_attrs);
 out_unreg_genl:
@@ -2137,7 +2114,7 @@ static int __init tcmu_module_init(void)
 
 static void __exit tcmu_module_exit(void)
 {
-	kthread_stop(unmap_thread);
+	cancel_work_sync(&tcmu_unmap_work);
 	target_backend_unregister(&tcmu_ops);
 	kfree(tcmu_attrs);
 	genl_unregister_family(&tcmu_genl_family);
-- 
1.8.3.1


  parent reply	other threads:[~2017-10-30  3:44 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-10-30  3:44 [PATCH 00/19] target/target_core_user: changes for 4.16 Mike Christie
2017-10-30  3:44 ` [PATCH 01/19] tcmu: fix crash when removing the tcmu device v4 Mike Christie
2017-10-30  3:44 ` [PATCH 02/19] tcmu: Add netlink command reply supported option for each device Mike Christie
2017-10-30  3:44 ` [PATCH 03/19] tcmu: Use macro to call container_of in tcmu_cmd_time_out_show Mike Christie
2017-10-30  3:44 ` [PATCH 04/19] tcmu: fix double se_cmd completion Mike Christie
2017-10-30  3:44 ` [PATCH 05/19] tcmu: merge common block release code Mike Christie
2017-10-30  3:44 ` [PATCH 06/19] tcmu: split unmap_thread_fn Mike Christie
2017-10-30  3:44 ` Mike Christie [this message]
2017-10-30  3:44 ` [PATCH 08/19] tcmu: move expired command completion to unmap thread Mike Christie
2017-10-30  3:44 ` [PATCH 09/19] tcmu: remove commands_lock Mike Christie
2017-10-30  3:44 ` [PATCH 10/19] tcmu: release blocks for partially setup cmds Mike Christie
2017-10-30  3:44 ` [PATCH 11/19] tcmu: simplify scatter_data_area error handling Mike Christie
2017-10-30  3:44 ` [PATCH 12/19] tcmu: fix free block calculation Mike Christie
2017-10-30  3:44 ` [PATCH 13/19] tcmu: clean up the scatter helper Mike Christie
2017-10-30  3:44 ` [PATCH 14/19] tcmu: prep queue_cmd_ring to be used by unmap wq Mike Christie
2017-10-30  3:44 ` [PATCH 15/19] tcmu: simplify dbi thresh handling Mike Christie
2017-10-30  3:44 ` [PATCH 16/19] tcmu: don't block submitting context for block waits Mike Christie
2017-10-30  3:44 ` [PATCH 17/19] tcmu: make ring buffer timer configurable Mike Christie
2017-10-30  3:44 ` [PATCH 18/19] tcmu: allow max block and global max blocks to be settable Mike Christie
2017-10-30  3:44 ` [PATCH 19/19] target: return SAM_STAT_TASK_SET_FULL for TCM_OUT_OF_RESOURCES Mike Christie

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1509335079-5276-8-git-send-email-mchristi@redhat.com \
    --to=mchristi@redhat.com \
    --cc=jejb@linux.vnet.ibm.com \
    --cc=linux-scsi@vger.kernel.org \
    --cc=martin.petersen@oracle.com \
    --cc=nab@linux-iscsi.org \
    --cc=target-devel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).