kswapd and MM overload fix

kswapd and MM overload fix

[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).

--- 2.4.5aa3/mm/vmscan.c.~1~	Sat May 26 04:03:50 2001
+++ 2.4.5aa3/mm/vmscan.c	Tue Jun  5 03:41:06 2001
@@ -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

Re: kswapd and MM overload fix

[Linus Torvalds]

On Wed, 6 Jun 2001, Andrea Arcangeli wrote:
>
> 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

Re: kswapd and MM overload fix

[Andrea Arcangeli]

On Tue, Jun 05, 2001 at 04:16:57PM -0700, Linus Torvalds wrote:
> 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

Re: kswapd and MM overload fix

[Linus Torvalds]

On Wed, 6 Jun 2001, Andrea Arcangeli wrote:
>
> 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

Re: kswapd and MM overload fix

[Xavier Bestel]

On 05 Jun 2001 16:51:13 -0700, Linus Torvalds wrote:
> 
> 
> On Wed, 6 Jun 2001, Andrea Arcangeli wrote:
> >
> > 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.

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

Xav

Re: kswapd and MM overload fix

[Roger Larsson]

On Wednesday66 0666 6 June 2001 01:16, Linus Torvalds wrote:
> On Wed, 6 Jun 2001, Andrea Arcangeli wrote:
> > 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?
>  

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

-- 
Roger Larsson
Skellefteå
Sweden