| Message ID | 1475604703-3381-1-git-send-email-rutu.shah.26@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
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 >
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>
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 >> >
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); }