Skip to contentSkip to conversation

New zone ZONE_EASY_RECLAIM take 4. (change build_zonelists)[3/8]

PATCH In review Yasunori Goto 2 participants 3 messages Dec 20, 2005 – Jan 5, 2006

Subsystems (3)
Memory management MM core Page allocator
lore
Share & tools
lore

Yasunori Goto

author
#0 lore raw

PATCH 1/1 New zone ZONE_EASY_RECLAIM take 4. (change build_zonelists)[3/8]

This is changing build_zonelists for new zone.

__GFP_xxxs are flag for requires of page allocation which zone is prefered. But, it is used as an index number for zonelists[] too. But after my patch, __GFP_xxx might be set at same time. So, last set bit number of __GFP is recognized for zonelists' index by this patch.

take3->take4:
take 3's modification was still wrong.
__GFP_EASY_RECLAIM is 0x04 on i386, so fls(__GFP_EASY_RECLAIM) is 3. zone 3 is ZONE_HIGHMEM, not ZONE_EASY_RECLAIM.
So, I rearranged __GFP_XXX flags (see: define gfp_easy_relcaim patch) and fls() is used at highest_zone() again.

take2 -> take 3:
This patch is modified take 3 to avoid panic on i386. __GFP_DMA32 is 0 for i386. So, ZONE_DMA32 is selected if zone_bits is 0 which means Zone_normal. Zone_DMA32 is not allocated on i386, so kernel paniced by no normal memory.
In this patch, even if zone_bits is 0 adn __GFP_DMA32 is 0, Zone_Normal is selected.

Signed-off-by Yasunori Goto <[email protected]>
mm/page_alloc.c +11 -9
Index: zone_reclaim/mm/page_alloc.c
===================================================================
@@ -1585,14 +1585,11 @@ static int __init build_zonelists_node(p
 {
 	struct zone *zone;
 
-	BUG_ON(zone_type > ZONE_HIGHMEM);
+	BUG_ON(zone_type > ZONE_EASY_RECLAIM);
 
 	do {
 		zone = pgdat->node_zones + zone_type;
 		if (populated_zone(zone)) {
-#ifndef CONFIG_HIGHMEM
-			BUG_ON(zone_type > ZONE_NORMAL);
-#endif
 			zonelist->zones[nr_zones++] = zone;
 			check_highest_zone(zone_type);
 		}

@@ -1605,12 +1602,17 @@ static int __init build_zonelists_node(p
 static inline int highest_zone(int zone_bits)
 {
 	int res = ZONE_NORMAL;
-	if (zone_bits & (__force int)__GFP_HIGHMEM)
-		res = ZONE_HIGHMEM;
-	if (zone_bits & (__force int)__GFP_DMA32)
-		res = ZONE_DMA32;
-	if (zone_bits & (__force int)__GFP_DMA)
+
+	if (zone_bits == fls((__force int)__GFP_DMA))
 		res = ZONE_DMA;
+	if (zone_bits == fls((__force int)__GFP_DMA32) &&
+	    (__force int)__GFP_DMA32 == 0x02)
+		res = ZONE_DMA32;
+	if (zone_bits == fls((__force int)__GFP_HIGHMEM))
+		res = ZONE_HIGHMEM;
+	if (zone_bits == fls((__force int)__GFP_EASY_RECLAIM))
+		res = ZONE_EASY_RECLAIM;
+
 	return res;
 }
include/linux/gfp.h +1 -1
Index: zone_reclaim/include/linux/gfp.h
===================================================================
@@ -81,7 +81,7 @@ struct vm_area_struct;
 
 static inline int gfp_zone(gfp_t gfp)
 {
-	int zone = GFP_ZONEMASK & (__force int) gfp;
+	int zone = fls(GFP_ZONEMASK & (__force int) gfp);
 	BUG_ON(zone >= GFP_ZONETYPES);
 	return zone;
 }
Signature · 8 lines
Yasunori Goto 


--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
#1 lore raw

mm/page_alloc.c static int __init build_zonelists_node(p · line 1588

Quoted 2 lines
mm/page_alloc.c +1 -1
-	BUG_ON(zone_type > ZONE_HIGHMEM);
+	BUG_ON(zone_type > ZONE_EASY_RECLAIM);

It might be nice to check ifndef CONFIG_HIGHMEM that the zone isn't particularly ZONE_HIGHMEM.

Quoted 9 lines
mm/page_alloc.c +2 -5
 	int res = ZONE_NORMAL;
-	if (zone_bits & (__force int)__GFP_HIGHMEM)
-		res = ZONE_HIGHMEM;
-	if (zone_bits & (__force int)__GFP_DMA32)
-		res = ZONE_DMA32;
-	if (zone_bits & (__force int)__GFP_DMA)
+
+	if (zone_bits == fls((__force int)__GFP_DMA))
 		res = ZONE_DMA;

mm/page_alloc.c static int __init build_zonelists_node(p · line 1617

Quoted 10 lines
mm/page_alloc.c +8 -0
+	if (zone_bits == fls((__force int)__GFP_DMA32) &&
+	    (__force int)__GFP_DMA32 == 0x02)
+		res = ZONE_DMA32;
+	if (zone_bits == fls((__force int)__GFP_HIGHMEM))
+		res = ZONE_HIGHMEM;
+	if (zone_bits == fls((__force int)__GFP_EASY_RECLAIM))
+		res = ZONE_EASY_RECLAIM;
+
 	return res;
 }

It is incredibly silly to check a constant for a value. When it is zero instead of 2 the first part of the statement will be false anyway.

Which reminds me. Why are we using fls again? I don't see why we aren't just (zone_bits & value) the types. It seems much easier to understand that way.

Quoted 7 lines

Index: zone_reclaim/include/linux/gfp.h
===================================================================

include/linux/gfp.h struct vm_area_struct; · line 87

Quoted 8 lines
include/linux/gfp.h +1 -1
@@ -81,7 +81,7 @@ struct vm_area_struct;
 
 static inline int gfp_zone(gfp_t gfp)
 {
-	int zone = GFP_ZONEMASK & (__force int) gfp;
+	int zone = fls(GFP_ZONEMASK & (__force int) gfp);
 	BUG_ON(zone >= GFP_ZONETYPES);
 	return zone;
 }

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

author
#2 lore raw
Quoted 5 lines

mm/page_alloc.c static int __init build_zonelists_node(p · line 1588

Quoted 2 lines
mm/page_alloc.c +1 -1
-	BUG_ON(zone_type > ZONE_HIGHMEM);
+	BUG_ON(zone_type > ZONE_EASY_RECLAIM);

It might be nice to check ifndef CONFIG_HIGHMEM that the zone isn't particularly ZONE_HIGHMEM.

Hmm. I was a bit lazy :-(.

Quoted 17 lines
Quoted 17 lines
mm/page_alloc.c +10 -5
 	int res = ZONE_NORMAL;
-	if (zone_bits & (__force int)__GFP_HIGHMEM)
-		res = ZONE_HIGHMEM;
-	if (zone_bits & (__force int)__GFP_DMA32)
-		res = ZONE_DMA32;
-	if (zone_bits & (__force int)__GFP_DMA)
+
+	if (zone_bits == fls((__force int)__GFP_DMA))
 		res = ZONE_DMA;
+	if (zone_bits == fls((__force int)__GFP_DMA32) &&
+	    (__force int)__GFP_DMA32 == 0x02)
+		res = ZONE_DMA32;
+	if (zone_bits == fls((__force int)__GFP_HIGHMEM))
+		res = ZONE_HIGHMEM;
+	if (zone_bits == fls((__force int)__GFP_EASY_RECLAIM))
+		res = ZONE_EASY_RECLAIM;
+
Quoted 8 lines
Quoted 2 lines

return res;
}

It is incredibly silly to check a constant for a value. When it is zero instead of 2 the first part of the statement will be false anyway. Which reminds me. Why are we using fls again? I don't see why we aren't just (zone_bits & value) the types. It seems much easier to understand that way.

Ahhh. I might be still confused around it. :-(
Its cause came from the value of __GFP_DMA32 which might be 0, 1, or 2. This was that I tried to solve making zonelists for all of them. But, something looks redundant.....

Thanks.

Quoted 18 lines
Quoted 16 lines

Index: zone_reclaim/include/linux/gfp.h
===================================================================

include/linux/gfp.h +1 -1
@@ -81,7 +81,7 @@ struct vm_area_struct;
 
 static inline int gfp_zone(gfp_t gfp)
 {
-	int zone = GFP_ZONEMASK & (__force int) gfp;
+	int zone = fls(GFP_ZONEMASK & (__force int) gfp);
 	BUG_ON(zone >= GFP_ZONETYPES);
 	return zone;
 }
Signature · 8 lines
Yasunori Goto 


--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>