On Thu, Sep 05, 2002 at 06:42:12AM +0200, Daniel Phillips wrote:
> I have a slight preference for the extra-lru-count version, because of the
> trylock in page_cache_release. This means that nobody will have to spin when
> shrink_cache is active. Instead, freed pages that collide with the lru lock
> can just be left on the lru list to be picked up efficiently later. The
> trylock also allows the lru lock to be acquired speculatively from interrupt
> context, without a requirement that lru lock holders disable interrupts.
> Both versions are provably correct, modulo implementation gaffs.
>
> The linked patch defaults to atomic_dec_and_lock version. To change to
> the extra count version, define LRU_PLUS_CACHE as 2 instead of 1.
>
> Christian, can you please run this one through your race detector?
I don't see a priciple problem with this approach (except that there
may be architectures that don't have cmpxchg or similar). But this hunk
@@ -455,7 +458,7 @@
} else {
/* failed to drop the buffers so stop here */
UnlockPage(page);
- page_cache_release(page);
+ put_page(page);
spin_lock(&pagemap_lru_lock);
continue;
looks a bit suspicious. put_page is not allowed if the page is still
on the lru and there is no other reference to it. As we don't hold any
locks between UnlockPage and put_page there is no formal guarantee that
the above condition is met. I don't have another path that could race
with this one though and chances are that there actually is none.
regards Christian
-- THAT'S ALL FOLKS! - 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/
This archive was generated by hypermail 2b29 : Sat Sep 07 2002 - 22:00:24 EST