2009-07-07 16:13:42

by Figo.zhang

Subject: [PATCH] vmalloc.c: fix double error checking

it is no need for double error checking.

Signed-off-by: Figo.zhang <[email protected]>
---
mm/vmalloc.c | 9 ++-------
1 files changed, 2 insertions(+), 7 deletions(-)

diff --git a/mm/vmalloc.c b/mm/vmalloc.c
index f8189a4..56b2469 100644
--- a/mm/vmalloc.c
+++ b/mm/vmalloc.c
@@ -159,20 +159,15 @@ static int vmap_page_range_noflush(unsigned long start, unsigned long end,
pgd_t *pgd;
unsigned long next;
unsigned long addr = start;
- int err = 0;
int nr = 0;

BUG_ON(addr >= end);
pgd = pgd_offset_k(addr);
do {
next = pgd_addr_end(addr, end);
- err = vmap_pud_range(pgd, addr, next, prot, pages, &nr);
- if (err)
- break;
+ if (vmap_pud_range(pgd, addr, next, prot, pages, &nr))
+ return -ENOMEM;
} while (pgd++, addr = next, addr != end);
-
- if (unlikely(err))
- return err;
return nr;
}

2009-07-08 00:19:37

by Tejun Heo

Subject: Re: [PATCH] vmalloc.c: fix double error checking

Figo.zhang wrote:
> it is no need for double error checking.
>
> Signed-off-by: Figo.zhang <[email protected]>
> - err = vmap_pud_range(pgd, addr, next, prot, pages, &nr);
> - if (err)
> - break;
> + if (vmap_pud_range(pgd, addr, next, prot, pages, &nr))
> + return -ENOMEM;

Wouldn't it be better to keep the error return value?

Thanks.

--
tejun

2009-07-08 00:40:28

by Figo.zhang

Subject: Re: [PATCH] vmalloc.c: fix double error checking

On Wed, 2009-07-08 at 09:19 +0900, Tejun Heo wrote:
> Figo.zhang wrote:
> > it is no need for double error checking.
> >
> > Signed-off-by: Figo.zhang <[email protected]>
> > - err = vmap_pud_range(pgd, addr, next, prot, pages, &nr);
> > - if (err)
> > - break;
> > + if (vmap_pud_range(pgd, addr, next, prot, pages, &nr))
> > + return -ENOMEM;
>
> Wouldn't it be better to keep the error return value?

see the vmap_pud_range(), if suceed return 0, if error return "-ENOMEM",
so it had better use clearly logic if error directly return "-EOMEM",
avoid double checking.

Best,
Figo.zhang


>
> Thanks.
>
> --
> tejun

2009-07-08 02:02:23

by Tejun Heo

Subject: Re: [PATCH] vmalloc.c: fix double error checking

Figo.zhang wrote:
> On Wed, 2009-07-08 at 09:19 +0900, Tejun Heo wrote:
>> Figo.zhang wrote:
>>> it is no need for double error checking.
>>>
>>> Signed-off-by: Figo.zhang <[email protected]>
>>> - err = vmap_pud_range(pgd, addr, next, prot, pages, &nr);
>>> - if (err)
>>> - break;
>>> + if (vmap_pud_range(pgd, addr, next, prot, pages, &nr))
>>> + return -ENOMEM;
>> Wouldn't it be better to keep the error return value?
>
> see the vmap_pud_range(), if suceed return 0, if error return "-ENOMEM",
> so it had better use clearly logic if error directly return "-EOMEM",
> avoid double checking.

Yeah, I saw that but I still think it would be (minutely) better to
just pass through the error value. It isn't exclusive with double
error checking, just do err = vmap_pud_range(); if (err) return err;

Thanks.

--
tejun

2009-07-08 02:12:13

by Figo.zhang

Subject: [PATCH v2] vmalloc.c: fix double error checking

it is no need for double error checking.

in v2, if vmap_pud_ranger() error, directly return err.

Signed-off-by: Figo.zhang <[email protected]>
---
mm/vmalloc.c | 4 +---
1 files changed, 1 insertions(+), 3 deletions(-)

diff --git a/mm/vmalloc.c b/mm/vmalloc.c
index f8189a4..ed6b4b9 100644
--- a/mm/vmalloc.c
+++ b/mm/vmalloc.c
@@ -168,11 +168,9 @@ static int vmap_page_range_noflush(unsigned long
start, unsigned long end,
next = pgd_addr_end(addr, end);
err = vmap_pud_range(pgd, addr, next, prot, pages, &nr);
if (err)
- break;
+ return err;
} while (pgd++, addr = next, addr != end);

- if (unlikely(err))
- return err;
return nr;
}

2009-07-08 02:17:02

by Tejun Heo

Subject: Re: [PATCH v2] vmalloc.c: fix double error checking

Figo.zhang wrote:
> it is no need for double error checking.
>
> in v2, if vmap_pud_ranger() error, directly return err.
>
> Signed-off-by: Figo.zhang <[email protected]>

Acked-by: Tejun Heo <[email protected]>

Thanks.

--
tejun