On Sat, Aug 31, 2013 at 02:52:34AM +0000, Nicholas A. Bellinger wrote:
> 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().
> 
> v3 changes:
>   - Rebase to v3.11-rc5 code
> 
> 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>

Reviewed-by: Asias He <as...@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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to