Re: [PATCH 17/17] tcmu: allow max block and global max blocks to be settable

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Mike,

We are testing this patch, and it looks good to me.

But one question about the tcmu_set_configfs_dev_params().

From the configshell code, we can see that the chars after ',' will be discarded by configshell. How did u test of this?

For now I'm using the targetcli tool by just adding ',' support in configshell, and test this patch works well.

Thanks,
BRs
Xiubo



On 2017年10月18日 15:14, Mike Christie wrote:
Users might have a physical system to a target so they could
have a lot more than 2 gigs of memory they want to devote to
tcmu. OTOH, we could be running in a vm and so a 2 gig
global and 1 gig per dev limit might be too high. This patch
allows the user to specify the limits.

Signed-off-by: Mike Christie <mchristi@xxxxxxxxxx>
---
  drivers/target/target_core_user.c | 153 +++++++++++++++++++++++++++++++-------
  1 file changed, 128 insertions(+), 25 deletions(-)

diff --git a/drivers/target/target_core_user.c b/drivers/target/target_core_user.c
index 1301b53..24bba6b 100644
--- a/drivers/target/target_core_user.c
+++ b/drivers/target/target_core_user.c
@@ -74,18 +74,17 @@
/*
   * For data area, the block size is PAGE_SIZE and
- * the total size is 256K * PAGE_SIZE.
+ * the total default size is 256K * PAGE_SIZE.
   */
  #define DATA_BLOCK_SIZE PAGE_SIZE
-#define DATA_BLOCK_BITS (256 * 1024)
-#define DATA_SIZE (DATA_BLOCK_BITS * DATA_BLOCK_SIZE)
+#define DATA_BLOCK_SHIFT PAGE_SHIFT
+#define TCMU_MBS_TO_BLOCKS(_mbs) (_mbs << (20 - DATA_BLOCK_SHIFT))
+#define TCMU_BLOCKS_TO_MBS(_blocks) (_blocks >> (20 - DATA_BLOCK_SHIFT))
+#define DEF_DATA_BLOCK_BITS (256 * 1024)
  #define DATA_BLOCK_INIT_BITS 128
-/* The total size of the ring is 8M + 256K * PAGE_SIZE */
-#define TCMU_RING_SIZE (CMDR_SIZE + DATA_SIZE)
-
  /* Default maximum of the global data blocks(512K * PAGE_SIZE) */
-#define TCMU_GLOBAL_MAX_BLOCKS (512 * 1024)
+#define DEF_TCMU_GLOBAL_MAX_BLOCKS (512 * 1024)
static u8 tcmu_kern_cmd_reply_supported; @@ -130,6 +129,8 @@ struct tcmu_dev {
  	/* Must add data_off and mb_addr to get the address */
  	size_t data_off;
  	size_t data_size;
+	uint32_t max_blocks;
+	size_t ring_size;
struct mutex cmdr_lock;
  	struct list_head cmdr_queue;
@@ -137,7 +138,7 @@ struct tcmu_dev {
  	uint32_t waiting_blocks;
  	uint32_t dbi_max;
  	uint32_t dbi_thresh;
-	DECLARE_BITMAP(data_bitmap, DATA_BLOCK_BITS);
+	unsigned long *data_bitmap;
  	struct radix_tree_root data_blocks;
struct idr commands;
@@ -204,6 +205,51 @@ static DEFINE_SPINLOCK(root_udev_waiter_lock);
  static LIST_HEAD(root_udev_waiter);
static atomic_t global_db_count = ATOMIC_INIT(0);
+static int tcmu_global_max_blocks = DEF_TCMU_GLOBAL_MAX_BLOCKS;
+
+static int tcmu_set_global_max_data_area(const char *str,
+					 const struct kernel_param *kp)
+{
+	int ret, max_area_mb;
+
+	mutex_lock(&root_udev_mutex);
+	if (!list_empty(&root_udev)) {
+		mutex_unlock(&root_udev_mutex);
+		pr_err("Cannot set global_max_data_area. Devices are accessing the global page pool\n");
+		return -EINVAL;
+	}
+	mutex_unlock(&root_udev_mutex);
+
+	ret = kstrtoint(str, 10, &max_area_mb);
+	if (ret)
+		return -EINVAL;
+
+	if (!max_area_mb) {
+		pr_err("global_max_data_area must be larger than 0.\n");
+		return -EINVAL;
+	}
+
+	tcmu_global_max_blocks = TCMU_MBS_TO_BLOCKS(max_area_mb);
+	return 0;
+}
+
+static int tcmu_get_global_max_data_area(char *buffer,
+					 const struct kernel_param *kp)
+{
+	return sprintf(buffer, "%d", TCMU_BLOCKS_TO_MBS(tcmu_global_max_blocks));
+}
+
+static const struct kernel_param_ops tcmu_global_max_data_area_op = {
+	.set = tcmu_set_global_max_data_area,
+	.get = tcmu_get_global_max_data_area,
+};
+
+module_param_cb(global_max_data_area_mb, &tcmu_global_max_data_area_op, NULL,
+		S_IWUSR | S_IRUGO);
+MODULE_PARM_DESC(global_max_data_area_mb,
+		 "Max MBs allowed to be allocated to all the tcmu device's "
+		 "data areas.");
+
  struct work_struct tcmu_unmap_work;
static struct kmem_cache *tcmu_cmd_cache;
@@ -368,7 +414,7 @@ static inline bool tcmu_get_empty_block(struct tcmu_dev *udev,
  	page = radix_tree_lookup(&udev->data_blocks, dbi);
  	if (!page) {
  		if (atomic_add_return(1, &global_db_count) >
-					TCMU_GLOBAL_MAX_BLOCKS) {
+				      tcmu_global_max_blocks) {
  			atomic_dec(&global_db_count);
  			return false;
  		}
@@ -716,8 +762,8 @@ static bool is_ring_space_avail(struct tcmu_dev *udev, struct tcmu_cmd *cmd,
  	/* try to check and get the data blocks as needed */
  	space = spc_bitmap_free(udev->data_bitmap, udev->dbi_thresh);
  	if ((space * DATA_BLOCK_SIZE) < data_needed) {
-		unsigned long blocks_left = DATA_BLOCK_BITS - udev->dbi_thresh +
-						space;
+		unsigned long blocks_left = udev->max_blocks -
+						udev->dbi_thresh + space;
  		unsigned long grow;
if (blocks_left < blocks_needed) {
@@ -737,12 +783,12 @@ static bool is_ring_space_avail(struct tcmu_dev *udev, struct tcmu_cmd *cmd,
  			/*
  			 * Grow the data area by max(blocks needed,
  			 * dbi_thresh / 2), but limited to the max
-			 * DATA_BLOCK_BITS size.
+			 * udev max_blocks size.
  			 */
  			grow = max(blocks_needed, udev->dbi_thresh / 2);
  			udev->dbi_thresh += grow;
-			if (udev->dbi_thresh > DATA_BLOCK_BITS)
-				udev->dbi_thresh = DATA_BLOCK_BITS;
+			if (udev->dbi_thresh > udev->max_blocks)
+				udev->dbi_thresh = udev->max_blocks;
  		}
  	}
@@ -1196,7 +1242,7 @@ static struct se_device *tcmu_alloc_device(struct se_hba *hba, const char *name)
  	udev->cmd_time_out = TCMU_TIME_OUT;
  	/* for backwards compat use the cmd_time_out */
  	udev->qfull_time_out = TCMU_TIME_OUT;
-
+	udev->max_blocks = DEF_DATA_BLOCK_BITS;
  	mutex_init(&udev->cmdr_lock);
INIT_LIST_HEAD(&udev->timedout_entry);
@@ -1281,7 +1327,7 @@ static int tcmu_irqcontrol(struct uio_info *info, s32 irq_on)
mutex_lock(&udev->cmdr_lock); - if (atomic_read(&global_db_count) == TCMU_GLOBAL_MAX_BLOCKS) {
+	if (atomic_read(&global_db_count) == tcmu_global_max_blocks) {
  		spin_lock(&root_udev_waiter_lock);
  		if (!list_empty(&root_udev_waiter)) {
  			/*
@@ -1369,7 +1415,7 @@ static struct page *tcmu_try_get_block_page(struct tcmu_dev *udev, uint32_t dbi)
/*
  		 * Since this case is rare in page fault routine, here we
-		 * will allow the global_db_count >= TCMU_GLOBAL_MAX_BLOCKS
+		 * will allow the global_db_count >= tcmu_global_max_blocks
  		 * to reduce possible page fault call trace.
  		 */
  		atomic_inc(&global_db_count);
@@ -1430,7 +1476,7 @@ static int tcmu_mmap(struct uio_info *info, struct vm_area_struct *vma)
  	vma->vm_private_data = udev;
/* Ensure the mmap is exactly the right size */
-	if (vma_pages(vma) != (TCMU_RING_SIZE >> PAGE_SHIFT))
+	if (vma_pages(vma) != (udev->ring_size >> PAGE_SHIFT))
  		return -EINVAL;
return 0;
@@ -1512,6 +1558,7 @@ static void tcmu_dev_kref_release(struct kref *kref)
  	WARN_ON(!all_expired);
tcmu_blocks_release(&udev->data_blocks, 0, udev->dbi_max + 1);
+	kfree(udev->data_bitmap);
  	mutex_unlock(&udev->cmdr_lock);
call_rcu(&dev->rcu_head, tcmu_dev_call_rcu);
@@ -1688,6 +1735,11 @@ static int tcmu_configure_device(struct se_device *dev)
info = &udev->uio_info; + udev->data_bitmap = kzalloc(BITS_TO_LONGS(udev->max_blocks) *
+				    sizeof(unsigned long), GFP_KERNEL);
+	if (!udev->data_bitmap)
+		goto err_bitmap_alloc;
+
  	udev->mb_addr = vzalloc(CMDR_SIZE);
  	if (!udev->mb_addr) {
  		ret = -ENOMEM;
@@ -1697,7 +1749,7 @@ static int tcmu_configure_device(struct se_device *dev)
  	/* mailbox fits in first part of CMDR space */
  	udev->cmdr_size = CMDR_SIZE - CMDR_OFF;
  	udev->data_off = CMDR_SIZE;
-	udev->data_size = DATA_SIZE;
+	udev->data_size = udev->max_blocks * DATA_BLOCK_SIZE;
  	udev->dbi_thresh = 0; /* Default in Idle state */
  	udev->waiting_blocks = 0;
@@ -1716,7 +1768,7 @@ static int tcmu_configure_device(struct se_device *dev) info->mem[0].name = "tcm-user command & data buffer";
  	info->mem[0].addr = (phys_addr_t)(uintptr_t)udev->mb_addr;
-	info->mem[0].size = TCMU_RING_SIZE;
+	info->mem[0].size = udev->ring_size = udev->data_size + CMDR_SIZE;
  	info->mem[0].memtype = UIO_MEM_NONE;
info->irqcontrol = tcmu_irqcontrol;
@@ -1769,6 +1821,9 @@ static int tcmu_configure_device(struct se_device *dev)
  	vfree(udev->mb_addr);
  	udev->mb_addr = NULL;
  err_vzalloc:
+	kfree(udev->data_bitmap);
+	udev->data_bitmap = NULL;
+err_bitmap_alloc:
  	kfree(info->name);
  	info->name = NULL;
@@ -1814,7 +1869,7 @@ static void tcmu_destroy_device(struct se_device *dev) enum {
  	Opt_dev_config, Opt_dev_size, Opt_hw_block_size, Opt_hw_max_sectors,
-	Opt_nl_reply_supported, Opt_err,
+	Opt_nl_reply_supported, Opt_max_data_area_mb, Opt_err,
  };
static match_table_t tokens = {
@@ -1823,6 +1878,7 @@ static match_table_t tokens = {
  	{Opt_hw_block_size, "hw_block_size=%u"},
  	{Opt_hw_max_sectors, "hw_max_sectors=%u"},
  	{Opt_nl_reply_supported, "nl_reply_supported=%d"},
+	{Opt_max_data_area_mb, "max_data_area_mb=%u"},
  	{Opt_err, NULL}
  };
@@ -1856,7 +1912,7 @@ static ssize_t tcmu_set_configfs_dev_params(struct se_device *dev,
  	struct tcmu_dev *udev = TCMU_DEV(dev);
  	char *orig, *ptr, *opts, *arg_p;
  	substring_t args[MAX_OPT_ARGS];
-	int ret = 0, token;
+	int ret = 0, token, tmpval;
opts = kstrdup(page, GFP_KERNEL);
  	if (!opts)
@@ -1908,6 +1964,39 @@ static ssize_t tcmu_set_configfs_dev_params(struct se_device *dev,
  			if (ret < 0)
  				pr_err("kstrtoul() failed for nl_reply_supported=\n");
  			break;
+		case Opt_max_data_area_mb:
+			if (dev->export_count) {
+				pr_err("Unable to set max_data_area_mb while exports exist\n");
+				ret = -EINVAL;
+				break;
+			}
+
+			arg_p = match_strdup(&args[0]);
+			if (!arg_p) {
+				ret = -ENOMEM;
+				break;
+			}
+			ret = kstrtoint(arg_p, 0, &tmpval);
+			kfree(arg_p);
+			if (ret < 0) {
+				pr_err("kstrtou32() failed for max_data_area_mb=\n");
+				break;
+			}
+
+			if (tmpval <= 0) {
+				pr_err("Invalid max_data_area %d\n", tmpval);
+				udev->max_blocks = DEF_DATA_BLOCK_BITS;
+				ret = -EINVAL;
+			}
+
+			udev->max_blocks = TCMU_MBS_TO_BLOCKS(tmpval);
+			if (udev->max_blocks > tcmu_global_max_blocks) {
+				pr_err("%d is too large. Adjusting max_data_area_mb to global limit of %u\n",
+				       tmpval,
+				       TCMU_BLOCKS_TO_MBS(tcmu_global_max_blocks));
+				udev->max_blocks = tcmu_global_max_blocks;
+			}
+			break;
  		default:
  			break;
  		}
@@ -1927,7 +2016,9 @@ static ssize_t tcmu_show_configfs_dev_params(struct se_device *dev, char *b)
bl = sprintf(b + bl, "Config: %s ",
  		     udev->dev_config[0] ? udev->dev_config : "NULL");
-	bl += sprintf(b + bl, "Size: %zu\n", udev->dev_size);
+	bl += sprintf(b + bl, "Size: %zu ", udev->dev_size);
+	bl += sprintf(b + bl, "MaxDataAreaMB: %u\n",
+		      TCMU_BLOCKS_TO_MBS(udev->max_blocks));
return bl;
  }
@@ -2011,6 +2102,17 @@ static ssize_t tcmu_qfull_time_out_store(struct config_item *item,
  }
  CONFIGFS_ATTR(tcmu_, qfull_time_out);
+static ssize_t tcmu_max_data_area_mb_show(struct config_item *item, char *page)
+{
+	struct se_dev_attrib *da = container_of(to_config_group(item),
+						struct se_dev_attrib, da_group);
+	struct tcmu_dev *udev = TCMU_DEV(da->da_dev);
+
+	return snprintf(page, PAGE_SIZE, "%u\n",
+			TCMU_BLOCKS_TO_MBS(udev->max_blocks));
+}
+CONFIGFS_ATTR_RO(tcmu_, max_data_area_mb);
+
  static ssize_t tcmu_dev_config_show(struct config_item *item, char *page)
  {
  	struct se_dev_attrib *da = container_of(to_config_group(item),
@@ -2157,6 +2259,7 @@ CONFIGFS_ATTR(tcmu_, emulate_write_cache);
  static struct configfs_attribute *tcmu_attrib_attrs[] = {
  	&tcmu_attr_cmd_time_out,
  	&tcmu_attr_qfull_time_out,
+	&tcmu_attr_max_data_area_mb,
  	&tcmu_attr_dev_config,
  	&tcmu_attr_dev_size,
  	&tcmu_attr_emulate_write_cache,
@@ -2240,7 +2343,7 @@ static uint32_t find_free_blocks(void)
  		if (tcmu_waiting_on_dev_blocks(udev)) {
  			/*
  			 * if we had to take pages from a dev that hit its
-			 * DATA_BLOCK_BITS limit put it on the waiter
+			 * max_blocks limit put it on the waiter
  			 * list so it gets rescheduled when pages are free.
  			 */
  			spin_lock(&root_udev_waiter_lock);
@@ -2281,7 +2384,7 @@ static bool run_cmdr_queues(void)
  	list_for_each_entry_safe(udev, tmp_udev, &devs, waiter) {
  		list_del_init(&udev->waiter);
- free_blocks = TCMU_GLOBAL_MAX_BLOCKS -
+		free_blocks = tcmu_global_max_blocks -
  						atomic_read(&global_db_count);
  		pr_debug("checking cmdr queue for %s: blocks waiting %u free db count %u\n",
  			 udev->name, udev->waiting_blocks, free_blocks);



--
To unsubscribe from this list: send the line "unsubscribe target-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux SCSI]     [Kernel Newbies]     [Linux SCSI Target Infrastructure]     [Share Photos]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Device Mapper]

  Powered by Linux