diff mbox series

[09/11] target_core_user: add backend plug/unplug callouts

Message ID 20210204113513.93204-10-michael.christie@oracle.com
State Superseded
Headers show
Series target: fix cmd plugging and completion | expand

Commit Message

Mike Christie Feb. 4, 2021, 11:35 a.m. UTC 
This patch adds plug/unplug callouts for tcmu, so we can avoid the
number of times we switch to userspace. Using this driver with tcm
loop is a common config, and dependng on the nr_hw_queues and fio
jobs this patch can increase IOPs by only around 5% because we
hit other issues like the big per tcmu device mutex.

Bodo, because the improvement is so small I'm not sure if we
want this patch. I was thinking when you fix those other issues
you've been working on then it might be more useful.

Signed-off-by: Mike Christie <michael.christie@oracle.com>
---
 drivers/target/target_core_user.c | 28 ++++++++++++++++++++++++++--
 1 file changed, 26 insertions(+), 2 deletions(-)

Comments

Mike Christie Feb. 7, 2021, 9:37 p.m. UTC | #1 
On 2/4/21 5:25 PM, Chaitanya Kulkarni wrote:
>>   * queue_cmd_ring - queue cmd to ring or internally

>>   * @tcmu_cmd: cmd to queue

>> @@ -1086,8 +1108,8 @@ static int queue_cmd_ring(struct tcmu_cmd *tcmu_cmd, sense_reason_t *scsi_err)

>>  

>>  	list_add_tail(&tcmu_cmd->queue_entry, &udev->inflight_queue);

>>  

>> -	/* TODO: only if FLUSH and FUA? */

>> -	uio_event_notify(&udev->uio_info);

>> +	if (!test_bit(TCM_DEV_BIT_PLUGGED, &udev->flags))

>> +		uio_event_notify(&udev->uio_info);

>>  

> Do we need to keep the TODO ?

I think it's not helpful.

The reason for the TODO was to avoid calling uio_event_notify for
every command. I think we had thought we could just key of a FLUSH
but then later figured out we might not always get one so that wouldn't
work. The comment should have been removed or if we like to keep TODOs
like that in code it should have been updated to better reflect what the
issue was and the idea to fix it.
diff mbox series

Patch

diff --git a/drivers/target/target_core_user.c b/drivers/target/target_core_user.c
index a5991df23581..d67be2f959b9 100644
--- a/drivers/target/target_core_user.c
+++ b/drivers/target/target_core_user.c
@@ -111,6 +111,7 @@  struct tcmu_dev {
 	struct kref kref;
 
 	struct se_device se_dev;
+	struct se_dev_plug se_plug;
 
 	char *name;
 	struct se_hba *hba;
@@ -119,6 +120,7 @@  struct tcmu_dev {
 #define TCMU_DEV_BIT_BROKEN 1
 #define TCMU_DEV_BIT_BLOCKED 2
 #define TCMU_DEV_BIT_TMR_NOTIFY 3
+#define TCM_DEV_BIT_PLUGGED 4
 	unsigned long flags;
 
 	struct uio_info uio_info;
@@ -959,6 +961,26 @@  static uint32_t ring_insert_padding(struct tcmu_dev *udev, size_t cmd_size)
 	return cmd_head;
 }
 
+static void tcmu_unplug_device(struct se_dev_plug *se_plug)
+{
+	struct se_device *se_dev = se_plug->se_dev;
+	struct tcmu_dev *udev = TCMU_DEV(se_dev);
+
+	uio_event_notify(&udev->uio_info);
+	clear_bit(TCM_DEV_BIT_PLUGGED, &udev->flags);
+}
+
+static struct se_dev_plug *tcmu_plug_device(struct se_cmd *se_cmd)
+{
+	struct se_device *se_dev = se_cmd->se_dev;
+	struct tcmu_dev *udev = TCMU_DEV(se_dev);
+
+	if (!test_and_set_bit(TCM_DEV_BIT_PLUGGED, &udev->flags))
+		return &udev->se_plug;
+
+	return NULL;
+}
+
 /**
  * queue_cmd_ring - queue cmd to ring or internally
  * @tcmu_cmd: cmd to queue
@@ -1086,8 +1108,8 @@  static int queue_cmd_ring(struct tcmu_cmd *tcmu_cmd, sense_reason_t *scsi_err)
 
 	list_add_tail(&tcmu_cmd->queue_entry, &udev->inflight_queue);
 
-	/* TODO: only if FLUSH and FUA? */
-	uio_event_notify(&udev->uio_info);
+	if (!test_bit(TCM_DEV_BIT_PLUGGED, &udev->flags))
+		uio_event_notify(&udev->uio_info);
 
 	return 0;
 
@@ -2840,6 +2862,8 @@  static struct target_backend_ops tcmu_ops = {
 	.configure_device	= tcmu_configure_device,
 	.destroy_device		= tcmu_destroy_device,
 	.free_device		= tcmu_free_device,
+	.unplug_device		= tcmu_unplug_device,
+	.plug_device		= tcmu_plug_device,
 	.parse_cdb		= tcmu_parse_cdb,
 	.tmr_notify		= tcmu_tmr_notify,
 	.set_configfs_dev_params = tcmu_set_configfs_dev_params,