[Qemu-devel] [PATCHv2] Reducing stack frame size in stream_process_mem2s()

[Qemu-devel] [PATCHv2] Reducing stack frame size in stream_process_mem2s()

[rutu.shah.26]

From: Rutuja Shah <rutu.shah.26@gmail.com>

Hi,
This patch allocates memory for txbuf in struct Stream rather than the stack.
As a result, the stack frame size is reduced of stream_process_mem2s().
Signed-off-by: Rutuja Shah <rutu.shah.26@gmail.com>
---
 hw/dma/xilinx_axidma.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/hw/dma/xilinx_axidma.c b/hw/dma/xilinx_axidma.c
index b135a5f..6065689 100644
--- a/hw/dma/xilinx_axidma.c
+++ b/hw/dma/xilinx_axidma.c
@@ -111,6 +111,7 @@ struct Stream {
     unsigned int complete_cnt;
     uint32_t regs[R_MAX];
     uint8_t app[20];
+    unsigned char txbuf[16 * 1024];
 };
 
 struct XilinxAXIDMAStreamSlave {
@@ -256,7 +257,6 @@ 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 int txlen;
 
     if (!stream_running(s) || stream_idle(s)) {
@@ -277,17 +277,17 @@ static void stream_process_mem2s(struct Stream *s, StreamSlave *tx_data_dev,
         }
 
         txlen = s->desc.control & SDESC_CTRL_LEN_MASK;
-        if ((txlen + s->pos) > sizeof txbuf) {
+        if ((txlen + s->pos) > sizeof s->txbuf) {
             hw_error("%s: too small internal txbuf! %d\n", __func__,
                      txlen + s->pos);
         }
 
         cpu_physical_memory_read(s->desc.buffer_address,
-                                 txbuf + s->pos, txlen);
+                                 s->txbuf + s->pos, txlen);
         s->pos += txlen;
 
         if (stream_desc_eof(&s->desc)) {
-            stream_push(tx_data_dev, txbuf, s->pos);
+            stream_push(tx_data_dev, s->txbuf, s->pos);
             s->pos = 0;
             stream_complete(s);
         }
-- 
1.9.1

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

[no-reply]

Hi,

Your series failed automatic build test. Please find the testing commands and
their output below. If you have docker installed, you can probably reproduce it
locally.

Type: series
Message-id: 1475604703-3381-1-git-send-email-rutu.shah.26@gmail.com
Subject: [Qemu-devel] [PATCHv2] Reducing stack frame size in stream_process_mem2s()

=== TEST SCRIPT BEGIN ===
#!/bin/bash
set -e
git submodule update --init dtc
# Let docker tests dump environment info
export SHOW_ENV=1
make J=8 docker-test-quick@centos6
make J=8 docker-test-mingw@fedora
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
4276ebf Reducing stack frame size in stream_process_mem2s()

=== OUTPUT BEGIN ===
Submodule 'dtc' (git://git.qemu-project.org/dtc.git) registered for path 'dtc'
Cloning into 'dtc'...
Submodule path 'dtc': checked out '65cc4d2748a2c2e6f27f1cf39e07a5dbabd80ebf'
  BUILD centos6
  ARCHIVE qemu.tgz
  ARCHIVE dtc.tgz
  COPY RUNNER
  RUN test-quick in centos6
=== OUTPUT END ===

Abort: command timeout (>3600 seconds)


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@freelists.org

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

[Edgar E. Iglesias]

On Tue, Oct 04, 2016 at 11:41:42PM +0530, rutu.shah.26@gmail.com wrote:
> From: Rutuja Shah <rutu.shah.26@gmail.com>
> 
> Hi,
> This patch allocates memory for txbuf in struct Stream rather than the stack.
> As a result, the stack frame size is reduced of stream_process_mem2s().

Hi Rutuja,

A nit-pick:
The commit message is not meant to be a converstation-like message, so
normally we don't include greetings.
The expected format is to have an empty line between the message and
the SoB line.

Here's an example:

Allocate memory for txbuf in struct Stream rather than on the stack.
As a result, the stack frame size is reduced for stream_process_mem2s().

Signed-off-by: Rutuja Shah <rutu.shah.26@gmail.com>


Other than that, this looks good to me:
Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>


> Signed-off-by: Rutuja Shah <rutu.shah.26@gmail.com>
> ---
>  hw/dma/xilinx_axidma.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/dma/xilinx_axidma.c b/hw/dma/xilinx_axidma.c
> index b135a5f..6065689 100644
> --- a/hw/dma/xilinx_axidma.c
> +++ b/hw/dma/xilinx_axidma.c
> @@ -111,6 +111,7 @@ struct Stream {
>      unsigned int complete_cnt;
>      uint32_t regs[R_MAX];
>      uint8_t app[20];
> +    unsigned char txbuf[16 * 1024];
>  };
>  
>  struct XilinxAXIDMAStreamSlave {
> @@ -256,7 +257,6 @@ 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 int txlen;
>  
>      if (!stream_running(s) || stream_idle(s)) {
> @@ -277,17 +277,17 @@ static void stream_process_mem2s(struct Stream *s, StreamSlave *tx_data_dev,
>          }
>  
>          txlen = s->desc.control & SDESC_CTRL_LEN_MASK;
> -        if ((txlen + s->pos) > sizeof txbuf) {
> +        if ((txlen + s->pos) > sizeof s->txbuf) {
>              hw_error("%s: too small internal txbuf! %d\n", __func__,
>                       txlen + s->pos);
>          }
>  
>          cpu_physical_memory_read(s->desc.buffer_address,
> -                                 txbuf + s->pos, txlen);
> +                                 s->txbuf + s->pos, txlen);
>          s->pos += txlen;
>  
>          if (stream_desc_eof(&s->desc)) {
> -            stream_push(tx_data_dev, txbuf, s->pos);
> +            stream_push(tx_data_dev, s->txbuf, s->pos);
>              s->pos = 0;
>              stream_complete(s);
>          }
> -- 
> 1.9.1
> 

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

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 527 bytes --]

On Tue, Oct 04, 2016 at 11:41:42PM +0530, rutu.shah.26@gmail.com wrote:
> From: Rutuja Shah <rutu.shah.26@gmail.com>
> 
> Hi,
> This patch allocates memory for txbuf in struct Stream rather than the stack.
> As a result, the stack frame size is reduced of stream_process_mem2s().
> Signed-off-by: Rutuja Shah <rutu.shah.26@gmail.com>
> ---
>  hw/dma/xilinx_axidma.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)

Thanks for your contribution!

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

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

[Alistair Francis]

On Tue, Oct 4, 2016 at 1:56 PM, Edgar E. Iglesias
<edgar.iglesias@gmail.com> wrote:
> On Tue, Oct 04, 2016 at 11:41:42PM +0530, rutu.shah.26@gmail.com wrote:
>> From: Rutuja Shah <rutu.shah.26@gmail.com>
>>
>> Hi,
>> This patch allocates memory for txbuf in struct Stream rather than the stack.
>> As a result, the stack frame size is reduced of stream_process_mem2s().
>
> Hi Rutuja,
>
> A nit-pick:
> The commit message is not meant to be a converstation-like message, so
> normally we don't include greetings.
> The expected format is to have an empty line between the message and
> the SoB line.
>
> Here's an example:
>
> Allocate memory for txbuf in struct Stream rather than on the stack.
> As a result, the stack frame size is reduced for stream_process_mem2s().
>
> Signed-off-by: Rutuja Shah <rutu.shah.26@gmail.com>
>
>
> Other than that, this looks good to me:
> Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>

I agree with Edgar, just taking out the hi and adding a new line
should be enough.
That can probably be done when applying it?

Reviewed-by: Alistair Francis <alistair.francis@xilinx.com>

Thanks,

Alistair

>
>
>> Signed-off-by: Rutuja Shah <rutu.shah.26@gmail.com>
>> ---
>>  hw/dma/xilinx_axidma.c | 8 ++++----
>>  1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/hw/dma/xilinx_axidma.c b/hw/dma/xilinx_axidma.c
>> index b135a5f..6065689 100644
>> --- a/hw/dma/xilinx_axidma.c
>> +++ b/hw/dma/xilinx_axidma.c
>> @@ -111,6 +111,7 @@ struct Stream {
>>      unsigned int complete_cnt;
>>      uint32_t regs[R_MAX];
>>      uint8_t app[20];
>> +    unsigned char txbuf[16 * 1024];
>>  };
>>
>>  struct XilinxAXIDMAStreamSlave {
>> @@ -256,7 +257,6 @@ 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 int txlen;
>>
>>      if (!stream_running(s) || stream_idle(s)) {
>> @@ -277,17 +277,17 @@ static void stream_process_mem2s(struct Stream *s, StreamSlave *tx_data_dev,
>>          }
>>
>>          txlen = s->desc.control & SDESC_CTRL_LEN_MASK;
>> -        if ((txlen + s->pos) > sizeof txbuf) {
>> +        if ((txlen + s->pos) > sizeof s->txbuf) {
>>              hw_error("%s: too small internal txbuf! %d\n", __func__,
>>                       txlen + s->pos);
>>          }
>>
>>          cpu_physical_memory_read(s->desc.buffer_address,
>> -                                 txbuf + s->pos, txlen);
>> +                                 s->txbuf + s->pos, txlen);
>>          s->pos += txlen;
>>
>>          if (stream_desc_eof(&s->desc)) {
>> -            stream_push(tx_data_dev, txbuf, s->pos);
>> +            stream_push(tx_data_dev, s->txbuf, s->pos);
>>              s->pos = 0;
>>              stream_complete(s);
>>          }
>> --
>> 1.9.1
>>
>