Re: [PATCH] serial: tegra: Fix memory leak on DMA setup failure
On Wed, May 20, 2015 at 8:21 PM, Jon Hunter wrote: > If the call to dmaengine_slave_config() fails, then the DMA buffer will > not be freed/unmapped. Fix this by moving the code that stores the > address of the buffer in the tegra_uart_port structure to before the > call to dmaengine_slave_config(). > > Reported-by: Alexandre Courbot > Signed-off-by: Jon Hunter Looks good, we had the same if/else condition appearing three times in this function for no real reason anyway. This considerably simplifies the code. > drivers/tty/serial/serial-tegra.c | 32 +++- > 1 file changed, 11 insertions(+), 21 deletions(-) Negative number of lines, another good point for this patch! Reviewed-by: Alexandre Courbot -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] serial: tegra: Fix memory leak on DMA setup failure
On 20/05/15 12:25, Jon Hunter wrote: > > On 20/05/15 12:21, Jon Hunter wrote: >> If the call to dmaengine_slave_config() fails, then the DMA buffer will >> not be freed/unmapped. Fix this by moving the code that stores the >> address of the buffer in the tegra_uart_port structure to before the >> call to dmaengine_slave_config(). > > By the way, just to be clear, I did try to fix this before [1], but > failed :-( To be doubly clear, this is targeted to be applied on top of the previous patch [1] which is now in linux-next. Jon > [1] https://lkml.org/lkml/2015/5/5/802 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] serial: tegra: Fix memory leak on DMA setup failure
On 20/05/15 12:21, Jon Hunter wrote:
> If the call to dmaengine_slave_config() fails, then the DMA buffer will
> not be freed/unmapped. Fix this by moving the code that stores the
> address of the buffer in the tegra_uart_port structure to before the
> call to dmaengine_slave_config().
By the way, just to be clear, I did try to fix this before [1], but
failed :-(
Thanks to Alex for pointing this out.
Cheers
Jon
[1] https://lkml.org/lkml/2015/5/5/802
> Reported-by: Alexandre Courbot
> Signed-off-by: Jon Hunter
> ---
> drivers/tty/serial/serial-tegra.c | 32 +++-
> 1 file changed, 11 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/tty/serial/serial-tegra.c
> b/drivers/tty/serial/serial-tegra.c
> index 3b63f103f0c9..cf0133ae762d 100644
> --- a/drivers/tty/serial/serial-tegra.c
> +++ b/drivers/tty/serial/serial-tegra.c
> @@ -999,6 +999,12 @@ static int tegra_uart_dma_channel_allocate(struct
> tegra_uart_port *tup,
> dma_release_channel(dma_chan);
> return -ENOMEM;
> }
> + dma_sconfig.src_addr = tup->uport.mapbase;
> + dma_sconfig.src_addr_width = DMA_SLAVE_BUSWIDTH_1_BYTE;
> + dma_sconfig.src_maxburst = 4;
> + tup->rx_dma_chan = dma_chan;
> + tup->rx_dma_buf_virt = dma_buf;
> + tup->rx_dma_buf_phys = dma_phys;
> } else {
> dma_phys = dma_map_single(tup->uport.dev,
> tup->uport.state->xmit.buf, UART_XMIT_SIZE,
> @@ -1009,39 +1015,23 @@ static int tegra_uart_dma_channel_allocate(struct
> tegra_uart_port *tup,
> return -ENOMEM;
> }
> dma_buf = tup->uport.state->xmit.buf;
> - }
> -
> - if (dma_to_memory) {
> - dma_sconfig.src_addr = tup->uport.mapbase;
> - dma_sconfig.src_addr_width = DMA_SLAVE_BUSWIDTH_1_BYTE;
> - dma_sconfig.src_maxburst = 4;
> - } else {
> dma_sconfig.dst_addr = tup->uport.mapbase;
> dma_sconfig.dst_addr_width = DMA_SLAVE_BUSWIDTH_1_BYTE;
> dma_sconfig.dst_maxburst = 16;
> + tup->tx_dma_chan = dma_chan;
> + tup->tx_dma_buf_virt = dma_buf;
> + tup->tx_dma_buf_phys = dma_phys;
> }
>
> ret = dmaengine_slave_config(dma_chan, _sconfig);
> if (ret < 0) {
> dev_err(tup->uport.dev,
> "Dma slave config failed, err = %d\n", ret);
> - goto scrub;
> + tegra_uart_dma_channel_free(tup, dma_to_memory);
> + return ret;
> }
>
> - if (dma_to_memory) {
> - tup->rx_dma_chan = dma_chan;
> - tup->rx_dma_buf_virt = dma_buf;
> - tup->rx_dma_buf_phys = dma_phys;
> - } else {
> - tup->tx_dma_chan = dma_chan;
> - tup->tx_dma_buf_virt = dma_buf;
> - tup->tx_dma_buf_phys = dma_phys;
> - }
> return 0;
> -
> -scrub:
> - tegra_uart_dma_channel_free(tup, dma_to_memory);
> - return ret;
> }
>
> static int tegra_uart_startup(struct uart_port *u)
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] serial: tegra: Fix memory leak on DMA setup failure
If the call to dmaengine_slave_config() fails, then the DMA buffer will
not be freed/unmapped. Fix this by moving the code that stores the
address of the buffer in the tegra_uart_port structure to before the
call to dmaengine_slave_config().
Reported-by: Alexandre Courbot
Signed-off-by: Jon Hunter
---
drivers/tty/serial/serial-tegra.c | 32 +++-
1 file changed, 11 insertions(+), 21 deletions(-)
diff --git a/drivers/tty/serial/serial-tegra.c
b/drivers/tty/serial/serial-tegra.c
index 3b63f103f0c9..cf0133ae762d 100644
--- a/drivers/tty/serial/serial-tegra.c
+++ b/drivers/tty/serial/serial-tegra.c
@@ -999,6 +999,12 @@ static int tegra_uart_dma_channel_allocate(struct
tegra_uart_port *tup,
dma_release_channel(dma_chan);
return -ENOMEM;
}
+ dma_sconfig.src_addr = tup->uport.mapbase;
+ dma_sconfig.src_addr_width = DMA_SLAVE_BUSWIDTH_1_BYTE;
+ dma_sconfig.src_maxburst = 4;
+ tup->rx_dma_chan = dma_chan;
+ tup->rx_dma_buf_virt = dma_buf;
+ tup->rx_dma_buf_phys = dma_phys;
} else {
dma_phys = dma_map_single(tup->uport.dev,
tup->uport.state->xmit.buf, UART_XMIT_SIZE,
@@ -1009,39 +1015,23 @@ static int tegra_uart_dma_channel_allocate(struct
tegra_uart_port *tup,
return -ENOMEM;
}
dma_buf = tup->uport.state->xmit.buf;
- }
-
- if (dma_to_memory) {
- dma_sconfig.src_addr = tup->uport.mapbase;
- dma_sconfig.src_addr_width = DMA_SLAVE_BUSWIDTH_1_BYTE;
- dma_sconfig.src_maxburst = 4;
- } else {
dma_sconfig.dst_addr = tup->uport.mapbase;
dma_sconfig.dst_addr_width = DMA_SLAVE_BUSWIDTH_1_BYTE;
dma_sconfig.dst_maxburst = 16;
+ tup->tx_dma_chan = dma_chan;
+ tup->tx_dma_buf_virt = dma_buf;
+ tup->tx_dma_buf_phys = dma_phys;
}
ret = dmaengine_slave_config(dma_chan, _sconfig);
if (ret < 0) {
dev_err(tup->uport.dev,
"Dma slave config failed, err = %d\n", ret);
- goto scrub;
+ tegra_uart_dma_channel_free(tup, dma_to_memory);
+ return ret;
}
- if (dma_to_memory) {
- tup->rx_dma_chan = dma_chan;
- tup->rx_dma_buf_virt = dma_buf;
- tup->rx_dma_buf_phys = dma_phys;
- } else {
- tup->tx_dma_chan = dma_chan;
- tup->tx_dma_buf_virt = dma_buf;
- tup->tx_dma_buf_phys = dma_phys;
- }
return 0;
-
-scrub:
- tegra_uart_dma_channel_free(tup, dma_to_memory);
- return ret;
}
static int tegra_uart_startup(struct uart_port *u)
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] serial: tegra: Fix memory leak on DMA setup failure
If the call to dmaengine_slave_config() fails, then the DMA buffer will
not be freed/unmapped. Fix this by moving the code that stores the
address of the buffer in the tegra_uart_port structure to before the
call to dmaengine_slave_config().
Reported-by: Alexandre Courbot acour...@nvidia.com
Signed-off-by: Jon Hunter jonath...@nvidia.com
---
drivers/tty/serial/serial-tegra.c | 32 +++-
1 file changed, 11 insertions(+), 21 deletions(-)
diff --git a/drivers/tty/serial/serial-tegra.c
b/drivers/tty/serial/serial-tegra.c
index 3b63f103f0c9..cf0133ae762d 100644
--- a/drivers/tty/serial/serial-tegra.c
+++ b/drivers/tty/serial/serial-tegra.c
@@ -999,6 +999,12 @@ static int tegra_uart_dma_channel_allocate(struct
tegra_uart_port *tup,
dma_release_channel(dma_chan);
return -ENOMEM;
}
+ dma_sconfig.src_addr = tup-uport.mapbase;
+ dma_sconfig.src_addr_width = DMA_SLAVE_BUSWIDTH_1_BYTE;
+ dma_sconfig.src_maxburst = 4;
+ tup-rx_dma_chan = dma_chan;
+ tup-rx_dma_buf_virt = dma_buf;
+ tup-rx_dma_buf_phys = dma_phys;
} else {
dma_phys = dma_map_single(tup-uport.dev,
tup-uport.state-xmit.buf, UART_XMIT_SIZE,
@@ -1009,39 +1015,23 @@ static int tegra_uart_dma_channel_allocate(struct
tegra_uart_port *tup,
return -ENOMEM;
}
dma_buf = tup-uport.state-xmit.buf;
- }
-
- if (dma_to_memory) {
- dma_sconfig.src_addr = tup-uport.mapbase;
- dma_sconfig.src_addr_width = DMA_SLAVE_BUSWIDTH_1_BYTE;
- dma_sconfig.src_maxburst = 4;
- } else {
dma_sconfig.dst_addr = tup-uport.mapbase;
dma_sconfig.dst_addr_width = DMA_SLAVE_BUSWIDTH_1_BYTE;
dma_sconfig.dst_maxburst = 16;
+ tup-tx_dma_chan = dma_chan;
+ tup-tx_dma_buf_virt = dma_buf;
+ tup-tx_dma_buf_phys = dma_phys;
}
ret = dmaengine_slave_config(dma_chan, dma_sconfig);
if (ret 0) {
dev_err(tup-uport.dev,
Dma slave config failed, err = %d\n, ret);
- goto scrub;
+ tegra_uart_dma_channel_free(tup, dma_to_memory);
+ return ret;
}
- if (dma_to_memory) {
- tup-rx_dma_chan = dma_chan;
- tup-rx_dma_buf_virt = dma_buf;
- tup-rx_dma_buf_phys = dma_phys;
- } else {
- tup-tx_dma_chan = dma_chan;
- tup-tx_dma_buf_virt = dma_buf;
- tup-tx_dma_buf_phys = dma_phys;
- }
return 0;
-
-scrub:
- tegra_uart_dma_channel_free(tup, dma_to_memory);
- return ret;
}
static int tegra_uart_startup(struct uart_port *u)
--
1.9.1
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] serial: tegra: Fix memory leak on DMA setup failure
On 20/05/15 12:21, Jon Hunter wrote:
If the call to dmaengine_slave_config() fails, then the DMA buffer will
not be freed/unmapped. Fix this by moving the code that stores the
address of the buffer in the tegra_uart_port structure to before the
call to dmaengine_slave_config().
By the way, just to be clear, I did try to fix this before [1], but
failed :-(
Thanks to Alex for pointing this out.
Cheers
Jon
[1] https://lkml.org/lkml/2015/5/5/802
Reported-by: Alexandre Courbot acour...@nvidia.com
Signed-off-by: Jon Hunter jonath...@nvidia.com
---
drivers/tty/serial/serial-tegra.c | 32 +++-
1 file changed, 11 insertions(+), 21 deletions(-)
diff --git a/drivers/tty/serial/serial-tegra.c
b/drivers/tty/serial/serial-tegra.c
index 3b63f103f0c9..cf0133ae762d 100644
--- a/drivers/tty/serial/serial-tegra.c
+++ b/drivers/tty/serial/serial-tegra.c
@@ -999,6 +999,12 @@ static int tegra_uart_dma_channel_allocate(struct
tegra_uart_port *tup,
dma_release_channel(dma_chan);
return -ENOMEM;
}
+ dma_sconfig.src_addr = tup-uport.mapbase;
+ dma_sconfig.src_addr_width = DMA_SLAVE_BUSWIDTH_1_BYTE;
+ dma_sconfig.src_maxburst = 4;
+ tup-rx_dma_chan = dma_chan;
+ tup-rx_dma_buf_virt = dma_buf;
+ tup-rx_dma_buf_phys = dma_phys;
} else {
dma_phys = dma_map_single(tup-uport.dev,
tup-uport.state-xmit.buf, UART_XMIT_SIZE,
@@ -1009,39 +1015,23 @@ static int tegra_uart_dma_channel_allocate(struct
tegra_uart_port *tup,
return -ENOMEM;
}
dma_buf = tup-uport.state-xmit.buf;
- }
-
- if (dma_to_memory) {
- dma_sconfig.src_addr = tup-uport.mapbase;
- dma_sconfig.src_addr_width = DMA_SLAVE_BUSWIDTH_1_BYTE;
- dma_sconfig.src_maxburst = 4;
- } else {
dma_sconfig.dst_addr = tup-uport.mapbase;
dma_sconfig.dst_addr_width = DMA_SLAVE_BUSWIDTH_1_BYTE;
dma_sconfig.dst_maxburst = 16;
+ tup-tx_dma_chan = dma_chan;
+ tup-tx_dma_buf_virt = dma_buf;
+ tup-tx_dma_buf_phys = dma_phys;
}
ret = dmaengine_slave_config(dma_chan, dma_sconfig);
if (ret 0) {
dev_err(tup-uport.dev,
Dma slave config failed, err = %d\n, ret);
- goto scrub;
+ tegra_uart_dma_channel_free(tup, dma_to_memory);
+ return ret;
}
- if (dma_to_memory) {
- tup-rx_dma_chan = dma_chan;
- tup-rx_dma_buf_virt = dma_buf;
- tup-rx_dma_buf_phys = dma_phys;
- } else {
- tup-tx_dma_chan = dma_chan;
- tup-tx_dma_buf_virt = dma_buf;
- tup-tx_dma_buf_phys = dma_phys;
- }
return 0;
-
-scrub:
- tegra_uart_dma_channel_free(tup, dma_to_memory);
- return ret;
}
static int tegra_uart_startup(struct uart_port *u)
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] serial: tegra: Fix memory leak on DMA setup failure
On 20/05/15 12:25, Jon Hunter wrote: On 20/05/15 12:21, Jon Hunter wrote: If the call to dmaengine_slave_config() fails, then the DMA buffer will not be freed/unmapped. Fix this by moving the code that stores the address of the buffer in the tegra_uart_port structure to before the call to dmaengine_slave_config(). By the way, just to be clear, I did try to fix this before [1], but failed :-( To be doubly clear, this is targeted to be applied on top of the previous patch [1] which is now in linux-next. Jon [1] https://lkml.org/lkml/2015/5/5/802 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] serial: tegra: Fix memory leak on DMA setup failure
On Wed, May 20, 2015 at 8:21 PM, Jon Hunter jonath...@nvidia.com wrote: If the call to dmaengine_slave_config() fails, then the DMA buffer will not be freed/unmapped. Fix this by moving the code that stores the address of the buffer in the tegra_uart_port structure to before the call to dmaengine_slave_config(). Reported-by: Alexandre Courbot acour...@nvidia.com Signed-off-by: Jon Hunter jonath...@nvidia.com Looks good, we had the same if/else condition appearing three times in this function for no real reason anyway. This considerably simplifies the code. drivers/tty/serial/serial-tegra.c | 32 +++- 1 file changed, 11 insertions(+), 21 deletions(-) Negative number of lines, another good point for this patch! Reviewed-by: Alexandre Courbot acour...@nvidia.com -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
