Re: strange locking __find_get_swapcache_page()

[Andrew Morton <akpm@zip.com.au>]

Linus Torvalds wrote:
> 
> On Mon, 30 Jul 2001, Rik van Riel wrote:
> >
> > I've encountered a suspicious piece of code in filemap.c:
> >
> > struct page * __find_get_swapcache_page( ... )
> 
> Hmm. I thin the whole PageSwapCache() test is bogus - if we found it on
> the swapper_space address space, then the page had better be a swap-cache
> page, and testing for it explicitly is silly.
> 
> Also, it appears that the only caller of this is
> find_get_swapcache_page(), which in itself really doesn't even care: it
> just uses the lookup as a boolen on whether to add a new page to the swap
> cache, and does even _that_ completely wrong. There's a big race there,
> see if you can spot it.

read_swap_cache_async()?  All code paths in that area are
under lock_kernel().

> The fix, I suspect, is to pretty much get rid of the code altogether, and
> make it use add_to_page_cache_unique() or whatever it is called that gets
> the duplicate check _right_.

The whole lot needed spring cleaning 1-2 years ago, but I see no
bugs in there.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/