Skip to contentSkip to conversation

swsusp: cleanup -- use list_for_each in head_of_free_region

PATCH Pavel Machek 2 participants 4 messages May 28, 2002

Subsystems (2)
Memory management Page allocator
lore
Share & tools
lore

Pavel Machek

author
#0 lore raw

Hi!

This cleans up is_head_of_free_region, thanks to William Lee Irwin III <[email protected]>. Please apply,

Pavel

mm/page_alloc.c renamed from /mm/page_alloc.c +19 -30
@@ -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 */
Signature · 2 lines
(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
#1 lore raw
Quoted 2 lines · Pavel Machek

This cleans up is_head_of_free_region, thanks to William Lee Irwin III <[email protected]>. 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

author
#2 lore raw
In reply to Pavel Machek

Hi!

Quoted 5 lines
Quoted 2 lines

This cleans up is_head_of_free_region, thanks to William Lee Irwin III <[email protected]>. 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

Signature · 2 lines
Casualities in World Trade Center: ~3k dead inside the building,
cryptography in U.S.A. and free speech in Czech Republic.
author
#3 lore raw

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

In reply to Pavel Machek

Hi!

Quoted 2 lines

This cleans up is_head_of_free_region, thanks to William Lee Irwin III <[email protected]>. 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

Quoted 67 lines
mm/page_alloc.c renamed from /mm/page_alloc.c +19 -30
@@ -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 */
Signature · 2 lines
(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