Re: [Qemu-devel] [PATCH V2 1/1] virtio:Allocate temporary VirtQueueElementOld on heap

[Michael S. Tsirkin]

On Tue, Mar 15, 2016 at 10:16:00AM +0100, Paolo Bonzini wrote:
> 
> 
> On 15/03/2016 08:36, Jaya Tiwari wrote:
> > 
> >     This is not what that page suggests. It says:
> >     Make the stack array
> >     smaller and allocate on the heap in the rare case that the
> >     data does not fit in the small array:
> > 
> >     This patch just uses heap unconditionally which is sure to hurt
> >     performance.
> 
> This is not a hot path.  It only happens when saving/loading data after
> migration.  Surely the few microseconds wasted in allocating data on the
> heap are beaten by zeroing the memory, by all the for loops in the
> functions, and of course by the 3-500 *milli*seconds of downtime caused
> by migration.
> 
> > Yes Okay.
> > Thank you for pointing it out.
> > So I should be including a condition to check with a small stack size,
> > and if the array crosses it, only then
> > it should be placed in heap, otherwise it should not be using heap.
> > Am I correct in my understanding here? 
> 
> Jaya, this patch is okay.  What Michael said is true in other cases, but
> not this one.
> 
> Paolo

Hmm, I got confused. You are right.
I'll redo the review, sorry about the noise.

-- 
MST