* [Qemu-devel] [PATCH] Reducing stack frame size in stream_process_mem2s() @ 2016-10-03 16:56 rutu.shah.26 2016-10-03 17:02 ` Rutuja Shah 2016-10-03 20:39 ` Paolo Bonzini 0 siblings, 2 replies; 6+ messages in thread From: rutu.shah.26 @ 2016-10-03 16:56 UTC (permalink / raw) To: qemu-devel Cc: Rutuja Shah, edgar.iglesias, alistair.francis, qemu-arm, stefanh 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); 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 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] Reducing stack frame size in stream_process_mem2s() 2016-10-03 16:56 [Qemu-devel] [PATCH] Reducing stack frame size in stream_process_mem2s() rutu.shah.26 @ 2016-10-03 17:02 ` Rutuja Shah 2016-10-03 22:26 ` Edgar E. Iglesias 2016-10-03 20:39 ` Paolo Bonzini 1 sibling, 1 reply; 6+ messages in thread From: Rutuja Shah @ 2016-10-03 17:02 UTC (permalink / raw) To: qemu-devel Cc: Rutuja Shah, Edgar E. Iglesias, alistair.francis, qemu-arm, Stefan Hajnoczi ++ 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); > 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 > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] Reducing stack frame size in stream_process_mem2s() 2016-10-03 17:02 ` Rutuja Shah @ 2016-10-03 22:26 ` Edgar E. Iglesias 2016-10-04 13:23 ` Rutuja Shah 0 siblings, 1 reply; 6+ messages in thread From: Edgar E. Iglesias @ 2016-10-03 22:26 UTC (permalink / raw) To: Rutuja Shah; +Cc: qemu-devel, alistair.francis, qemu-arm, Stefan Hajnoczi 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 > > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] Reducing stack frame size in stream_process_mem2s() 2016-10-03 22:26 ` Edgar E. Iglesias @ 2016-10-04 13:23 ` Rutuja Shah 2016-10-04 17:19 ` Edgar E. Iglesias 0 siblings, 1 reply; 6+ messages in thread From: Rutuja Shah @ 2016-10-04 13:23 UTC (permalink / raw) To: Edgar E. Iglesias; +Cc: qemu-devel, alistair.francis, qemu-arm, Stefan Hajnoczi 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? 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 >> > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] Reducing stack frame size in stream_process_mem2s() 2016-10-04 13:23 ` Rutuja Shah @ 2016-10-04 17:19 ` Edgar E. Iglesias 0 siblings, 0 replies; 6+ messages in thread From: Edgar E. Iglesias @ 2016-10-04 17:19 UTC (permalink / raw) To: Rutuja Shah; +Cc: qemu-devel, alistair.francis, qemu-arm, Stefan Hajnoczi 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 > >> > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] Reducing stack frame size in stream_process_mem2s() 2016-10-03 16:56 [Qemu-devel] [PATCH] Reducing stack frame size in stream_process_mem2s() rutu.shah.26 2016-10-03 17:02 ` Rutuja Shah @ 2016-10-03 20:39 ` Paolo Bonzini 1 sibling, 0 replies; 6+ messages in thread From: Paolo Bonzini @ 2016-10-03 20:39 UTC (permalink / raw) To: rutu.shah.26, qemu-devel Cc: edgar.iglesias, stefanh, qemu-arm, alistair.francis On 03/10/2016 18:56, 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); Hello, please place the allocated buffer in struct Stream (see xilinx_axidma_realize) instead. This function is a hot path, and it's better to keep allocations outside it. Thanks, Paolo > 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, > ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-10-04 17:19 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-10-03 16:56 [Qemu-devel] [PATCH] Reducing stack frame size in stream_process_mem2s() rutu.shah.26 2016-10-03 17:02 ` Rutuja Shah 2016-10-03 22:26 ` Edgar E. Iglesias 2016-10-04 13:23 ` Rutuja Shah 2016-10-04 17:19 ` Edgar E. Iglesias 2016-10-03 20:39 ` Paolo Bonzini
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.