Messages in this thread | | | Date | Fri, 02 Mar 2007 15:45:51 +0000 | From | Andy Whitcroft <> | Subject | Re: [PATCH 1/5] Lumpy Reclaim V3 |
| |
Christoph Lameter wrote: > On Tue, 27 Feb 2007, Andy Whitcroft wrote: > >> +static int __isolate_lru_page(struct page *page, int active) >> +{ >> + int ret = -EINVAL; >> + >> + if (PageLRU(page) && (PageActive(page) == active)) { >> + ret = -EBUSY; >> + if (likely(get_page_unless_zero(page))) { >> + /* >> + * Be careful not to clear PageLRU until after we're >> + * sure the page is not being freed elsewhere -- the >> + * page release code relies on it. >> + */ >> + ClearPageLRU(page); >> + ret = 0; > > Is that really necessary? PageLRU is clear when a page is freed right? > And clearing PageLRU requires the zone->lru_lock since we have to move it > off the LRU.
Although the PageLRU is stable as we have the zone->lru_lock we cannot take the page off the LRU unless we can take a reference to it preventing it from being released. The page release code relies on the the caller who takes the reference count to zero having exclusive access to the page to release it. To prevent a race with put_page/__page_cache_release we cannot touch the page once its count drops to zero, which occurs before the PageLRU is cleared. PageLRU being sampled outside the zone->lru_lock in that path to avoid taking the lock if not required.
> >> - ClearPageLRU(page); >> - target = dst; >> + active = PageActive(page); > > Why are we saving the active state? Page cannot be moved between LRUs > while we hold the lru lock anyways.
This is used later in the function for comparison against all of the other pages in the 'order' sized area rooted about the target page. Its mearly an optimisation.
-apw
- 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/
| |