Re: [PATCH] total_free_shortage() using zone_free_shortage()

[Marcelo Tosatti]

On Tue, 7 Aug 2001, Linus Torvalds wrote:

> 
> On Mon, 6 Aug 2001, Marcelo Tosatti wrote:
> >
> > The following patch changes total_free_shortage() to use
> > zone_free_shortage() to calculate the sum of perzone free shortages.
> 
> Marcelo, the patch looks ok per se, but I think the real _problem_ is that
> you did the same mistake in do_page_launder() as you originally did in the
> VM scanning.
> 
> The code should NOT use
> 
> 	if (zone && !zone_free_shortage(page->zone))
> 		.. don't touch ..
> 
> because that is completely nonsensical anyway.
> 
> As with the VM scanning code, it should do
> 
> 	if (zone_free_plenty(page->zone))
> 		.. don't touch ..
> 
> and you should make
> 
> 	zone_free_plenty(zone)
> 	{
> 		return zone->free_pages + zone->inactive_clean_pages > zone->max_free_pages;
> 	}
> 
> and
> 
> 	zone_free_shortage(zone)
> 	{
> 		return zone->free_pages + zone->inactive_clean_pages < zone->low_free_pages;
> 	}
> 
> Note the anti-hysteresis by using max_free_pages vs min_free_pages.
> 
> This will clean up the code (remove those silly "zone as a boolean"), and
> I bet it will behave better too with less of a spike in behaviour around
> "max_free_pages".

I agree that not writing out pages for zones which are not under shortage
does make sense in theory. If the solution for a constant free shortage is
to clean dirty pages, we want to keep writing pages (to some extent) even
if we have left the free shortage. It avoids spikes, as you mentioned.

But there is a problem: In case we have a specific zone under critical
shortage, we have to fix that shortage as fast as possible instead
blocking on IO (IO for the page_launder case) for zones which are not
under critical conditions.

I'm thinking of a way to fix both (spikes on page writeout and zone
critical shortages) problems, but IMO you're suggestion is going to
hurt the latter.