From: Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> To: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> Cc: "linux-mm@kvack.org" <linux-mm@kvack.org>, "balbir@linux.vnet.ibm.com" <balbir@linux.vnet.ibm.com>, "hugh.dickins@tiscali.co.uk" <hugh.dickins@tiscali.co.uk>, "hannes@cmpxchg.org" <hannes@cmpxchg.org>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> Subject: Re: [RFC][PATCH 2/5] add SWAP_HAS_CACHE flag to swap_map Date: Thu, 28 May 2009 09:41:57 +0900 [thread overview] Message-ID: <20090528094157.5c39ac57.nishimura@mxp.nes.nec.co.jp> (raw) In-Reply-To: <20090526121547.ce866fe4.kamezawa.hiroyu@jp.fujitsu.com> > @@ -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 ? 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. 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... 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(). Thanks, Daisuke Nishimura. > + p->swap_map[offset] = make_swap_count(count, 1); > + result = 1; > + } > + } else if (count || has_cache) { > + if (count < SWAP_MAP_MAX - 1) { > + p->swap_map[offset] = make_swap_count(count + 1, > + has_cache); > result = 1; > - } else if (p->swap_map[offset] <= SWAP_MAP_MAX) { > + } else if (count <= SWAP_MAP_MAX) { > if (swap_overflow++ < 5) > - printk(KERN_WARNING "swap_dup: swap entry overflow\n"); > - p->swap_map[offset] = SWAP_MAP_MAX; > + printk(KERN_WARNING > + "swap_dup: swap entry overflow\n"); > + p->swap_map[offset] = make_swap_count(SWAP_MAP_MAX, > + has_cache); > result = 1; > } > } > +unlock_out: > spin_unlock(&swap_lock); > out: > return result;
WARNING: multiple messages have this Message-ID (diff)
From: Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> To: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> Cc: "linux-mm@kvack.org" <linux-mm@kvack.org>, "balbir@linux.vnet.ibm.com" <balbir@linux.vnet.ibm.com>, "hugh.dickins@tiscali.co.uk" <hugh.dickins@tiscali.co.uk>, "hannes@cmpxchg.org" <hannes@cmpxchg.org>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> Subject: Re: [RFC][PATCH 2/5] add SWAP_HAS_CACHE flag to swap_map Date: Thu, 28 May 2009 09:41:57 +0900 [thread overview] Message-ID: <20090528094157.5c39ac57.nishimura@mxp.nes.nec.co.jp> (raw) In-Reply-To: <20090526121547.ce866fe4.kamezawa.hiroyu@jp.fujitsu.com> > @@ -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 ? 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. 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... 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(). Thanks, Daisuke Nishimura. > + p->swap_map[offset] = make_swap_count(count, 1); > + result = 1; > + } > + } else if (count || has_cache) { > + if (count < SWAP_MAP_MAX - 1) { > + p->swap_map[offset] = make_swap_count(count + 1, > + has_cache); > result = 1; > - } else if (p->swap_map[offset] <= SWAP_MAP_MAX) { > + } else if (count <= SWAP_MAP_MAX) { > if (swap_overflow++ < 5) > - printk(KERN_WARNING "swap_dup: swap entry overflow\n"); > - p->swap_map[offset] = SWAP_MAP_MAX; > + printk(KERN_WARNING > + "swap_dup: swap entry overflow\n"); > + p->swap_map[offset] = make_swap_count(SWAP_MAP_MAX, > + has_cache); > result = 1; > } > } > +unlock_out: > spin_unlock(&swap_lock); > out: > return result; -- 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/ . Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
next prev parent reply other threads:[~2009-05-28 0:45 UTC|newest] Thread overview: 48+ messages / expand[flat|nested] mbox.gz Atom feed top 2009-05-26 3:12 [RFC][PATCH] memcg: fix swap account (26/May)[0/5] KAMEZAWA Hiroyuki 2009-05-26 3:12 ` KAMEZAWA Hiroyuki 2009-05-26 3:14 ` [RFC][PATCH 1/5] change swap cache interfaces KAMEZAWA Hiroyuki 2009-05-26 3:14 ` KAMEZAWA Hiroyuki 2009-05-26 3:15 ` [RFC][PATCH 2/5] add SWAP_HAS_CACHE flag to swap_map KAMEZAWA Hiroyuki 2009-05-26 3:15 ` KAMEZAWA Hiroyuki 2009-05-27 4:02 ` Daisuke Nishimura 2009-05-27 4:02 ` Daisuke Nishimura 2009-05-27 4:36 ` KAMEZAWA Hiroyuki 2009-05-27 4:36 ` KAMEZAWA Hiroyuki 2009-05-27 5:00 ` Daisuke Nishimura 2009-05-27 5:00 ` Daisuke Nishimura 2009-05-28 0:41 ` Daisuke Nishimura [this message] 2009-05-28 0:41 ` Daisuke Nishimura 2009-05-28 1:05 ` KAMEZAWA Hiroyuki 2009-05-28 1:05 ` KAMEZAWA Hiroyuki 2009-05-28 1:40 ` Daisuke Nishimura 2009-05-28 1:40 ` Daisuke Nishimura 2009-05-28 1:44 ` KAMEZAWA Hiroyuki 2009-05-28 1:44 ` KAMEZAWA Hiroyuki 2009-05-26 3:16 ` [RFC][PATCH 3/5] count cache-only swaps KAMEZAWA Hiroyuki 2009-05-26 3:16 ` KAMEZAWA Hiroyuki 2009-05-26 17:37 ` Johannes Weiner 2009-05-26 17:37 ` Johannes Weiner 2009-05-26 23:49 ` KAMEZAWA Hiroyuki 2009-05-26 23:49 ` KAMEZAWA Hiroyuki 2009-05-26 3:17 ` [RFC][PATCH 4/5] memcg: fix swap account KAMEZAWA Hiroyuki 2009-05-26 3:17 ` KAMEZAWA Hiroyuki 2009-05-26 3:18 ` [RFC][PATCH 5/5] (experimental) chase and free cache only swap KAMEZAWA Hiroyuki 2009-05-26 3:18 ` KAMEZAWA Hiroyuki 2009-05-26 18:14 ` Johannes Weiner 2009-05-26 18:14 ` Johannes Weiner 2009-05-27 0:08 ` KAMEZAWA Hiroyuki 2009-05-27 0:08 ` KAMEZAWA Hiroyuki 2009-05-27 1:26 ` Johannes Weiner 2009-05-27 1:26 ` Johannes Weiner 2009-05-27 1:31 ` KAMEZAWA Hiroyuki 2009-05-27 1:31 ` KAMEZAWA Hiroyuki 2009-05-27 2:06 ` Johannes Weiner 2009-05-27 2:06 ` Johannes Weiner 2009-05-27 5:14 ` KAMEZAWA Hiroyuki 2009-05-27 5:14 ` KAMEZAWA Hiroyuki 2009-05-27 6:30 ` Daisuke Nishimura 2009-05-27 6:30 ` Daisuke Nishimura 2009-05-27 6:50 ` KAMEZAWA Hiroyuki 2009-05-27 6:50 ` KAMEZAWA Hiroyuki 2009-05-27 6:43 ` [RFC][PATCH] memcg: fix swap account (26/May)[0/5] KAMEZAWA Hiroyuki 2009-05-27 6:43 ` KAMEZAWA Hiroyuki
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20090528094157.5c39ac57.nishimura@mxp.nes.nec.co.jp \ --to=nishimura@mxp.nes.nec.co.jp \ --cc=balbir@linux.vnet.ibm.com \ --cc=hannes@cmpxchg.org \ --cc=hugh.dickins@tiscali.co.uk \ --cc=kamezawa.hiroyu@jp.fujitsu.com \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-mm@kvack.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.