From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48422) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1brWlH-0004wU-JX for qemu-devel@nongnu.org; Tue, 04 Oct 2016 16:56:28 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1brWlD-0006To-5r for qemu-devel@nongnu.org; Tue, 04 Oct 2016 16:56:27 -0400 Date: Tue, 4 Oct 2016 22:56:19 +0200 From: "Edgar E. Iglesias" Message-ID: <20161004205619.GA31579@toto> References: <1475604703-3381-1-git-send-email-rutu.shah.26@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1475604703-3381-1-git-send-email-rutu.shah.26@gmail.com> Subject: Re: [Qemu-devel] [PATCHv2] Reducing stack frame size in stream_process_mem2s() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: rutu.shah.26@gmail.com Cc: qemu-devel@nongnu.org, alistair.francis@xilinx.com, qemu-arm@nongnu.org, stefanha@gmail.com On Tue, Oct 04, 2016 at 11:41:42PM +0530, rutu.shah.26@gmail.com wrote: > From: Rutuja Shah > > 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 Other than that, this looks good to me: Reviewed-by: Edgar E. Iglesias > Signed-off-by: Rutuja Shah > --- > 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 >