Re: [PATCH-v5 4/6] vhost/scsi: Add pre-allocation for tv_cmd SGL + upages memory

Asias He
2013-09-03T08:14:28Z

On Sat, Aug 31, 2013 at 02:52:34AM +0000, Nicholas A. Bellinger wrote:
> From: Nicholas Bellinger <nab@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().
>
> v3 changes:
> - Rebase to v3.11-rc5 code
>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Cc: Asias He <asias@redhat.com>
> Cc: Kent Overstreet <kmo@daterainc.com>
> Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>

Reviewed-by: Asias He <asias@redhat.com>

> ---
> drivers/vhost/scsi.c | 99 ++++++++++++++++++++++++++++++++++++++++----------
> 1 files changed, 80 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
> index 8cd545a..d52a3a0 100644
> --- a/drivers/vhost/scsi.c
> +++ b/drivers/vhost/scsi.c
> @@ -56,6 +56,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 */
> @@ -81,6 +83,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 */
> @@ -458,8 +461,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);
> @@ -716,6 +717,8 @@ vhost_scsi_get_tag(struct vhost_virtqueue *vq,
> struct tcm_vhost_cmd *cmd;
> struct tcm_vhost_nexus *tv_nexus;
> struct se_session *se_sess;
> + struct scatterlist *sg;
> + struct page **pages;
> int tag;
>
> tv_nexus = tpg->tpg_nexus;
> @@ -727,8 +730,12 @@ vhost_scsi_get_tag(struct vhost_virtqueue *vq,
>
> tag = percpu_ida_alloc(&se_sess->sess_tag_pool, GFP_KERNEL);
> cmd = &((struct tcm_vhost_cmd *)se_sess->sess_cmd_map)[tag];
> + sg = cmd->tvc_sgl;
> + pages = cmd->tvc_upages;
> memset(cmd, 0, sizeof(struct tcm_vhost_cmd));
>
> + cmd->tvc_sgl = sg;
> + cmd->tvc_upages = pages;
> cmd->tvc_se_cmd.map_tag = tag;
> cmd->tvc_tag = v_req->tag;
> cmd->tvc_task_attr = v_req->task_attr;
> @@ -746,7 +753,8 @@ vhost_scsi_get_tag(struct vhost_virtqueue *vq,
> * Returns the number of scatterlist entries used or -errno on error.
> */
> static int
> -vhost_scsi_map_to_sgl(struct scatterlist *sgl,
> +vhost_scsi_map_to_sgl(struct tcm_vhost_cmd *tv_cmd,
> + struct scatterlist *sgl,
> unsigned int sgl_count,
> struct iovec *iov,
> int write)
> @@ -758,13 +766,25 @@ 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 */
> @@ -789,7 +809,6 @@ vhost_scsi_map_to_sgl(struct scatterlist *sgl,
> }
>
> out:
> - kfree(pages);
> return ret;
> }
>
> @@ -813,24 +832,20 @@ vhost_scsi_map_iov_to_sgl(struct tcm_vhost_cmd *cmd,
>
> /* TODO overflow checking */
>
> - sg = kmalloc(sizeof(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 = cmd->tvc_sgl;
> + pr_debug("%s sg %p sgl_count %u\n", __func__, sg, sgl_count);
> sg_init_table(sg, sgl_count);
>
> - cmd->tvc_sgl = sg;
> 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(cmd, sg, sgl_count, &iov[i],
> + write);
> if (ret < 0) {
> for (i = 0; i < cmd->tvc_sgl_count; i++)
> put_page(sg_page(&cmd->tvc_sgl[i]));
> - kfree(cmd->tvc_sgl);
> - cmd->tvc_sgl = NULL;
> +
> cmd->tvc_sgl_count = 0;
> return ret;
> }
> @@ -1660,11 +1675,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;
> + unsigned 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 *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(&tpg->tv_tpg_mutex);
> if (tpg->tpg_nexus) {
> @@ -1692,6 +1727,26 @@ static int tcm_vhost_make_nexus(struct tcm_vhost_tpg *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(&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(&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
> @@ -1703,9 +1758,7 @@ static int tcm_vhost_make_nexus(struct tcm_vhost_tpg *tpg,
> mutex_unlock(&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
> @@ -1717,6 +1770,12 @@ static int tcm_vhost_make_nexus(struct tcm_vhost_tpg *tpg,
>
> mutex_unlock(&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)
> @@ -1756,6 +1815,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
>

--
Asias