Re: [PATCH] drivers/vhost/scsi.c: avoid a 10-order allocation

Re: [PATCH] drivers/vhost/scsi.c: avoid a 10-order allocation

[Michael S. Tsirkin]

On Sun, Aug 18, 2013 at 12:18:38PM +0300, Michael S. Tsirkin wrote:
> On Sun, Aug 18, 2013 at 11:48:56AM +0300, Dan Aloni wrote:
> > On 3.10.7 and x86_64, as a result of sizeof(struct vhost_scsi) being
> > 2152960 bytes the allocation failed once on my development machine.
> > 
> > Saw it would be prudent to split the bulk of it, which is the vqs array
> > into separately allocated parts. sizeof(struct vhost_virtqueue) is
> > currently 16816 bytes.
> > 
> > Signed-off-by: Dan Aloni <alonid@stratoscale.com>
> 
> This extra indirection is likely to have measureable cost though.
> 
> net core saw a similar problem, it was fixed in patch
>     net: allow large number of tx queues
> 
> So let's do it in a similar way: try to allocate with
> GFP_KERNEL | __GFP_NOWARN | __GFP_REPEAT
> and if that fails, do vmalloc.
> 
> To free, we can do
>        if (is_vmalloc_addr())
>                vfree();
>        else
>                kfree();
> 
> 

Hi Dan,
were you going to make this change? Or prefer me to do it?

-- 
MST

Re: [PATCH] drivers/vhost/scsi.c: avoid a 10-order allocation

[Dan Aloni]

On Wed, Sep 04, 2013 at 12:02:01PM +0300, Michael S. Tsirkin wrote:
> On Sun, Aug 18, 2013 at 12:18:38PM +0300, Michael S. Tsirkin wrote:
> > On Sun, Aug 18, 2013 at 11:48:56AM +0300, Dan Aloni wrote:
> > > On 3.10.7 and x86_64, as a result of sizeof(struct vhost_scsi) being
> > > 2152960 bytes the allocation failed once on my development machine.
> > > 
> > > Saw it would be prudent to split the bulk of it, which is the vqs array
> > > into separately allocated parts. sizeof(struct vhost_virtqueue) is
> > > currently 16816 bytes.
> > > 
> > > Signed-off-by: Dan Aloni <alonid@stratoscale.com>
> > 
> > This extra indirection is likely to have measureable cost though.
> > 
> > net core saw a similar problem, it was fixed in patch
> >     net: allow large number of tx queues
> > 
> > So let's do it in a similar way: try to allocate with
> > GFP_KERNEL | __GFP_NOWARN | __GFP_REPEAT
> > and if that fails, do vmalloc.
> > 
> > To free, we can do
> >        if (is_vmalloc_addr())
> >                vfree();
> >        else
> >                kfree();
> > 
> > 
> 
> Hi Dan,
> were you going to make this change? Or prefer me to do it?

Hey Michael,

I prefer you go ahead and do as your suggestion. I got distracted with 
other matters in the meanwhile. 

-- 
Dan Aloni