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
