Re: refill_inactive_zone question

[Marcelo Tosatti <marcelo.tosatti@cyclades.com>]

On Fri, Aug 27, 2004 at 10:17:35PM +0100, Hugh Dickins wrote:
> On Fri, 27 Aug 2004, Marcelo Tosatti wrote:
> > 
> > Is it possible to have AnonPages without a mapping to them? I dont think so.
> 
> It was impossible, but my "remove page_map_lock" patches had to change that.
> 
> > Can't the check "if (total_swap_pages == 0 && PageAnon(page))" be moved
> > inside "if (page_mapped(page))" ? 
> 
> Yes: it's like that in -mm, and I believe now in Linus' bk tree too.
> 
> Hugh

Hi Hugh, 

Oh thanks! I see that. So you just dropped the bit spinlocked and changed
mapcount to an atomic variable, right?  Cool. Do you have any numbers on 
big SMP systems for that change? 

Talking about refill_inactive_zone(), the next stage of the algorithm:

        while (!list_empty(&l_active)) {
                page = lru_to_page(&l_active);
                prefetchw_prev_lru_page(page, &l_active, flags);
                if (TestSetPageLRU(page))
                        BUG();
                BUG_ON(!PageActive(page));
                list_move(&page->lru, &zone->active_list);
                pgmoved++;
                if (!pagevec_add(&pvec, page)) {
                        zone->nr_active += pgmoved;
                        pgmoved = 0;
                        spin_unlock_irq(&zone->lru_lock);
                        __pagevec_release(&pvec);
                        spin_lock_irq(&zone->lru_lock);
                }
        }

Several things:

1) __pagevec_release does lru_add_drain(), which moves pages in the deferred lru 
queues (active & inactive) to the actual lists. But at that point in refill_inactive()
thats not a direct benefit (we already moved scanned the inactive list at the beginning
of the algo). So, we could remove that lru_add_drain from refill_inactive_zone->__pagevec_release.

The bad part of doing unecessary lru_add_drain's is that we minimize the chance from 
the queue growing big. And the queues growing big means we batch the list moving, better
cache locality. 

Is there any good reason for doing that lru_add_drain from the refill_inactive_zone()
callchain?


2) before calling __pagevec_release we drop the zone lock, to lock it again at 
__pagevec_release->release_pages. Acquiring locks is usually more expensive 
then it seems (thanks Paul McKenney!), release_pages handles pagevec's containing pages
 from different zones, but we know all pages on this pagevec are on the same zone.
Couldnt it all be under the zone lock?

3) What happens if that __pagevec_release frees one or more pages page (and deletes it/them 
from the LRU list)?  We will still count those pages in "pgmoved" which will then be 
accounted in zone->nr_active. Whoops.

Hope I'm full of shit.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"aart@kvack.org"> aart@kvack.org </a>