[PATCH-v2 0/3] target/vhost-scsi: Add per-cpu ida tag pre-allocation for v3.12

2 views
Skip to first unread message

Nicholas A. Bellinger

unread,
Jul 6, 2013, 6:03:20 PM7/6/13
to target-devel, lf-virt, lkml, Michael S. Tsirkin, Asias He, Paolo Bonzini, Kent Overstreet, Andrew Morton, Jens Axboe, Tejun Heo, Nicholas Bellinger
From: Nicholas Bellinger <n...@linux-iscsi.org>

Hi folks,

This is an updated series for adding tag pre-allocation support of
target fabric descriptor memory, utilizing Kent's latest in-flight
per-cpu ida bits here:

[PATCH v3] lib/idr.c rewrite, percpu ida/tag allocator
http://marc.info/?l=linux-kernel&m=137308733526328&w=2

Patch #1 includes target-core setup of se_sess->sess_cmd_map +
se_sess->sess_tag_pool resources at session creation time, using
fabric independent code in transport_init_session_tags().

Patch #2 is the initial conversion of vhost-scsi fabric code to use
per-cpu ida logic for obtaining a new tcm_vhost_cmd descriptor via
vhost_scsi_get_tag() during vhost_work_fn_t->handle_kick() ->
vhost_scsi_handle_vq() callback execution.

And patch #3 is a vhost-scsi change that adds pre-allocation of per
tcm_vhost_cmd descriptor scatterlist + user-space page pointer memory,
that allows the last two fast-path allocations to be dropped from
tcm_vhost_submission_work() -> vhost_scsi_map_to_sgl() fast-path
execution.

Please review as v3.12 material.

Thanks!

--nab

Nicholas Bellinger (3):
target: Add transport_init_session_tags using per-cpu ida
vhost/scsi: Convert to per-cpu ida_alloc + ida_free command map
vhost/scsi: Add pre-allocation for tv_cmd SGL + upages memory

drivers/target/target_core_transport.c | 33 ++++++++
drivers/vhost/scsi.c | 130 ++++++++++++++++++++++++--------
include/target/target_core_base.h | 5 +
include/target/target_core_fabric.h | 1 +
4 files changed, 137 insertions(+), 32 deletions(-)

--
1.7.2.5

Nicholas A. Bellinger

unread,
Jul 6, 2013, 6:03:21 PM7/6/13
to target-devel, lf-virt, lkml, Michael S. Tsirkin, Asias He, Paolo Bonzini, Kent Overstreet, Andrew Morton, Jens Axboe, Tejun Heo, Nicholas Bellinger
From: Nicholas Bellinger <n...@linux-iscsi.org>

This patch adds lib/idr.c based transport_init_session_tags() logic
that allows fabric drivers to setup a per-cpu se_sess->sess_tag_pool
and associated se_sess->sess_cmd_map for basic tagged pre-allocation
of fabric descriptor sized memory.

v3: Update to percpu-ida usage

Cc: Kent Overstreet <k...@daterainc.com>
Cc: Asias He <as...@redhat.com>
Cc: Michael S. Tsirkin <m...@redhat.com>
Signed-off-by: Nicholas Bellinger <n...@linux-iscsi.org>
---
drivers/target/target_core_transport.c | 33 ++++++++++++++++++++++++++++++++
include/target/target_core_base.h | 5 ++++
include/target/target_core_fabric.h | 1 +
3 files changed, 39 insertions(+), 0 deletions(-)

diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c
index 7172d00..64ceb51 100644
--- a/drivers/target/target_core_transport.c
+++ b/drivers/target/target_core_transport.c
@@ -232,6 +232,35 @@ struct se_session *transport_init_session(void)
}
EXPORT_SYMBOL(transport_init_session);

+struct se_session *transport_init_session_tags(unsigned int tag_num,
+ unsigned int tag_size)
+{
+ struct se_session *se_sess;
+ int rc;
+
+ se_sess = transport_init_session();
+ if (IS_ERR(se_sess))
+ return se_sess;
+
+ se_sess->sess_cmd_map = kzalloc(tag_num * tag_size, GFP_KERNEL);
+ if (!se_sess->sess_cmd_map) {
+ pr_err("Unable to allocate se_sess->sess_cmd_map\n");
+ transport_free_session(se_sess);
+ return ERR_PTR(-ENOMEM);
+ }
+
+ rc = percpu_ida_init(&se_sess->sess_tag_pool, tag_num);
+ if (rc < 0) {
+ pr_err("Unable to init se_sess->sess_tag_pool,"
+ " tag_num: %u\n", tag_num);
+ transport_free_session(se_sess);
+ return ERR_PTR(-ENOMEM);
+ }
+
+ return se_sess;
+}
+EXPORT_SYMBOL(transport_init_session_tags);
+
/*
* Called with spin_lock_irqsave(&struct se_portal_group->session_lock called.
*/
@@ -367,6 +396,10 @@ EXPORT_SYMBOL(transport_deregister_session_configfs);

void transport_free_session(struct se_session *se_sess)
{
+ if (se_sess->sess_cmd_map) {
+ percpu_ida_destroy(&se_sess->sess_tag_pool);
+ kfree(se_sess->sess_cmd_map);
+ }
kmem_cache_free(se_sess_cache, se_sess);
}
EXPORT_SYMBOL(transport_free_session);
diff --git a/include/target/target_core_base.h b/include/target/target_core_base.h
index d92ec67..a842884 100644
--- a/include/target/target_core_base.h
+++ b/include/target/target_core_base.h
@@ -5,6 +5,7 @@
#include <linux/configfs.h>
#include <linux/dma-mapping.h>
#include <linux/blkdev.h>
+#include <linux/idr.h>
#include <scsi/scsi_cmnd.h>
#include <net/sock.h>
#include <net/tcp.h>
@@ -418,6 +419,8 @@ struct se_cmd {
enum dma_data_direction data_direction;
/* For SAM Task Attribute */
int sam_task_attr;
+ /* Used for se_sess->sess_tag_pool */
+ unsigned int map_tag;
/* Transport protocol dependent state, see transport_state_table */
enum transport_state_table t_state;
unsigned cmd_wait_set:1;
@@ -539,6 +542,8 @@ struct se_session {
struct list_head sess_wait_list;
spinlock_t sess_cmd_lock;
struct kref sess_kref;
+ void *sess_cmd_map;
+ struct percpu_ida sess_tag_pool;
};

struct se_device;
diff --git a/include/target/target_core_fabric.h b/include/target/target_core_fabric.h
index 1dcce9c..d34f2ce 100644
--- a/include/target/target_core_fabric.h
+++ b/include/target/target_core_fabric.h
@@ -84,6 +84,7 @@ struct target_core_fabric_ops {
};

struct se_session *transport_init_session(void);
+struct se_session *transport_init_session_tags(unsigned int, unsigned int);
void __transport_register_session(struct se_portal_group *,
struct se_node_acl *, struct se_session *, void *);
void transport_register_session(struct se_portal_group *,
--
1.7.2.5

Nicholas A. Bellinger

unread,
Jul 6, 2013, 6:03:22 PM7/6/13
to target-devel, lf-virt, lkml, Michael S. Tsirkin, Asias He, Paolo Bonzini, Kent Overstreet, Andrew Morton, Jens Axboe, Tejun Heo, Nicholas Bellinger
From: Nicholas Bellinger <n...@linux-iscsi.org>

This patch changes vhost/scsi to use transport_init_session_tags()
pre-allocation logic for per-cpu session tag pooling with internal
ida_alloc() + ida_free() calls based upon the saved se_cmd->map_tag id.

FIXME: Make transport_init_session_tags() number of tags setup
configurable per vring client setting via configfs

v3: Update to percpu-ida usage

Cc: Michael S. Tsirkin <m...@redhat.com>
Cc: Asias He <as...@redhat.com>
Cc: Kent Overstreet <k...@daterainc.com>
Signed-off-by: Nicholas Bellinger <n...@linux-iscsi.org>
---
drivers/vhost/scsi.c | 33 +++++++++++++++++++++------------
1 files changed, 21 insertions(+), 12 deletions(-)

diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
index 1e5e820..6224434 100644
--- a/drivers/vhost/scsi.c
+++ b/drivers/vhost/scsi.c
@@ -48,6 +48,7 @@
#include <linux/virtio_scsi.h>
#include <linux/llist.h>
#include <linux/bitmap.h>
+#include <linux/idr.h>

#include "vhost.c"
#include "vhost.h"
@@ -55,6 +56,7 @@
#define TCM_VHOST_VERSION "v0.1"
#define TCM_VHOST_NAMELEN 256
#define TCM_VHOST_MAX_CDB_SIZE 32
+#define TCM_VHOST_DEFAULT_TAGS 256

struct vhost_scsi_inflight {
/* Wait for the flush operation to finish */
@@ -448,6 +450,7 @@ static void tcm_vhost_release_cmd(struct se_cmd *se_cmd)
{
struct tcm_vhost_cmd *tv_cmd = container_of(se_cmd,
struct tcm_vhost_cmd, tvc_se_cmd);
+ struct se_session *se_sess = se_cmd->se_sess;

if (tv_cmd->tvc_sgl_count) {
u32 i;
@@ -458,7 +461,7 @@ static void tcm_vhost_release_cmd(struct se_cmd *se_cmd)
}

tcm_vhost_put_inflight(tv_cmd->inflight);
- kfree(tv_cmd);
+ percpu_ida_free(&se_sess->sess_tag_pool, se_cmd->map_tag);
}

static int tcm_vhost_shutdown_session(struct se_session *se_sess)
@@ -700,7 +703,7 @@ static void vhost_scsi_complete_cmd_work(struct vhost_work *work)
vhost_signal(&vs->dev, &vs->vqs[vq].vq);
}

-static struct tcm_vhost_cmd *vhost_scsi_allocate_cmd(
+static struct tcm_vhost_cmd *vhost_scsi_get_tag(
struct vhost_virtqueue *vq,
struct tcm_vhost_tpg *tv_tpg,
struct virtio_scsi_cmd_req *v_req,
@@ -709,18 +712,21 @@ static struct tcm_vhost_cmd *vhost_scsi_allocate_cmd(
{
struct tcm_vhost_cmd *tv_cmd;
struct tcm_vhost_nexus *tv_nexus;
+ struct se_session *se_sess;
+ int tag;

tv_nexus = tv_tpg->tpg_nexus;
if (!tv_nexus) {
pr_err("Unable to locate active struct tcm_vhost_nexus\n");
return ERR_PTR(-EIO);
}
+ se_sess = tv_nexus->tvn_se_sess;

- tv_cmd = kzalloc(sizeof(struct tcm_vhost_cmd), GFP_ATOMIC);
- if (!tv_cmd) {
- pr_err("Unable to allocate struct tcm_vhost_cmd\n");
- return ERR_PTR(-ENOMEM);
- }
+ tag = percpu_ida_alloc(&se_sess->sess_tag_pool, GFP_KERNEL);
+ tv_cmd = &((struct tcm_vhost_cmd *)se_sess->sess_cmd_map)[tag];
+ memset(tv_cmd, 0, sizeof(struct tcm_vhost_cmd));
+
+ tv_cmd->tvc_se_cmd.map_tag = tag;
tv_cmd->tvc_tag = v_req->tag;
tv_cmd->tvc_task_attr = v_req->task_attr;
tv_cmd->tvc_exp_data_len = exp_data_len;
@@ -982,10 +988,10 @@ static void vhost_scsi_handle_vq(struct vhost_scsi *vs,
for (i = 0; i < data_num; i++)
exp_data_len += vq->iov[data_first + i].iov_len;

- tv_cmd = vhost_scsi_allocate_cmd(vq, tv_tpg, &v_req,
- exp_data_len, data_direction);
+ tv_cmd = vhost_scsi_get_tag(vq, tv_tpg, &v_req,
+ exp_data_len, data_direction);
if (IS_ERR(tv_cmd)) {
- vq_err(vq, "vhost_scsi_allocate_cmd failed %ld\n",
+ vq_err(vq, "vhost_scsi_get_tag failed %ld\n",
PTR_ERR(tv_cmd));
goto err_cmd;
}
@@ -1662,9 +1668,12 @@ static int tcm_vhost_make_nexus(struct tcm_vhost_tpg *tv_tpg,
return -ENOMEM;
}
/*
- * Initialize the struct se_session pointer
+ * Initialize the struct se_session pointer and setup tagpool
+ * for struct tcm_vhost_cmd descriptors
*/
- tv_nexus->tvn_se_sess = transport_init_session();
+ tv_nexus->tvn_se_sess = transport_init_session_tags(
+ TCM_VHOST_DEFAULT_TAGS,
+ sizeof(struct tcm_vhost_cmd));
if (IS_ERR(tv_nexus->tvn_se_sess)) {
mutex_unlock(&tv_tpg->tv_tpg_mutex);
kfree(tv_nexus);
--
1.7.2.5

Nicholas A. Bellinger

unread,
Jul 6, 2013, 6:03:23 PM7/6/13
to target-devel, lf-virt, lkml, Michael S. Tsirkin, Asias He, Paolo Bonzini, Kent Overstreet, Andrew Morton, Jens Axboe, Tejun Heo, Nicholas Bellinger
From: Nicholas Bellinger <n...@linux-iscsi.org>

This patch adds support for pre-allocation of per tv_cmd descriptor
scatterlist + user-space page pointer memory using se_sess->sess_cmd_map
within tcm_vhost_make_nexus() code.

This includes sanity checks within vhost_scsi_map_to_sgl()
to reject I/O that exceeds these initial hardcoded values, and
the necessary cleanup in tcm_vhost_make_nexus() failure path +
tcm_vhost_drop_nexus().

Cc: Michael S. Tsirkin <m...@redhat.com>
Cc: Asias He <as...@redhat.com>
Cc: Kent Overstreet <k...@daterainc.com>
Signed-off-by: Nicholas Bellinger <n...@linux-iscsi.org>
---
drivers/vhost/scsi.c | 97 +++++++++++++++++++++++++++++++++++++++----------
1 files changed, 77 insertions(+), 20 deletions(-)

diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
index 6224434..f200b17 100644
--- a/drivers/vhost/scsi.c
+++ b/drivers/vhost/scsi.c
@@ -57,6 +57,8 @@
#define TCM_VHOST_NAMELEN 256
#define TCM_VHOST_MAX_CDB_SIZE 32
#define TCM_VHOST_DEFAULT_TAGS 256
+#define TCM_VHOST_PREALLOC_SGLS 2048
+#define TCM_VHOST_PREALLOC_PAGES 2048

struct vhost_scsi_inflight {
/* Wait for the flush operation to finish */
@@ -82,6 +84,7 @@ struct tcm_vhost_cmd {
u32 tvc_lun;
/* Pointer to the SGL formatted memory from virtio-scsi */
struct scatterlist *tvc_sgl;
+ struct page **tvc_upages;
/* Pointer to response */
struct virtio_scsi_cmd_resp __user *tvc_resp;
/* Pointer to vhost_scsi for our device */
@@ -456,8 +459,6 @@ static void tcm_vhost_release_cmd(struct se_cmd *se_cmd)
u32 i;
for (i = 0; i < tv_cmd->tvc_sgl_count; i++)
put_page(sg_page(&tv_cmd->tvc_sgl[i]));
-
- kfree(tv_cmd->tvc_sgl);
}

tcm_vhost_put_inflight(tv_cmd->inflight);
@@ -713,6 +714,8 @@ static struct tcm_vhost_cmd *vhost_scsi_get_tag(
struct tcm_vhost_cmd *tv_cmd;
struct tcm_vhost_nexus *tv_nexus;
struct se_session *se_sess;
+ struct scatterlist *sg;
+ struct page **pages;
int tag;

tv_nexus = tv_tpg->tpg_nexus;
@@ -724,8 +727,12 @@ static struct tcm_vhost_cmd *vhost_scsi_get_tag(

tag = percpu_ida_alloc(&se_sess->sess_tag_pool, GFP_KERNEL);
tv_cmd = &((struct tcm_vhost_cmd *)se_sess->sess_cmd_map)[tag];
+ sg = tv_cmd->tvc_sgl;
+ pages = tv_cmd->tvc_upages;
memset(tv_cmd, 0, sizeof(struct tcm_vhost_cmd));

+ tv_cmd->tvc_sgl = sg;
+ tv_cmd->tvc_upages = pages;
tv_cmd->tvc_se_cmd.map_tag = tag;
tv_cmd->tvc_tag = v_req->tag;
tv_cmd->tvc_task_attr = v_req->task_attr;
@@ -742,7 +749,7 @@ static struct tcm_vhost_cmd *vhost_scsi_get_tag(
*
* Returns the number of scatterlist entries used or -errno on error.
*/
-static int vhost_scsi_map_to_sgl(struct scatterlist *sgl,
+static int vhost_scsi_map_to_sgl(struct tcm_vhost_cmd *tv_cmd, struct scatterlist *sgl,
unsigned int sgl_count, struct iovec *iov, int write)
{
unsigned int npages = 0, pages_nr, offset, nbytes;
@@ -752,13 +759,25 @@ static int vhost_scsi_map_to_sgl(struct scatterlist *sgl,
struct page **pages;
int ret, i;

+ if (sgl_count > TCM_VHOST_PREALLOC_SGLS) {
+ pr_err("vhost_scsi_map_to_sgl() psgl_count: %u greater than"
+ " preallocated TCM_VHOST_PREALLOC_SGLS: %u\n",
+ sgl_count, TCM_VHOST_PREALLOC_SGLS);
+ return -ENOBUFS;
+ }
+
pages_nr = iov_num_pages(iov);
if (pages_nr > sgl_count)
return -ENOBUFS;

- pages = kmalloc(pages_nr * sizeof(struct page *), GFP_KERNEL);
- if (!pages)
- return -ENOMEM;
+ if (pages_nr > TCM_VHOST_PREALLOC_PAGES) {
+ pr_err("vhost_scsi_map_to_sgl() pages_nr: %u greater than"
+ " preallocated TCM_VHOST_PREALLOC_PAGES: %u\n",
+ pages_nr, TCM_VHOST_PREALLOC_PAGES);
+ return -ENOBUFS;
+ }
+
+ pages = tv_cmd->tvc_upages;

ret = get_user_pages_fast((unsigned long)ptr, pages_nr, write, pages);
/* No pages were pinned */
@@ -783,7 +802,6 @@ static int vhost_scsi_map_to_sgl(struct scatterlist *sgl,
}

out:
- kfree(pages);
return ret;
}

@@ -803,25 +821,18 @@ static int vhost_scsi_map_iov_to_sgl(struct tcm_vhost_cmd *tv_cmd,
sgl_count += iov_num_pages(&iov[i]);

/* TODO overflow checking */
-
- sg = kmalloc(sizeof(tv_cmd->tvc_sgl[0]) * sgl_count, GFP_ATOMIC);
- if (!sg)
- return -ENOMEM;
- pr_debug("%s sg %p sgl_count %u is_err %d\n", __func__,
- sg, sgl_count, !sg);
+ sg = tv_cmd->tvc_sgl;
sg_init_table(sg, sgl_count);

- tv_cmd->tvc_sgl = sg;
tv_cmd->tvc_sgl_count = sgl_count;

pr_debug("Mapping %u iovecs for %u pages\n", niov, sgl_count);
for (i = 0; i < niov; i++) {
- ret = vhost_scsi_map_to_sgl(sg, sgl_count, &iov[i], write);
+ ret = vhost_scsi_map_to_sgl(tv_cmd, sg, sgl_count, &iov[i], write);
if (ret < 0) {
for (i = 0; i < tv_cmd->tvc_sgl_count; i++)
put_page(sg_page(&tv_cmd->tvc_sgl[i]));
- kfree(tv_cmd->tvc_sgl);
- tv_cmd->tvc_sgl = NULL;
+
tv_cmd->tvc_sgl_count = 0;
return ret;
}
@@ -1647,11 +1658,31 @@ static void tcm_vhost_drop_nodeacl(struct se_node_acl *se_acl)
kfree(nacl);
}

+static void tcm_vhost_free_cmd_map_res(struct tcm_vhost_nexus *nexus,
+ struct se_session *se_sess)
+{
+ struct tcm_vhost_cmd *tv_cmd;
+ int i;
+
+ if (!se_sess->sess_cmd_map)
+ return;
+
+ for (i = 0; i < TCM_VHOST_DEFAULT_TAGS; i++) {
+ tv_cmd = &((struct tcm_vhost_cmd *)se_sess->sess_cmd_map)[i];
+
+ kfree(tv_cmd->tvc_sgl);
+ kfree(tv_cmd->tvc_upages);
+ }
+}
+
static int tcm_vhost_make_nexus(struct tcm_vhost_tpg *tv_tpg,
const char *name)
{
struct se_portal_group *se_tpg;
+ struct se_session *se_sess;
struct tcm_vhost_nexus *tv_nexus;
+ struct tcm_vhost_cmd *tv_cmd;
+ unsigned int i;

mutex_lock(&tv_tpg->tv_tpg_mutex);
if (tv_tpg->tpg_nexus) {
@@ -1679,6 +1710,26 @@ static int tcm_vhost_make_nexus(struct tcm_vhost_tpg *tv_tpg,
kfree(tv_nexus);
return -ENOMEM;
}
+ se_sess = tv_nexus->tvn_se_sess;
+ for (i = 0; i < TCM_VHOST_DEFAULT_TAGS; i++) {
+ tv_cmd = &((struct tcm_vhost_cmd *)se_sess->sess_cmd_map)[i];
+
+ tv_cmd->tvc_sgl = kzalloc(sizeof(struct scatterlist) *
+ TCM_VHOST_PREALLOC_SGLS, GFP_KERNEL);
+ if (!tv_cmd->tvc_sgl) {
+ mutex_unlock(&tv_tpg->tv_tpg_mutex);
+ pr_err("Unable to allocate tv_cmd->tvc_sgl\n");
+ goto out;
+ }
+
+ tv_cmd->tvc_upages = kzalloc(sizeof(struct page *) *
+ TCM_VHOST_PREALLOC_PAGES, GFP_KERNEL);
+ if (!tv_cmd->tvc_upages) {
+ mutex_unlock(&tv_tpg->tv_tpg_mutex);
+ pr_err("Unable to allocate tv_cmd->tvc_upages\n");
+ goto out;
+ }
+ }
/*
* Since we are running in 'demo mode' this call with generate a
* struct se_node_acl for the tcm_vhost struct se_portal_group with
@@ -1690,9 +1741,7 @@ static int tcm_vhost_make_nexus(struct tcm_vhost_tpg *tv_tpg,
mutex_unlock(&tv_tpg->tv_tpg_mutex);
pr_debug("core_tpg_check_initiator_node_acl() failed"
" for %s\n", name);
- transport_free_session(tv_nexus->tvn_se_sess);
- kfree(tv_nexus);
- return -ENOMEM;
+ goto out;
}
/*
* Now register the TCM vhost virtual I_T Nexus as active with the
@@ -1704,6 +1753,12 @@ static int tcm_vhost_make_nexus(struct tcm_vhost_tpg *tv_tpg,

mutex_unlock(&tv_tpg->tv_tpg_mutex);
return 0;
+
+out:
+ tcm_vhost_free_cmd_map_res(tv_nexus, se_sess);
+ transport_free_session(se_sess);
+ kfree(tv_nexus);
+ return -ENOMEM;
}

static int tcm_vhost_drop_nexus(struct tcm_vhost_tpg *tpg)
@@ -1743,6 +1798,8 @@ static int tcm_vhost_drop_nexus(struct tcm_vhost_tpg *tpg)
pr_debug("TCM_vhost_ConfigFS: Removing I_T Nexus to emulated"
" %s Initiator Port: %s\n", tcm_vhost_dump_proto_id(tpg->tport),
tv_nexus->tvn_se_sess->se_node_acl->initiatorname);
+
+ tcm_vhost_free_cmd_map_res(tv_nexus, se_sess);
/*
* Release the SCSI I_T Nexus to the emulated vhost Target Port
*/
--
1.7.2.5

Reply all
Reply to author
Forward
0 new messages