* [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 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
* 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
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.