Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1759644Ab3ICIO2 (ORCPT ); Tue, 3 Sep 2013 04:14:28 -0400 Received: from mx1.redhat.com ([209.132.183.28]:20506 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752633Ab3ICIOY (ORCPT ); Tue, 3 Sep 2013 04:14:24 -0400 Date: Tue, 3 Sep 2013 16:13:54 +0800 From: Asias He To: "Nicholas A. Bellinger" Cc: target-devel , lkml , "Michael S. Tsirkin" , Kent Overstreet , Andrew Morton , Jens Axboe , Tejun Heo , Ingo Molnar , Andi Kleen , Christoph Lameter Subject: Re: [PATCH-v5 4/6] vhost/scsi: Add pre-allocation for tv_cmd SGL + upages memory Message-ID: <20130903081354.GC26859@hj.localdomain> References: <1377917556-11955-1-git-send-email-nab@linux-iscsi.org> <1377917556-11955-5-git-send-email-nab@linux-iscsi.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1377917556-11955-5-git-send-email-nab@linux-iscsi.org> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 8420 Lines: 258 On Sat, Aug 31, 2013 at 02:52:34AM +0000, Nicholas A. Bellinger wrote: > From: Nicholas Bellinger > > 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 > Cc: Asias He > Cc: Kent Overstreet > Signed-off-by: Nicholas Bellinger Reviewed-by: Asias He > --- > 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 majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/