On Mon, 2013-09-23 at 22:57 +0300, Michael S. Tsirkin wrote: > On Mon, Sep 23, 2013 at 07:10:24PM +0000, Nicholas A. Bellinger wrote: > > From: Nicholas Bellinger <nab@xxxxxxxxxxxxxxx> > > > > This patch changes transport_alloc_session_tags() to fall back to > > use vzalloc when kzalloc fails for big tag_num that end up generating > > larger order allocations. > > Interesting. > How large can they be? > Does userspace control this? > > If yes we definitely need to limit the memory this can > consume in some way ... > Depends on the fabric module in question.. With vhost this is hardcoded to 256, but with iscsi/iser-target, this can be changed by user with default_cmdsn_depth or on a per NodeACL basis. The current hardcoded max for iscsi/iser-target is 512. struct se_cmd is currently 520 bytes on 64-bit btw.. > > Also use is_vmalloc_addr() in transport_alloc_session_tags() failure > > path, and normal transport_free_session() patch > > which patch? Whoops, this should have been 'path'. > > > to determine when > > vfree() needs to be called instead of kfree(). > > > > Cc: Michael S. Tsirkin <mst@xxxxxxxxxx> > > Cc: Asias He <asias@xxxxxxxxxx> > > Cc: Kent Overstreet <kmo@xxxxxxxxxxxxxx> > > Signed-off-by: Nicholas Bellinger <nab@xxxxxxxxxxxxxxx> > > --- > > drivers/target/target_core_transport.c | 17 +++++++++++++---- > > 1 file changed, 13 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c > > index 4aace78..fd7a063 100644 > > --- a/drivers/target/target_core_transport.c > > +++ b/drivers/target/target_core_transport.c > > @@ -238,15 +238,21 @@ int transport_alloc_session_tags(struct se_session *se_sess, > > > > se_sess->sess_cmd_map = kzalloc(tag_num * tag_size, GFP_KERNEL); > > This will spit warnings on failure, which you don't > want since it's expected here. > So add __GFP_NOWARN. <nod>, adding __GFP_NOWARN here.. > > Also, vmalloc memory is slower, since this affects > data path, add __GFP_REPEAT too? Looks OK to add, but not sure on the implications on here.. --nab > > > if (!se_sess->sess_cmd_map) { > > - pr_err("Unable to allocate se_sess->sess_cmd_map\n"); > > - return -ENOMEM; > > + se_sess->sess_cmd_map = vzalloc(tag_num * tag_size); > > + if (!se_sess->sess_cmd_map) { > > + pr_err("Unable to allocate se_sess->sess_cmd_map\n"); > > + return -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); > > - kfree(se_sess->sess_cmd_map); > > + if (is_vmalloc_addr(se_sess->sess_cmd_map)) > > + vfree(se_sess->sess_cmd_map); > > + else > > + kfree(se_sess->sess_cmd_map); > > se_sess->sess_cmd_map = NULL; > > return -ENOMEM; > > } > > @@ -412,7 +418,10 @@ 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); > > + if (is_vmalloc_addr(se_sess->sess_cmd_map)) > > + vfree(se_sess->sess_cmd_map); > > + else > > + kfree(se_sess->sess_cmd_map); > > } > > kmem_cache_free(se_sess_cache, se_sess); > > } > > -- > > 1.7.10.4 -- To unsubscribe from this list: send the line "unsubscribe target-devel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html