swsusp: cleanup -- use list_for_each in head_of_free_region

swsusp: cleanup -- use list_for_each in head_of_free_region

[Pavel Machek]

Hi!

This cleans up is_head_of_free_region, thanks to William Lee Irwin III
<wli@holomorphy.com>. Please apply,

									Pavel

--- linux-swsusp.test//mm/page_alloc.c	Sun May 26 19:32:05 2002
+++ linux-swsusp/mm/page_alloc.c	Tue May 28 21:19:26 2002
@@ -249,42 +249,31 @@
 }
 
 #ifdef CONFIG_SOFTWARE_SUSPEND
-int is_head_of_free_region(struct page *p)
+int is_head_of_free_region(struct page *page)
 {
-	pg_data_t *pgdat = pgdat_list;
-	unsigned type;
-	unsigned long flags;
+        zone_t *zone, *node_zones = pgdat_list->node_zones;
+        unsigned long flags;
 
-	for (type=0;type < MAX_NR_ZONES; type++) {
-		zone_t *zone = pgdat->node_zones + type;
-		int order = MAX_ORDER - 1;
-		free_area_t *area;
-		struct list_head *head, *curr;
-		spin_lock_irqsave(&zone->lock, flags);	/* Should not matter as we need quiescent system for suspend anyway, but... */
+        for (zone = node_zones; zone - node_zones < MAX_NR_ZONES; ++zone) {
+                int order;
+                list_t *curr;
 
-		do {
-			area = zone->free_area + order;
-			head = &area->free_list;
-			curr = head;
-
-			for(;;) {
-				if(!curr) {
-//					printk("FIXME: this should not happen but it does!!!");
+                /*
+                 * Should not matter as we need quiescent system for
+                 * suspend anyway, but...
+                 */
+                spin_lock_irqsave(&zone->lock, flags);
+                for (order = MAX_ORDER - 1; order >= 0; --order)
+                        list_for_each(curr, &zone->free_area[order].free_list) {
+				if (!curr)
 					break;
-				}
-				if(p != memlist_entry(curr, struct page, list)) {
-					curr = memlist_next(curr);
-					if (curr == head)
-						break;
-					continue;
-				}
-				return 1 << order;
+                                if (page == list_entry(curr, struct page, list))
+                                        return 1 << order;
 			}
-		} while(order--);
-		spin_unlock_irqrestore(&zone->lock, flags);
+                spin_unlock_irqrestore(&zone->lock, flags);
 
-	}
-	return 0;
+        }
+        return 0;
 }
 #endif /* CONFIG_SOFTWARE_SUSPEND */
 



-- 
(about SSSCA) "I don't say this lightly.  However, I really think that the U.S.
no longer is classifiable as a democracy, but rather as a plutocracy." --hpa

Re: swsusp: cleanup -- use list_for_each in head_of_free_region

[William Lee Irwin III]

On Tue, May 28, 2002 at 09:33:57PM +0200, Pavel Machek wrote:
> This cleans up is_head_of_free_region, thanks to William Lee Irwin III
> <wli@holomorphy.com>. Please apply,

Whoa! Mike Galbraith noticed a return without dropping the lock in there,
it's really easy to fix (of course), though.


Cheers,
Bill

Re: swsusp: cleanup -- use list_for_each in head_of_free_region

[Pavel Machek]

Hi!

> > This cleans up is_head_of_free_region, thanks to William Lee Irwin III
> > <wli@holomorphy.com>. Please apply,
> 
> Whoa! Mike Galbraith noticed a return without dropping the lock in there,
> it's really easy to fix (of course), though.

How is that possible? I actually tested that code! Fixed now...
									Pavel

-- 
Casualities in World Trade Center: ~3k dead inside the building,
cryptography in U.S.A. and free speech in Czech Republic.

DO NOT APPLY Re: swsusp: cleanup -- use list_for_each in head_of_free_region

[Pavel Machek]

Hi!

> This cleans up is_head_of_free_region, thanks to William Lee Irwin III
> <wli@holomorphy.com>. Please apply,

It forgets to spin_unlock when it sees free page. Sorry about
that. Just forget this patch, I'll get you better variant in
2.5.19. Its only cosmetics, anyway.
								Pavel


> --- linux-swsusp.test//mm/page_alloc.c	Sun May 26 19:32:05 2002
> +++ linux-swsusp/mm/page_alloc.c	Tue May 28 21:19:26 2002
> @@ -249,42 +249,31 @@
>  }
>  
>  #ifdef CONFIG_SOFTWARE_SUSPEND
> -int is_head_of_free_region(struct page *p)
> +int is_head_of_free_region(struct page *page)
>  {
> -	pg_data_t *pgdat = pgdat_list;
> -	unsigned type;
> -	unsigned long flags;
> +        zone_t *zone, *node_zones = pgdat_list->node_zones;
> +        unsigned long flags;
>  
> -	for (type=0;type < MAX_NR_ZONES; type++) {
> -		zone_t *zone = pgdat->node_zones + type;
> -		int order = MAX_ORDER - 1;
> -		free_area_t *area;
> -		struct list_head *head, *curr;
> -		spin_lock_irqsave(&zone->lock, flags);	/* Should not matter as we need quiescent system for suspend anyway, but... */
> +        for (zone = node_zones; zone - node_zones < MAX_NR_ZONES; ++zone) {
> +                int order;
> +                list_t *curr;
>  
> -		do {
> -			area = zone->free_area + order;
> -			head = &area->free_list;
> -			curr = head;
> -
> -			for(;;) {
> -				if(!curr) {
> -//					printk("FIXME: this should not happen but it does!!!");
> +                /*
> +                 * Should not matter as we need quiescent system for
> +                 * suspend anyway, but...
> +                 */
> +                spin_lock_irqsave(&zone->lock, flags);
> +                for (order = MAX_ORDER - 1; order >= 0; --order)
> +                        list_for_each(curr, &zone->free_area[order].free_list) {
> +				if (!curr)
>  					break;
> -				}
> -				if(p != memlist_entry(curr, struct page, list)) {
> -					curr = memlist_next(curr);
> -					if (curr == head)
> -						break;
> -					continue;
> -				}
> -				return 1 << order;
> +                                if (page == list_entry(curr, struct page, list))
> +                                        return 1 << order;
>  			}
> -		} while(order--);
> -		spin_unlock_irqrestore(&zone->lock, flags);
> +                spin_unlock_irqrestore(&zone->lock, flags);
>  
> -	}
> -	return 0;
> +        }
> +        return 0;
>  }
>  #endif /* CONFIG_SOFTWARE_SUSPEND */
>  
> 
> 
> 

-- 
(about SSSCA) "I don't say this lightly.  However, I really think that the U.S.
no longer is classifiable as a democracy, but rather as a plutocracy." --hpa