Re: [PATCH] 1/4: rework alloc_pages

Nick Piggin
2004-08-06T06:05:26Z

Andrew Morton wrote:
> Nick Piggin <nickpiggin@yahoo.com.au> wrote:
>
>>Andrew Morton wrote:
>>
>>>Nick Piggin <nickpiggin@yahoo.com.au> wrote:
>>>
>>>
>>>>Previously the ->protection[] logic was broken. It was difficult to follow
>>>>and basically didn't use the asynch reclaim watermarks properly.
>>>
>>>
>>>eh?
>>>
>>>Broken how?
>>>
>>
>>min = (1<<order) + z->protection[alloc_type];
>>
>>This value is used both as the condition for waking kswapd, and
>>whether or not to enter synch reclaim.
>>
>>What should happen is kswapd gets woken at pages_low, and synch
>>reclaim is started at pages_min.
>
>
> Are you aware of this:
>
> void wakeup_kswapd(struct zone *zone)
> {
> if (zone->free_pages > zone->pages_low)
> return;
>
> ?
>

Err, yes?

>
>>pages_low + protection and pages_min + protection, etc.
>
>
> Nick, sorry, but I shouldn't have to expend these many braincells
> decrypting your work. Please: much better explanations, more testing
> results. This stuff is fiddly, sensitive and has a habit of blowing up in
> our faces weeks later. We need to be cautious. The barriers are higher
> nowadays.
>
>

OK previously, in a nutshell:

for_each_zone(z) {
if (z->free_pages < z->protection)
continue;
else
goto got_pg;
}

for_each_zone(z)
wakeup_kswapd(z);

for_each_zone(z) {
if (z->free_pages < z->protection)
continue;
else
goto got_pg;
}

try_to_free_pages();

try again;

After my patch:
for_each_zone(z) {
if (z->free_pages < z->pages_low + z->protection)
continue;
else
goto got_pg;
}

for_each_zone(z)
wakeup_kswapd(z);

for_each_zone(z) {
if (z->free_pages < z->pages_min + z->protection)
continue;
else
goto got_pg;
}

try_to_free_pages();

try again;

Ie, we have the (pages_low - pages_min) buffer after waking kswapd
before entering synch reclaim. Previously there was no buffer. I thought
this was the point of background reclaim. I don't know if I can explain
it any better than that sorry.