Re: [PATCH 08/11] target iblock: add backend plug/unplug callouts

On 2/4/21 5:23 PM, Chaitanya Kulkarni wrote:

On 2/4/21 03:40, Mike Christie wrote:

quoted

This patch adds plug/unplug callouts for iblock. For initiator drivers
like iscsi which wants to pass multiple cmds to its xmit thread instead
of one cmd at a time, this increases IOPs by around 10% with vhost-scsi
(combined with the last patches we can see a total 40-50% increase). For
driver combos like tcm_loop and faster drivers like the iser initiator, we
can still see IOPs increase by 20-30% when tcm_loop's nr_hw_queues setting
is also increased.

Signed-off-by: Mike Christie <michael.christie@oracle.com>
---
  drivers/target/target_core_iblock.c | 41 ++++++++++++++++++++++++++++-
  drivers/target/target_core_iblock.h | 10 +++++++
  2 files changed, 50 insertions(+), 1 deletion(-)

diff --git a/drivers/target/target_core_iblock.c b/drivers/target/target_core_iblock.c
index 8ed93fd205c7..a4951e662615 100644
--- a/drivers/target/target_core_iblock.c
+++ b/drivers/target/target_core_iblock.c

@@ -61,9 +61,18 @@ static struct se_device *iblock_alloc_device(struct se_hba *hba, const char *nam
  		return NULL;
  	}
  
+	ib_dev->ibd_plug = kcalloc(nr_cpu_ids, sizeof(*ib_dev->ibd_plug),
+				   GFP_KERNEL);
+	if (!ib_dev->ibd_plug)
+		goto free_dev;
+
  	pr_debug( "IBLOCK: Allocated ib_dev for %s\n", name);
  
  	return &ib_dev->dev;
+
+free_dev:
+	kfree(ib_dev);
+	return NULL;
  }
  
  static int iblock_configure_device(struct se_device *dev)

@@ -171,6 +180,7 @@ static void iblock_dev_call_rcu(struct rcu_head *p)
  	struct se_device *dev = container_of(p, struct se_device, rcu_head);
  	struct iblock_dev *ib_dev = IBLOCK_DEV(dev);
  
+	kfree(ib_dev->ibd_plug);
  	kfree(ib_dev);
  }
  

@@ -188,6 +198,30 @@ static void iblock_destroy_device(struct se_device *dev)
  	bioset_exit(&ib_dev->ibd_bio_set);
  }
  
+static struct se_dev_plug *iblock_plug_device(struct se_cmd *se_cmd)
+{
+	struct se_device *se_dev = se_cmd->se_dev;
+	struct iblock_dev *ib_dev = IBLOCK_DEV(se_dev);
+	struct iblock_dev_plug *ib_dev_plug;
+
+	ib_dev_plug = &ib_dev->ibd_plug[se_cmd->cpuid];
+	if (test_and_set_bit(IBD_PLUGF_PLUGGED, &ib_dev_plug->flags))
+		return NULL;
+
+	blk_start_plug(&ib_dev_plug->blk_plug);
+	return &ib_dev_plug->se_plug;
+}
+
+static void iblock_unplug_device(struct se_dev_plug *se_plug)
+{
+	struct iblock_dev_plug *ib_dev_plug =
+				container_of(se_plug, struct iblock_dev_plug,
+					     se_plug);

I think something like on the new line read much easier for me atleast :-

         ib_dev_plug = container_of(se_plug, struct iblock_dev_plug,
se_plug);

Yeah nicer. Will change this and the other one.