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: Wed, 27 May 2009 14:00:01 +0900 [thread overview] Message-ID: <20090527140001.80360afb.nishimura@mxp.nes.nec.co.jp> (raw) In-Reply-To: <20090527133629.142aa42f.kamezawa.hiroyu@jp.fujitsu.com> On Wed, 27 May 2009 13:36:29 +0900, KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> wrote: > On Wed, 27 May 2009 13:02:46 +0900 > Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> wrote: > > > > @@ -1067,21 +1113,21 @@ static int try_to_unuse(unsigned int typ > > > } > > > > > > /* > > > - * How could swap count reach 0x7fff when the maximum > > > - * pid is 0x7fff, and there's no way to repeat a swap > > > - * page within an mm (except in shmem, where it's the > > > - * shared object which takes the reference count)? > > > - * We believe SWAP_MAP_MAX cannot occur in Linux 2.4. > > > - * > > > + * How could swap count reach 0x7ffe ? > > > + * There's no way to repeat a swap page within an mm > > > + * (except in shmem, where it's the shared object which takes > > > + * the reference count)? > > > + * We believe SWAP_MAP_MAX cannot occur.(if occur, unsigned > > > + * short is too small....) > > > * If that's wrong, then we should worry more about > > > * exit_mmap() and do_munmap() cases described above: > > > * we might be resetting SWAP_MAP_MAX too early here. > > > * We know "Undead"s can happen, they're okay, so don't > > > * report them; but do report if we reset SWAP_MAP_MAX. > > > */ > > > - if (*swap_map == SWAP_MAP_MAX) { > > > + if (swap_count(*swap_map) == SWAP_MAP_MAX) { > > > spin_lock(&swap_lock); > > > - *swap_map = 1; > > > + *swap_map = make_swap_count(0, 1); > > Can we assume the entry has SWAP_HAS_CACHE here ? > > Shouldn't we check PageSwapCache beforehand ? > > > > IIUC, in this try_to_unuse code, the page is added to swap cache and locked > before reaches here. But....ah,ok, unuse_mm() may release lock_page() before > reach here. Then... > And the owner process might have removed the swap cache before we take the lock, as the following comments in try_to_unuse() says. > if (PageSwapCache(page) && swap_count(*swap_map) == SWAP_MAP_MAX) > > is right ? (maybe original code, set to "1" is also buggy.) > Reading the following code in try_to_unuse(), I think int valid_swap_cache = !!(PageSwapCache(page) && page_private(page) == entry.val) : *swap_map = make_swap_count(0(or 1?), valid_swap_cache); might be better. But I can't confirm it anyway. I've never hit SWAP_MAP_MAX. Thanks, Daisuke Nishimura.
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: Wed, 27 May 2009 14:00:01 +0900 [thread overview] Message-ID: <20090527140001.80360afb.nishimura@mxp.nes.nec.co.jp> (raw) In-Reply-To: <20090527133629.142aa42f.kamezawa.hiroyu@jp.fujitsu.com> On Wed, 27 May 2009 13:36:29 +0900, KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> wrote: > On Wed, 27 May 2009 13:02:46 +0900 > Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> wrote: > > > > @@ -1067,21 +1113,21 @@ static int try_to_unuse(unsigned int typ > > > } > > > > > > /* > > > - * How could swap count reach 0x7fff when the maximum > > > - * pid is 0x7fff, and there's no way to repeat a swap > > > - * page within an mm (except in shmem, where it's the > > > - * shared object which takes the reference count)? > > > - * We believe SWAP_MAP_MAX cannot occur in Linux 2.4. > > > - * > > > + * How could swap count reach 0x7ffe ? > > > + * There's no way to repeat a swap page within an mm > > > + * (except in shmem, where it's the shared object which takes > > > + * the reference count)? > > > + * We believe SWAP_MAP_MAX cannot occur.(if occur, unsigned > > > + * short is too small....) > > > * If that's wrong, then we should worry more about > > > * exit_mmap() and do_munmap() cases described above: > > > * we might be resetting SWAP_MAP_MAX too early here. > > > * We know "Undead"s can happen, they're okay, so don't > > > * report them; but do report if we reset SWAP_MAP_MAX. > > > */ > > > - if (*swap_map == SWAP_MAP_MAX) { > > > + if (swap_count(*swap_map) == SWAP_MAP_MAX) { > > > spin_lock(&swap_lock); > > > - *swap_map = 1; > > > + *swap_map = make_swap_count(0, 1); > > Can we assume the entry has SWAP_HAS_CACHE here ? > > Shouldn't we check PageSwapCache beforehand ? > > > > IIUC, in this try_to_unuse code, the page is added to swap cache and locked > before reaches here. But....ah,ok, unuse_mm() may release lock_page() before > reach here. Then... > And the owner process might have removed the swap cache before we take the lock, as the following comments in try_to_unuse() says. > if (PageSwapCache(page) && swap_count(*swap_map) == SWAP_MAP_MAX) > > is right ? (maybe original code, set to "1" is also buggy.) > Reading the following code in try_to_unuse(), I think int valid_swap_cache = !!(PageSwapCache(page) && page_private(page) == entry.val) : *swap_map = make_swap_count(0(or 1?), valid_swap_cache); might be better. But I can't confirm it anyway. I've never hit SWAP_MAP_MAX. Thanks, Daisuke Nishimura. -- 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-27 5:08 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 [this message] 2009-05-27 5:00 ` Daisuke Nishimura 2009-05-28 0:41 ` Daisuke Nishimura 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=20090527140001.80360afb.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.