Skip to contentSkip to conversation

kswapd and MM overload fix

PATCH Andrea Arcangeli 4 participants 6 messages Jun 5, 2001 – Jun 6, 2001

Subsystems (3)
Memory management MGLRU MM reclaim
lore
Share & tools
lore

Andrea Arcangeli

author
#0 lore raw

Anybody running on a machine with some zone empty (<16Mbyte boxes (PDAs), <1G x86 with highmem enabled kernel or an arch with an iommu like alpha) probably noticed that the MM was unusable on those hardware configurations (as I also incidentally mentioned a few times on l-k in the last months).

Yesterday I checked and here it is the fix (included in 2.4.5aa3).

mm/vmscan.c renamed from mm/vmscan.c.~1~ +2 -0
@@ -791,6 +788,8 @@
 		for(i = 0; i < MAX_NR_ZONES; i++) {
 			int zone_shortage;
 			zone_t *zone = pgdat->node_zones+ i;
+			if (!zone->size)
+				continue;
 
 			zone_shortage = zone->pages_high;
 			zone_shortage -= zone->inactive_dirty_pages;

Andrea

#1 lore raw
Quoted 8 lines · Andrea Arcangeli

Anybody running on a machine with some zone empty (<16Mbyte boxes (PDAs), <1G x86 with highmem enabled kernel or an arch with an iommu like alpha) probably noticed that the MM was unusable on those hardware configurations (as I also incidentally mentioned a few times on l-k in the last months).

Yesterday I checked and here it is the fix (included in 2.4.5aa3).

I think the real problem is that zone->pages_{min,low,high} aren't initialized at all for empty zones. If they were initialized (to zero, of course), the shortage calculations would have worked automatically.

Using uninitialized variables is always bad. Your patch is certainly fine, but I think we should also make the init code clear the zone data for empty zones so that these kinds of "use uninitialized stuff" things cannot happen. No?

Linus

#2 lore raw
Quoted 9 lines · Linus Torvalds
Quoted 7 lines · Andrea Arcangeli

Anybody running on a machine with some zone empty (<16Mbyte boxes (PDAs), <1G x86 with highmem enabled kernel or an arch with an iommu like alpha) probably noticed that the MM was unusable on those hardware configurations (as I also incidentally mentioned a few times on l-k in the last months).

Yesterday I checked and here it is the fix (included in 2.4.5aa3).

Quoted 9 lines

I think the real problem is that zone->pages_{min,low,high} aren't initialized at all for empty zones. If they were initialized (to zero, of course), the shortage calculations would have worked automatically.

Using uninitialized variables is always bad. Your patch is certainly fine, but I think we should also make the init code clear the zone data for empty zones so that these kinds of "use uninitialized stuff" things cannot happen. No?

Lets see - that zone will have no free nor inactive pages

In page_alloc.c:254 function __alloc_pages_limit where water_mark will be zero too...

              if (z->free_pages + z->inactive_clean_pages >= water_mark) {    
we will attempt a lot of interesting/unnecessary stuff.
But it should be caught by the test a few lines up...
                if (!z->size)
                        BUG();

In page_alloc.c:331 (function __alloc_pages)

                if (z->free_pages >= z->pages_low) {
                        page = rmqueue(z, order);
                        if (page)
                                return page;

Hmm... a lot more than first meets the eye.
Note: >= matches < in another place, removing the = will leave the mm stuck...

/RogerL

Signature · 3 lines
Roger Larsson
Skellefteå
Sweden
author
#3 lore raw
In reply to Andrea Arcangeli
Quoted 3 lines · Linus Torvalds

but I think we should also make the init code clear the zone data for empty zones so that these kinds of "use uninitialized stuff" things cannot happen. No?

that would fix it too indeed, OTOH I think changing the empty zone handling in the kernel is beyond the scope of the bugfix (grep for zone->size in mm/*.c :). Certainly making empty zones transparent to the vm sounds much cleaner from a mm/*.c point of view but it may be faster to skip the block with a branch instead of always computing it.

Andrea

#4 lore raw
Quoted 6 lines · Andrea Arcangeli

that would fix it too indeed, OTOH I think changing the empty zone handling in the kernel is beyond the scope of the bugfix (grep for zone->size in mm/*.c :). Certainly making empty zones transparent to the vm sounds much cleaner from a mm/*.c point of view but it may be faster to skip the block with a branch instead of always computing it.

Agreed. I'd like to do both. Having uninitialized random stuff around and depending on jumping over code that might access it sounds like really bad practice.

Linus

#5 lore raw
Quoted 10 lines · Linus Torvalds
Quoted 6 lines · Andrea Arcangeli

that would fix it too indeed, OTOH I think changing the empty zone handling in the kernel is beyond the scope of the bugfix (grep for zone->size in mm/*.c :). Certainly making empty zones transparent to the vm sounds much cleaner from a mm/*.c point of view but it may be faster to skip the block with a branch instead of always computing it.

Quoted 3 lines

Agreed. I'd like to do both. Having uninitialized random stuff around and depending on jumping over code that might access it sounds like really bad practice.

Why not explicitely initializing it to weird values, to catch code which should jump over it (but doesn't) ?

Xav