Re: 2.4.10pre VM changes: Potential race condition on swap code

[Hugh Dickins]

On Fri, 14 Sep 2001, Marcelo Tosatti wrote:
> 
> I would prefer to make get_swap_page() not lock the swap lock anymore,
> making it necessary to its callers to do the locking themselves. So:
> 
> try_to_swap_out() {
> 	swap_device_lock()
> 	get_swap_page()
> 	add_to_swap_cache()
> 	swap_device_unlock()
> }
> 
> read_swap_cache_async() {
> 	page = alloc_page(page)
> 	swap_device_lock()
> 	if(!swap_map[offset]) {
> 		page_cache_release(page)
> 		swap_device_unlock()
> 		return 1;
> 	}
> 	alias = __find_page()
> 	if (!alias) {
> 		swap_map[offset]++;
> 		add_to_swap_cache(page)
> 	}
> 	swap_device_unlock()
> 	rw_swap_page(page)
> }

This does gloss over the distinction between the swap_list_lock()
and the swap_device_lock(si).  The latter is the more crucial here,
but difficult to use in this way.  Though if you were to throw it
away and convert to swap_list_lock() throughout, I wonder if we'd
lose much (only gain on systems with just one swap area).  But I
wasn't daring to combine them myself.

> If you don't have that one already done, I can write it as soon as you
> answer me.

Don't wait on me: I'm not ready with my implementation yet, and
you think a lot faster than I do.  If you find you can resolve
the details, go ahead.  Beware shmem_writepage, where the one
page metamorphoses from being a file page to being a swap page.
Do you intend to scrap the BKL bracketing now?

I hope to resume tomorrow, unless you've sewn it up by then.

Hugh