On Tue, 10 Jun 2008 00:45:59 +0400
Alexey Dobriyan <adobriyan@gmail.com> wrote:
We unlocked an already-unlocked page.
Although pretty straightforward, shrink_page_list() is, umm, large.
This part:
if (PagePrivate(page)) {
if (!try_to_release_page(page, sc->gfp_mask))
goto activate_locked;
if (!mapping && page_count(page) == 1) {
unlock_page(page);
if (put_page_testzero(page))
goto free_it;
else {
nr_reclaimed++;
continue;
}
}
}
if (!mapping || !__remove_mapping(mapping, page))
goto keep_locked;
free_it:
unlock_page(page);
has a very obvious double-unlock. It was added by the obviously-buggy,
reviewed-by-everyone mm-speculative-page-references.patch - part of
Nick's lockless pagecache work.
argh. This means that I need to a) stop merging anything and b) be
sent a fix really fast or drop them all and fix up all the fallout and
c) get -mm2 out asap to that someone can test all the other
page-reclaim changes. argh.
Also, what's up with that "continue" which got added there? We just
leave the page floating about without reattaching it to any LRU?
Where's the code comment explaining wth is going on in there?
More argh.
--