* [PATCH 2/2] ARM: dma-mapping: fix leak in consistent_init
@ 2012-02-17 13:09 Ajeet Yadav
2012-02-17 13:12 ` Russell King - ARM Linux
2012-02-18 15:58 ` Tixy
0 siblings, 2 replies; 6+ messages in thread
From: Ajeet Yadav @ 2012-02-17 13:09 UTC (permalink / raw)
To: linux-arm-kernel
Although the error in this case is unlikely, but logically
if error occurs then we leak memory.
Signed-off-by: Ajeet Yadav <ajeet.yadav.77@gmail.com>
---
arch/arm/mm/dma-mapping.c | 24 ++++++++++++------------
1 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
index 04bfa76..b8cf062 100644
--- a/arch/arm/mm/dma-mapping.c
+++ b/arch/arm/mm/dma-mapping.c
@@ -161,7 +161,6 @@ static struct arm_vmregion_head consistent_head = {
*/
static int __init consistent_init(void)
{
- int ret = 0;
pgd_t *pgd;
pud_t *pud;
pmd_t *pmd;
@@ -171,7 +170,7 @@ static int __init consistent_init(void)
unsigned long num_ptes = (CONSISTENT_END - base) >> PMD_SHIFT;
consistent_pte = kmalloc(num_ptes * sizeof(pte_t *), GFP_KERNEL);
- if (!consistent_pte) {
+ if (unlikely(!consistent_pte)) {
pr_err("%s: no memory\n", __func__);
return -ENOMEM;
}
@@ -183,32 +182,33 @@ static int __init consistent_init(void)
pgd = pgd_offset(&init_mm, base);
pud = pud_alloc(&init_mm, pgd, base);
- if (!pud) {
+ if (unlikely(!pud)) {
printk(KERN_ERR "%s: no pud tables\n", __func__);
- ret = -ENOMEM;
- break;
+ goto err;
}
pmd = pmd_alloc(&init_mm, pud, base);
- if (!pmd) {
+ if (unlikely(!pmd)) {
printk(KERN_ERR "%s: no pmd tables\n", __func__);
- ret = -ENOMEM;
- break;
+ goto err;
}
WARN_ON(!pmd_none(*pmd));
pte = pte_alloc_kernel(pmd, base);
- if (!pte) {
+ if (unlikely(!pte)) {
printk(KERN_ERR "%s: no pte tables\n", __func__);
- ret = -ENOMEM;
- break;
+ goto err;
}
consistent_pte[i++] = pte;
base += PMD_SIZE;
} while (base < CONSISTENT_END);
- return ret;
+ return 0;
+err:
+ kfree(consistent_pte);
+ consistent_pte = NULL;
+ return -ENOMEM;
}
core_initcall(consistent_init);
--
1.7.8.4
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 2/2] ARM: dma-mapping: fix leak in consistent_init
2012-02-17 13:09 [PATCH 2/2] ARM: dma-mapping: fix leak in consistent_init Ajeet Yadav
@ 2012-02-17 13:12 ` Russell King - ARM Linux
2012-02-17 13:28 ` Ajeet Yadav
2012-02-18 15:58 ` Tixy
1 sibling, 1 reply; 6+ messages in thread
From: Russell King - ARM Linux @ 2012-02-17 13:12 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Feb 17, 2012 at 06:39:55PM +0530, Ajeet Yadav wrote:
> Although the error in this case is unlikely, but logically
> if error occurs then we leak memory.
>
> Signed-off-by: Ajeet Yadav <ajeet.yadav.77@gmail.com>
> ---
> arch/arm/mm/dma-mapping.c | 24 ++++++++++++------------
> 1 files changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
> index 04bfa76..b8cf062 100644
> --- a/arch/arm/mm/dma-mapping.c
> +++ b/arch/arm/mm/dma-mapping.c
> @@ -161,7 +161,6 @@ static struct arm_vmregion_head consistent_head = {
> */
> static int __init consistent_init(void)
> {
> - int ret = 0;
> pgd_t *pgd;
> pud_t *pud;
> pmd_t *pmd;
> @@ -171,7 +170,7 @@ static int __init consistent_init(void)
> unsigned long num_ptes = (CONSISTENT_END - base) >> PMD_SHIFT;
>
> consistent_pte = kmalloc(num_ptes * sizeof(pte_t *), GFP_KERNEL);
> - if (!consistent_pte) {
> + if (unlikely(!consistent_pte)) {
Please get rid of these unlikelys. This really isn't a performance
critical path.
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH 2/2] ARM: dma-mapping: fix leak in consistent_init
2012-02-17 13:12 ` Russell King - ARM Linux
@ 2012-02-17 13:28 ` Ajeet Yadav
0 siblings, 0 replies; 6+ messages in thread
From: Ajeet Yadav @ 2012-02-17 13:28 UTC (permalink / raw)
To: linux-arm-kernel
thank you for kind review, but how can I resent the patch using same mail chain
git send-email --smtp-encryption=tls --smtp-server=smtp.gmail.com
--smtp-user=ajeet.yadav.77 at gmail.com --smtp-server-port=587 --from
"Ajeet Yadav <ajeet.yadav.77@gmail.com>" --to "Russell King
<linux@arm.linux.org.uk>" --to "Jon Medhurst <tixy@yxit.co.uk>" --to
"Nicolas Pitre <nicolas.pitre@linaro.org>" --to "Catalin Marinas
<catalin.marinas@arm.com>" --to "Sumit Bhattacharya
<sumitb@nvidia.com>" --to "linux-arm-kernel at lists.infradead.org" --to
"linux-kernel at vger.kernel.org"
0002-ARM-dma-mapping-fix-leak-in-consistent_init.patch
On Fri, Feb 17, 2012 at 6:42 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Fri, Feb 17, 2012 at 06:39:55PM +0530, Ajeet Yadav wrote:
>> Although the error in this case is unlikely, but logically
>> if error occurs then we leak memory.
>>
>> Signed-off-by: Ajeet Yadav <ajeet.yadav.77@gmail.com>
>> ---
>> ?arch/arm/mm/dma-mapping.c | ? 24 ++++++++++++------------
>> ?1 files changed, 12 insertions(+), 12 deletions(-)
>>
>> diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
>> index 04bfa76..b8cf062 100644
>> --- a/arch/arm/mm/dma-mapping.c
>> +++ b/arch/arm/mm/dma-mapping.c
>> @@ -161,7 +161,6 @@ static struct arm_vmregion_head consistent_head = {
>> ? */
>> ?static int __init consistent_init(void)
>> ?{
>> - ? ? int ret = 0;
>> ? ? ? pgd_t *pgd;
>> ? ? ? pud_t *pud;
>> ? ? ? pmd_t *pmd;
>> @@ -171,7 +170,7 @@ static int __init consistent_init(void)
>> ? ? ? unsigned long num_ptes = (CONSISTENT_END - base) >> PMD_SHIFT;
>>
>> ? ? ? consistent_pte = kmalloc(num_ptes * sizeof(pte_t *), GFP_KERNEL);
>> - ? ? if (!consistent_pte) {
>> + ? ? if (unlikely(!consistent_pte)) {
>
> Please get rid of these unlikelys. ?This really isn't a performance
> critical path.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/2] ARM: dma-mapping: fix leak in consistent_init
2012-02-17 13:09 [PATCH 2/2] ARM: dma-mapping: fix leak in consistent_init Ajeet Yadav
2012-02-17 13:12 ` Russell King - ARM Linux
@ 2012-02-18 15:58 ` Tixy
[not found] ` <CAB4K4y5CV_MuEegiQuYn=_JN7zwxF11b4_kPvQVFS-V6mBwQmg@mail.gmail.com>
1 sibling, 1 reply; 6+ messages in thread
From: Tixy @ 2012-02-18 15:58 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, 2012-02-17 at 18:39 +0530, Ajeet Yadav wrote:
> Although the error in this case is unlikely, but logically
> if error occurs then we leak memory.
>
> Signed-off-by: Ajeet Yadav <ajeet.yadav.77@gmail.com>
If you want to fix all the memory leaks then the page tables allocated
by pte_alloc_kernel() need freeing as well, (and the pud and pmd
tables?).
However, if we run out of memory this early in boot, then the system is
unusable anyway and it doesn't seem worth adding the extra code
complexity to avoid any of these memory leaks.
--
Tixy
> ---
> arch/arm/mm/dma-mapping.c | 24 ++++++++++++------------
> 1 files changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
> index 04bfa76..b8cf062 100644
> --- a/arch/arm/mm/dma-mapping.c
> +++ b/arch/arm/mm/dma-mapping.c
> @@ -161,7 +161,6 @@ static struct arm_vmregion_head consistent_head = {
> */
> static int __init consistent_init(void)
> {
> - int ret = 0;
> pgd_t *pgd;
> pud_t *pud;
> pmd_t *pmd;
> @@ -171,7 +170,7 @@ static int __init consistent_init(void)
> unsigned long num_ptes = (CONSISTENT_END - base) >> PMD_SHIFT;
>
> consistent_pte = kmalloc(num_ptes * sizeof(pte_t *), GFP_KERNEL);
> - if (!consistent_pte) {
> + if (unlikely(!consistent_pte)) {
> pr_err("%s: no memory\n", __func__);
> return -ENOMEM;
> }
> @@ -183,32 +182,33 @@ static int __init consistent_init(void)
> pgd = pgd_offset(&init_mm, base);
>
> pud = pud_alloc(&init_mm, pgd, base);
> - if (!pud) {
> + if (unlikely(!pud)) {
> printk(KERN_ERR "%s: no pud tables\n", __func__);
> - ret = -ENOMEM;
> - break;
> + goto err;
> }
>
> pmd = pmd_alloc(&init_mm, pud, base);
> - if (!pmd) {
> + if (unlikely(!pmd)) {
> printk(KERN_ERR "%s: no pmd tables\n", __func__);
> - ret = -ENOMEM;
> - break;
> + goto err;
> }
> WARN_ON(!pmd_none(*pmd));
>
> pte = pte_alloc_kernel(pmd, base);
> - if (!pte) {
> + if (unlikely(!pte)) {
> printk(KERN_ERR "%s: no pte tables\n", __func__);
> - ret = -ENOMEM;
> - break;
> + goto err;
> }
>
> consistent_pte[i++] = pte;
> base += PMD_SIZE;
> } while (base < CONSISTENT_END);
>
> - return ret;
> + return 0;
> +err:
> + kfree(consistent_pte);
> + consistent_pte = NULL;
> + return -ENOMEM;
> }
>
> core_initcall(consistent_init);
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/2] ARM: dma-mapping: fix leak in consistent_init
@ 2012-02-17 13:27 Ajeet Yadav
0 siblings, 0 replies; 6+ messages in thread
From: Ajeet Yadav @ 2012-02-17 13:27 UTC (permalink / raw)
To: linux-arm-kernel
Although the error in this case is unlikely, but logically
if error occurs then we leak memory.
Signed-off-by: Ajeet Yadav <ajeet.yadav.77@gmail.com>
---
arch/arm/mm/dma-mapping.c | 16 ++++++++--------
1 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
index 04bfa76..932d288 100644
--- a/arch/arm/mm/dma-mapping.c
+++ b/arch/arm/mm/dma-mapping.c
@@ -161,7 +161,6 @@ static struct arm_vmregion_head consistent_head = {
*/
static int __init consistent_init(void)
{
- int ret = 0;
pgd_t *pgd;
pud_t *pud;
pmd_t *pmd;
@@ -185,30 +184,31 @@ static int __init consistent_init(void)
pud = pud_alloc(&init_mm, pgd, base);
if (!pud) {
printk(KERN_ERR "%s: no pud tables\n", __func__);
- ret = -ENOMEM;
- break;
+ goto err;
}
pmd = pmd_alloc(&init_mm, pud, base);
if (!pmd) {
printk(KERN_ERR "%s: no pmd tables\n", __func__);
- ret = -ENOMEM;
- break;
+ goto err;
}
WARN_ON(!pmd_none(*pmd));
pte = pte_alloc_kernel(pmd, base);
if (!pte) {
printk(KERN_ERR "%s: no pte tables\n", __func__);
- ret = -ENOMEM;
- break;
+ goto err;
}
consistent_pte[i++] = pte;
base += PMD_SIZE;
} while (base < CONSISTENT_END);
- return ret;
+ return 0;
+err:
+ kfree(consistent_pte);
+ consistent_pte = NULL;
+ return -ENOMEM;
}
core_initcall(consistent_init);
--
1.7.8.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-02-20 19:08 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-02-17 13:09 [PATCH 2/2] ARM: dma-mapping: fix leak in consistent_init Ajeet Yadav
2012-02-17 13:12 ` Russell King - ARM Linux
2012-02-17 13:28 ` Ajeet Yadav
2012-02-18 15:58 ` Tixy
[not found] ` <CAB4K4y5CV_MuEegiQuYn=_JN7zwxF11b4_kPvQVFS-V6mBwQmg@mail.gmail.com>
2012-02-20 19:08 ` Tixy
-- strict thread matches above, loose matches on Subject: below --
2012-02-17 13:27 Ajeet Yadav
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).