| Message ID | 1508310852-15366-18-git-send-email-mchristi@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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@redhat.com> > --- > 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 10/25/2017 04:10 AM, Xiubo Li wrote: > 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? > Hey, Sorry about that. I forgot configshell has those restrictions. I use ceph-iscsi-cli with these patches: https://github.com/open-iscsi/rtslib-fb/pull/112 https://github.com/ceph/ceph-iscsi-config/pull/31 > For now I'm using the targetcli tool by just adding ',' support in > configshell, and test this patch works well. > Thanks. I will do a PR for configshell if you do not beat me to it. > 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@redhat.com> >> --- >> 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 2017年10月25日 23:25, Mike Christie wrote: > On 10/25/2017 04:10 AM, Xiubo Li wrote: >> 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? >> > Hey, > > Sorry about that. I forgot configshell has those restrictions. I use > ceph-iscsi-cli with these patches: > > https://github.com/open-iscsi/rtslib-fb/pull/112 > https://github.com/ceph/ceph-iscsi-config/pull/31 Yes, I have two test environments, one is based ceph-iscsi-gateway and still doing other test cases. The other one is the targetcli. > >> For now I'm using the targetcli tool by just adding ',' support in >> configshell, and test this patch works well. >> > Thanks. I will do a PR for configshell if you do not beat me to it. > Yeah go ahead and i will test it for you. Thanks, BRs Xiubo >> 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@redhat.com> >>> --- >>> 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 10/25/2017 08:03 PM, Xiubo Li wrote: >> >>> For now I'm using the targetcli tool by just adding ',' support in >>> configshell, and test this patch works well. >>> >> Thanks. I will do a PR for configshell if you do not beat me to it. >> > Yeah go ahead and i will test it for you. Here is a patch for targetcli: https://github.com/open-iscsi/targetcli-fb/pull/95 There is also a updated rtslib patch: https://github.com/open-iscsi/rtslib-fb/pull/112 -- To unsubscribe from this list: send the line "unsubscribe target-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 2017年10月26日 13:30, Mike Christie wrote: > On 10/25/2017 08:03 PM, Xiubo Li wrote: >>>> For now I'm using the targetcli tool by just adding ',' support in >>>> configshell, and test this patch works well. >>>> >>> Thanks. I will do a PR for configshell if you do not beat me to it. >>> >> Yeah go ahead and i will test it for you. > Here is a patch for targetcli: > > https://github.com/open-iscsi/targetcli-fb/pull/95 > > There is also a updated rtslib patch: > > https://github.com/open-iscsi/rtslib-fb/pull/112 Yes, test this and works for me. /> /backstores/user:file create name=file1 size=1G cfgstring=file1 max_data_area_mb=2048 /> /backstores/user:file/file1/ get attribute max_data_area_mb max_data_area_mb=2048 [ro] Thanks, BRs -- To unsubscribe from this list: send the line "unsubscribe target-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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);
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@redhat.com> --- drivers/target/target_core_user.c | 153 +++++++++++++++++++++++++++++++------- 1 file changed, 128 insertions(+), 25 deletions(-)