[PATCH] vmalloc.c: fix double error checking

[PATCH] vmalloc.c: fix double error checking

[Figo.zhang]

it is no need for double error checking.

Signed-off-by: Figo.zhang <figo1802@gmail.com>
--- 
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;
 }
 

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

[Tejun Heo]

Figo.zhang wrote:
> it is no need for double error checking.
> 
> Signed-off-by: Figo.zhang <figo1802@gmail.com>
> -		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

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

[Figo.zhang]

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 <figo1802@gmail.com>
> > -		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

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

[Tejun Heo]

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 <figo1802@gmail.com>
>>> -		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

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

[Figo.zhang]

it is no need for double error checking.

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

Signed-off-by: Figo.zhang <figo1802@gmail.com>
---
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;
 }
 

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

[Tejun Heo]

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 <figo1802@gmail.com>

Acked-by: Tejun Heo <tj@kernel.org>

Thanks.

-- 
tejun