Re: [RFC] Alternative raceless page free

Christian Ehrhardt
2002-09-05T12:29:38Z

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!