| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <939e59f2-d43b-7972-b554-2be2b734e6de@gmail.com>
Date: Wed, 16 Feb 2022 20:20:31 +0100
From: Bodo Stroesser <bostroesser@...il.com>
To: Guixin Liu <kanie@...ux.alibaba.com>, martin.petersen@...cle.com
Cc: linux-scsi@...r.kernel.org, target-devel@...r.kernel.org,
linux-kernel@...r.kernel.org, xiaoguang.wang@...ux.alibaba.com,
xlpang@...ux.alibaba.com
Subject: Re: [PATCH V4] scsi: target: tcmu: Make cmd_ring_size changeable via
configfs
On 16.02.22 03:21, Guixin Liu wrote:
> Make cmd_ring_size changeable similar to the way it is done for
> max_data_area_mb, the reason is that our tcmu client will create
> thousands of tcmu instances, and this will consume lots of mem with
> default 8Mb cmd ring size for every backstore.
>
> One can change the value by typing:
> echo "cmd_ring_size_mb=N" > control
> The "N" is a integer between 1 to 8, if set 1, the cmd ring can hold
> about 6k cmds(tcmu_cmd_entry about 176 byte) at least.
>
> The value is printed when doing:
> cat info
> In addition, a new readonly attribute 'cmd_ring_size_mb' returns the
> value in read.
>
> Reviewed-by: Xiaoguang Wang <xiaoguang.wang@...ux.alibaba.com>
> Signed-off-by: Guixin Liu <kanie@...ux.alibaba.com>
Thanks!
Reviewed-by: Bodo Stroesser <bostroesser@...il.com>
> ---
> drivers/target/target_core_user.c | 73 +++++++++++++++++++++++++++++++++------
> 1 file changed, 63 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/target/target_core_user.c b/drivers/target/target_core_user.c
> index 7b2a89a..95d4ca5 100644
> --- a/drivers/target/target_core_user.c
> +++ b/drivers/target/target_core_user.c
> @@ -61,10 +61,10 @@
> #define TCMU_TIME_OUT (30 * MSEC_PER_SEC)
>
> /* For mailbox plus cmd ring, the size is fixed 8MB */
> -#define MB_CMDR_SIZE (8 * 1024 * 1024)
> +#define MB_CMDR_SIZE_DEF (8 * 1024 * 1024)
> /* Offset of cmd ring is size of mailbox */
> -#define CMDR_OFF sizeof(struct tcmu_mailbox)
> -#define CMDR_SIZE (MB_CMDR_SIZE - CMDR_OFF)
> +#define CMDR_OFF ((__u32)sizeof(struct tcmu_mailbox))
> +#define CMDR_SIZE_DEF (MB_CMDR_SIZE_DEF - CMDR_OFF)
>
> /*
> * For data area, the default block size is PAGE_SIZE and
> @@ -1617,6 +1617,7 @@ static struct se_device *tcmu_alloc_device(struct se_hba *hba, const char *name)
>
> udev->data_pages_per_blk = DATA_PAGES_PER_BLK_DEF;
> udev->max_blocks = DATA_AREA_PAGES_DEF / udev->data_pages_per_blk;
> + udev->cmdr_size = CMDR_SIZE_DEF;
> udev->data_area_mb = TCMU_PAGES_TO_MBS(DATA_AREA_PAGES_DEF);
>
> mutex_init(&udev->cmdr_lock);
> @@ -2189,7 +2190,7 @@ static int tcmu_configure_device(struct se_device *dev)
> goto err_bitmap_alloc;
> }
>
> - mb = vzalloc(MB_CMDR_SIZE);
> + mb = vzalloc(udev->cmdr_size + CMDR_OFF);
> if (!mb) {
> ret = -ENOMEM;
> goto err_vzalloc;
> @@ -2198,10 +2199,9 @@ static int tcmu_configure_device(struct se_device *dev)
> /* mailbox fits in first part of CMDR space */
> udev->mb_addr = mb;
> udev->cmdr = (void *)mb + CMDR_OFF;
> - udev->cmdr_size = CMDR_SIZE;
> - udev->data_off = MB_CMDR_SIZE;
> + udev->data_off = udev->cmdr_size + CMDR_OFF;
> data_size = TCMU_MBS_TO_PAGES(udev->data_area_mb) << PAGE_SHIFT;
> - udev->mmap_pages = (data_size + MB_CMDR_SIZE) >> PAGE_SHIFT;
> + udev->mmap_pages = (data_size + udev->cmdr_size + CMDR_OFF) >> PAGE_SHIFT;
> udev->data_blk_size = udev->data_pages_per_blk * PAGE_SIZE;
> udev->dbi_thresh = 0; /* Default in Idle state */
>
> @@ -2221,7 +2221,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 = data_size + MB_CMDR_SIZE;
> + info->mem[0].size = data_size + udev->cmdr_size + CMDR_OFF;
> info->mem[0].memtype = UIO_MEM_NONE;
>
> info->irqcontrol = tcmu_irqcontrol;
> @@ -2401,7 +2401,7 @@ static void tcmu_reset_ring(struct tcmu_dev *udev, u8 err_level)
> enum {
> Opt_dev_config, Opt_dev_size, Opt_hw_block_size, Opt_hw_max_sectors,
> Opt_nl_reply_supported, Opt_max_data_area_mb, Opt_data_pages_per_blk,
> - Opt_err,
> + Opt_cmd_ring_size_mb, Opt_err,
> };
>
> static match_table_t tokens = {
> @@ -2412,6 +2412,7 @@ enum {
> {Opt_nl_reply_supported, "nl_reply_supported=%d"},
> {Opt_max_data_area_mb, "max_data_area_mb=%d"},
> {Opt_data_pages_per_blk, "data_pages_per_blk=%d"},
> + {Opt_cmd_ring_size_mb, "cmd_ring_size_mb=%d"},
> {Opt_err, NULL}
> };
>
> @@ -2509,6 +2510,41 @@ static int tcmu_set_data_pages_per_blk(struct tcmu_dev *udev, substring_t *arg)
> return ret;
> }
>
> +static int tcmu_set_cmd_ring_size(struct tcmu_dev *udev, substring_t *arg)
> +{
> + int val, ret;
> +
> + ret = match_int(arg, &val);
> + if (ret < 0) {
> + pr_err("match_int() failed for cmd_ring_size_mb=. Error %d.\n",
> + ret);
> + return ret;
> + }
> +
> + if (val <= 0) {
> + pr_err("Invalid cmd_ring_size_mb %d.\n", val);
> + return -EINVAL;
> + }
> +
> + mutex_lock(&udev->cmdr_lock);
> + if (udev->data_bitmap) {
> + pr_err("Cannot set cmd_ring_size_mb after it has been enabled.\n");
> + ret = -EINVAL;
> + goto unlock;
> + }
> +
> + udev->cmdr_size = (val << 20) - CMDR_OFF;
> + if (val > (MB_CMDR_SIZE_DEF >> 20)) {
> + pr_err("%d is too large. Adjusting cmd_ring_size_mb to global limit of %u\n",
> + val, (MB_CMDR_SIZE_DEF >> 20));
> + udev->cmdr_size = CMDR_SIZE_DEF;
> + }
> +
> +unlock:
> + mutex_unlock(&udev->cmdr_lock);
> + return ret;
> +}
> +
> static ssize_t tcmu_set_configfs_dev_params(struct se_device *dev,
> const char *page, ssize_t count)
> {
> @@ -2563,6 +2599,9 @@ static ssize_t tcmu_set_configfs_dev_params(struct se_device *dev,
> case Opt_data_pages_per_blk:
> ret = tcmu_set_data_pages_per_blk(udev, &args[0]);
> break;
> + case Opt_cmd_ring_size_mb:
> + ret = tcmu_set_cmd_ring_size(udev, &args[0]);
> + break;
> default:
> break;
> }
> @@ -2584,7 +2623,9 @@ static ssize_t tcmu_show_configfs_dev_params(struct se_device *dev, char *b)
> udev->dev_config[0] ? udev->dev_config : "NULL");
> bl += sprintf(b + bl, "Size: %llu ", udev->dev_size);
> bl += sprintf(b + bl, "MaxDataAreaMB: %u ", udev->data_area_mb);
> - bl += sprintf(b + bl, "DataPagesPerBlk: %u\n", udev->data_pages_per_blk);
> + bl += sprintf(b + bl, "DataPagesPerBlk: %u ", udev->data_pages_per_blk);
> + bl += sprintf(b + bl, "CmdRingSizeMB: %u\n",
> + (udev->cmdr_size + CMDR_OFF) >> 20);
>
> return bl;
> }
> @@ -2693,6 +2734,17 @@ static ssize_t tcmu_data_pages_per_blk_show(struct config_item *item,
> }
> CONFIGFS_ATTR_RO(tcmu_, data_pages_per_blk);
>
> +static ssize_t tcmu_cmd_ring_size_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",
> + (udev->cmdr_size + CMDR_OFF) >> 20);
> +}
> +CONFIGFS_ATTR_RO(tcmu_, cmd_ring_size_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),
> @@ -3064,6 +3116,7 @@ static ssize_t tcmu_free_kept_buf_store(struct config_item *item, const char *pa
> &tcmu_attr_qfull_time_out,
> &tcmu_attr_max_data_area_mb,
> &tcmu_attr_data_pages_per_blk,
> + &tcmu_attr_cmd_ring_size_mb,
> &tcmu_attr_dev_config,
> &tcmu_attr_dev_size,
> &tcmu_attr_emulate_write_cache,
Powered by blists - more mailing lists