[patch] zone_watermark_ok() rework

[patch] zone_watermark_ok() rework

[Coywolf Qi Hunt]

Hello,

In zone_watermark_ok(), the original algorithm seems not logical. This is a
rework. Comments?

		Coywolf


Signed-off-by: Coywolf Qi Hunt <coywolf@sosdg.org>
---

 page_alloc.c |   15 +++++----------
 1 file changed, 5 insertions(+), 10 deletions(-)

--- 2.6.14-rc2-mm1/mm/page_alloc.c~orig	2005-09-29 20:37:07.000000000 +0800
+++ 2.6.14-rc2-mm1/mm/page_alloc.c	2005-10-04 01:19:31.000000000 +0800
@@ -772,7 +772,7 @@
 int zone_watermark_ok(struct zone *z, int order, unsigned long mark,
 		      int classzone_idx, int can_try_harder, int gfp_high)
 {
-	/* free_pages my go negative - that's OK */
+	/* free_pages may go negative - that's OK */
 	long min = mark, free_pages = z->free_pages - (1 << order) + 1;
 	int o;
 
@@ -783,17 +783,12 @@
 
 	if (free_pages <= min + z->lowmem_reserve[classzone_idx])
 		return 0;
-	for (o = 0; o < order; o++) {
-		/* At the next order, this order's pages become unavailable */
-		free_pages -= z->free_area[o].nr_free << o;
 
-		/* Require fewer higher order pages to be free */
-		min >>= 1;
-
-		if (free_pages <= min)
-			return 0;
+	for (o = order; o < MAX_ORDER; o++) {
+		if (z->free_area[o].nr_free)
+			return 1;
 	}
-	return 1;
+	return 0;
 }
 
 static inline int

Re: [patch] zone_watermark_ok() rework

[Nick Piggin]

On Tue, 2005-10-04 at 01:41 +0800, Coywolf Qi Hunt wrote:
> Hello,
> 
> In zone_watermark_ok(), the original algorithm seems not logical. This is a
> rework. Comments?
> 
> 		Coywolf
> 
> 
> Signed-off-by: Coywolf Qi Hunt <coywolf@sosdg.org>
> ---
> 
>  page_alloc.c |   15 +++++----------
>  1 file changed, 5 insertions(+), 10 deletions(-)
> 
> --- 2.6.14-rc2-mm1/mm/page_alloc.c~orig	2005-09-29 20:37:07.000000000 +0800
> +++ 2.6.14-rc2-mm1/mm/page_alloc.c	2005-10-04 01:19:31.000000000 +0800
> @@ -772,7 +772,7 @@
>  int zone_watermark_ok(struct zone *z, int order, unsigned long mark,
>  		      int classzone_idx, int can_try_harder, int gfp_high)
>  {
> -	/* free_pages my go negative - that's OK */
> +	/* free_pages may go negative - that's OK */
>  	long min = mark, free_pages = z->free_pages - (1 << order) + 1;
>  	int o;
>  

Typo. Thanks.

> @@ -783,17 +783,12 @@
>  
>  	if (free_pages <= min + z->lowmem_reserve[classzone_idx])
>  		return 0;
> -	for (o = 0; o < order; o++) {
> -		/* At the next order, this order's pages become unavailable */
> -		free_pages -= z->free_area[o].nr_free << o;
>  
> -		/* Require fewer higher order pages to be free */
> -		min >>= 1;
> -
> -		if (free_pages <= min)
> -			return 0;
> +	for (o = order; o < MAX_ORDER; o++) {
> +		if (z->free_area[o].nr_free)
> +			return 1;
>  	}
> -	return 1;
> +	return 0;
>  }
>  
>  static inline int

Well the original algorithm's intention is to include some watermarks
for higher order pages for eg. GFP_ATOMIC and PF_MEMALLOC. To that
goal I believe the algorithm is logical?

Yours is correct if our intention was simply to ensure _any_ higher
order page is available, however it is also redundant because rmqueue
will catch that for us. However, that is not the intention :)

Any reasons why it should be changed?

Thanks,
Nick

-- 
SUSE Labs, Novell Inc.



Send instant messages to your online friends http://au.messenger.yahoo.com