Messages in this thread | | | Date | Thu, 28 May 2009 10:05:01 +0900 | From | KAMEZAWA Hiroyuki <> | Subject | Re: [RFC][PATCH 2/5] add SWAP_HAS_CACHE flag to swap_map |
| |
On Thu, 28 May 2009 09:41:57 +0900 Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> wrote:
> > @@ -1969,17 +2017,33 @@ int swap_duplicate(swp_entry_t entry) > > offset = swp_offset(entry); > > > > spin_lock(&swap_lock); > > - if (offset < p->max && p->swap_map[offset]) { > > - if (p->swap_map[offset] < SWAP_MAP_MAX - 1) { > > - p->swap_map[offset]++; > > + > > + if (unlikely(offset >= p->max)) > > + goto unlock_out; > > + > > + count = swap_count(p->swap_map[offset]); > > + has_cache = swap_has_cache(p->swap_map[offset]); > > + if (cache) { > > + /* set SWAP_HAS_CACHE if there is no cache and entry is used */ > > + if (!has_cache && count) { > Should we check !has_cache here ? I added !has_cache to return 0 in racy case.
> > Concurrent read_swap_cache_async() might have set SWAP_HAS_CACHE, but not have added > a page to swap cache yet when find_get_page() was called. yes.
> add_to_swap_cache() would handle the race of concurrent read_swap_cache_async(), > but considering more, swapcache_free() at the end of the loop might dangerous in this case...
I can't catch what you mean.
I think swapcache_prepare() returns 0 in racy case and no add_to_swap_cache() happens. wrong ?
> So I think it should be like: > > read_swap_cache_async() > : > valid = swapcache_prepare(entry); > if (!valid) > break; > if (valid == -EAGAIN); > continue; > > to let the context that succeeded in swapcache_prepare() do add_to_swap_cache(). >
What you reccomend is code like this ?
== ret = swapcache_prapare(entry); if (ret == -ENOENT) break; /* unused swap entry */ if (ret == -EBUSY) continue; /* to call find_get_page() again */ ==
-Kame
| |