Re: [Qemu-devel] [PATCH] Reducing stack frame size in stream_process_mem2s()

["Edgar E. Iglesias" <edgar.iglesias@gmail.com>]

On Tue, Oct 04, 2016 at 06:53:20PM +0530, Rutuja Shah wrote:
> Hi,
> Allocating txbuf in struct Stream seems to be good. I can see other
> pointers of struct Stream being allocated in xilinx_axidma_realize(),
> but I am not able to find their deallocation point anywhere?

Hi,

Actually, since the allocation is of fixed size you can just
use an array in the Stream struct.
Otherwise, there's an unrealize method that I think is meant
to be used for cleanup.

Best regards,
Edgar


> Regards
> Rutuja Shah
> 
> 
> On Tue, Oct 4, 2016 at 3:56 AM, Edgar E. Iglesias
> <edgar.iglesias@gmail.com> wrote:
> > On Mon, Oct 03, 2016 at 10:32:40PM +0530, Rutuja Shah wrote:
> >> ++ stefan Sorry for the typo.
> >> Regards
> >> Rutuja Shah
> >>
> >>
> >> On Mon, Oct 3, 2016 at 10:26 PM,  <rutu.shah.26@gmail.com> wrote:
> >> > From: Rutuja Shah <rutu.shah.26@gmail.com>
> >> >
> >> > This patch allocates memory for txbuf array on the heap rather than the stack.
> >> > As a result, the stack frame size is reduced.
> >> >
> >> > Signed-off-by: Rutuja Shah <rutu.shah.26@gmail.com>
> >> > ---
> >> >  hw/dma/xilinx_axidma.c | 4 +++-
> >> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >> >
> >> > diff --git a/hw/dma/xilinx_axidma.c b/hw/dma/xilinx_axidma.c
> >> > index b135a5f..6c63575 100644
> >> > --- a/hw/dma/xilinx_axidma.c
> >> > +++ b/hw/dma/xilinx_axidma.c
> >> > @@ -256,13 +256,14 @@ static void stream_process_mem2s(struct Stream *s, StreamSlave *tx_data_dev,
> >> >                                   StreamSlave *tx_control_dev)
> >> >  {
> >> >      uint32_t prev_d;
> >> > -    unsigned char txbuf[16 * 1024];
> >> > +    unsigned char *txbuf;
> >> >      unsigned int txlen;
> >> >
> >> >      if (!stream_running(s) || stream_idle(s)) {
> >> >          return;
> >> >      }
> >> >
> >> > +    txbuf = g_malloc(16 * 1024);
> >
> > Hi,
> >
> > Two comments.
> >
> > We need to move the allocation from the data-path to initialization of the DMA objects. (e.g put txbuf into the Stream struct)
> > We also need to fix up the use of sizeof txbuf.
> >
> > Best regards,
> > Edgar
> >
> >
> >> >      while (1) {
> >> >          stream_desc_load(s, s->regs[R_CURDESC]);
> >> >
> >> > @@ -304,6 +305,7 @@ static void stream_process_mem2s(struct Stream *s, StreamSlave *tx_data_dev,
> >> >              break;
> >> >          }
> >> >      }
> >> > +    g_free(txbuf);
> >> >  }
> >> >
> >> >  static size_t stream_process_s2mem(struct Stream *s, unsigned char *buf,
> >> > --
> >> > 1.9.1
> >> >