Messages in this thread | | | Date | Tue, 21 Mar 2006 23:22:31 +1100 | From | Nick Piggin <> | Subject | Re: [PATCH][8/8] mm: lru interface change |
| |
Stone Wang wrote: > Add Wired list to LRU. > Add PG_Wired bit to page. >
I may have missed something very trivial, but... why are they on a list at all if they don't get scanned?
> diff -urN linux-2.6.15.orig/mm/swap.c linux-2.6.15/mm/swap.c > --- linux-2.6.15.orig/mm/swap.c 2006-01-02 22:21:10.000000000 -0500 > +++ linux-2.6.15/mm/swap.c 2006-03-07 11:45:37.000000000 -0500 > @@ -110,6 +110,44 @@ > spin_unlock_irq(&zone->lru_lock); > } > > +/* Wire the page; if the page is in LRU, > + * try move it to Wired list. > + */ > +void fastcall wire_page(struct page *page)
Ahh, here's the elusive beast.
> +{ > + struct zone *zone = page_zone(page); > + > + spin_lock_irq(&zone->lru_lock);
This is a fairly heavy lock for something which is going into page fault fastpaths and such. You might be better off making wired_count atomic and not using a lock in the common case if possible (even if it is only for mlocked pages, I'm sure someone will complain).
> + page->wired_count ++;
Oh dear, I missed this change you made to struct page, tucked away in 5/8. This alone pretty much makes it a showstopper, I'm afraid. You'll have to work out some other way to do it so as not to penalise 99.999% of machines which don't need this.
(Oh, and making the field a short usually won't help either, because of alignment constraints).
> + if(!PageWired(page)){ > + if(PageLRU(page)){ > + del_page_from_lru(zone, page); > + add_page_to_wired_list(zone,page); > + SetPageWired(page); > + } > + } > + spin_unlock_irq(&zone->lru_lock); > +} > + > +/* Unwire the page. > + * If it isnt wired by any process, try move it to active list. > + */ > +void fastcall unwire_page(struct page *page) > +{ > + struct zone *zone = page_zone(page); > + > + spin_lock_irq(&zone->lru_lock); > + page->wired_count --; > + if(!page->wired_count){ > + if(PageLRU(page) && TestClearPageWired(page)){ > + del_page_from_wired_list(zone,page); > + add_page_to_active_list(zone,page); > + SetPageActive(page); > + } > + } > + spin_unlock_irq(&zone->lru_lock); > +} > + > /* > * Mark a page as having seen activity. > * > @@ -119,11 +157,13 @@ > */ > void fastcall mark_page_accessed(struct page *page) > { > - if (!PageActive(page) && PageReferenced(page) && PageLRU(page)) { > - activate_page(page); > - ClearPageReferenced(page); > - } else if (!PageReferenced(page)) { > - SetPageReferenced(page); > + if(!PageWired(page)) { > + if (!PageActive(page) && PageReferenced(page) && PageLRU(page)) { > + activate_page(page); > + ClearPageReferenced(page); > + } else if (!PageReferenced(page)) { > + SetPageReferenced(page); > + } > } > } >
So, umm... what happens when the page gets wired before activate_page()?
> @@ -178,13 +218,15 @@ > struct zone *zone = page_zone(page); > > spin_lock_irqsave(&zone->lru_lock, flags); > - if (TestClearPageLRU(page)) > - del_page_from_lru(zone, page); > - if (page_count(page) != 0) > - page = NULL; > + if(!PageWired(page)) { > + if (TestClearPageLRU(page)) > + del_page_from_lru(zone, page); > + if (page_count(page) != 0) > + page = NULL; > + if (page) > + free_hot_page(page); > + } > spin_unlock_irqrestore(&zone->lru_lock, flags); > - if (page) > - free_hot_page(page); > } >
Hmm... how do PageWired pages get freed, then?
> EXPORT_SYMBOL(__page_cache_release); > @@ -214,7 +256,8 @@ > > if (!put_page_testzero(page)) > continue; > - > + if(PageWired(page)) > + continue; > pagezone = page_zone(page); > if (pagezone != zone) { > if (zone)
Ditto.
-- SUSE Labs, Novell Inc.
Send instant messages to your online friends http://au.messenger.yahoo.com
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |