linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Ric Mason <ric.masonn@gmail.com>
To: Hugh Dickins <hughd@google.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Mel Gorman <mgorman@suse.de>, Petr Holasek <pholasek@redhat.com>,
	Andrea Arcangeli <aarcange@redhat.com>,
	Izik Eidus <izik.eidus@ravellosystems.com>,
	linux-kernel@vger.kernel.org, linux-mm@kvack.org
Subject: Re: [PATCH 2/7] ksm: treat unstable nid like in stable tree
Date: Wed, 06 Mar 2013 14:58:36 +0800	[thread overview]
Message-ID: <5136E91C.3020700@gmail.com> (raw)
In-Reply-To: <alpine.LNX.2.00.1303052031320.29433@eggly.anvils>

Hi Hugh,
On 03/06/2013 01:05 PM, Hugh Dickins wrote:
> On Wed, 6 Mar 2013, Ric Mason wrote:
> [ I've deleted the context because that was about the unstable tree,
>    and here you have moved to asking about a case in the stable tree. ]
>> For the case of a ksm page is migrated to a different NUMA node and migrate
>> its stable node to  the right tree and collide with an existing stable node.
>> get_kpfn_nid(stable_node->kpfn) != NUMA(stable_node->nid) can capture nothing
> That's not so: as I've pointed out before, ksm_migrate_page() updates
> stable_node->kpfn for the new page on the new NUMA node; but it cannot
> (get the right locking to) move the stable_node to its new tree at that time.
>
> It's moved out once ksmd notices that it's in the wrong NUMA node tree -
> perhaps when one its rmap_items reaches the head of cmp_and_merge_page(),
> or perhaps here in stable_tree_search() when it matches another page
> coming in to cmp_and_merge_page().
>
> You may be concentrating on the case when that "another page" is a ksm
> page migrated from a different NUMA node; and overlooking the case of
> when the matching ksm page in this stable tree has itself been migrated.
>
>> since stable_node is the node in the right stable tree, nothing happen to it
>> before this check. Did you intend to check get_kpfn_nid(page_node->kpfn) !=
>> NUMA(page_node->nid) ?
> Certainly not: page_node is usually NULL.  But I could have checked

Are you sure? list_del(&page_node->list) and DO_NUMA(page_node->nid = 
nid) will trigger panic now.
> get_kpfn_nid(stable_node->kpfn) != nid: I was duplicating the test
> from cmp_and_merge_page(), but here we do have local variable nid.
>
> Hugh

--
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>

  reply	other threads:[~2013-03-06  6:58 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-02-21  8:17 [PATCH 0/7] ksm: responses to NUMA review Hugh Dickins
2013-02-21  8:19 ` [PATCH 1/7] ksm: add some comments Hugh Dickins
2013-02-22  4:26   ` Ric Mason
2013-02-22 20:50     ` Hugh Dickins
2013-02-21  8:20 ` [PATCH 2/7] ksm: treat unstable nid like in stable tree Hugh Dickins
2013-02-22  7:13   ` Ric Mason
2013-02-22 21:03     ` Hugh Dickins
2013-03-01  5:29       ` Ric Mason
2013-03-01 20:03         ` Hugh Dickins
2013-03-02  1:10           ` Ric Mason
2013-03-02  2:57             ` Hugh Dickins
2013-03-06  1:28               ` Will Huck
2013-03-06  4:31                 ` Hugh Dickins
2013-03-06  2:37               ` Ric Mason
2013-03-06  5:05                 ` Hugh Dickins
2013-03-06  6:58                   ` Ric Mason [this message]
2013-03-06 10:18                   ` Ric Mason
2013-03-07 23:26                     ` Ric Mason
2013-02-21  8:22 ` [PATCH 3/7] ksm: shrink 32-bit rmap_item back to 32 bytes Hugh Dickins
2013-02-21  8:23 ` [PATCH 4/7] mm,ksm: FOLL_MIGRATION do migration_entry_wait Hugh Dickins
2013-02-21  8:25 ` [PATCH 5/7] mm,ksm: swapoff might need to copy Hugh Dickins
2013-02-21 14:53   ` Johannes Weiner
2013-02-22 17:16     ` Hugh Dickins
2013-02-21  8:27 ` [PATCH 6/7] mm: cleanup "swapcache" in do_swap_page Hugh Dickins
2013-02-21  8:29 ` [PATCH 7/7] ksm: allocate roots when needed Hugh Dickins
2013-02-22  3:44 ` [PATCH 0/7] ksm: responses to NUMA review Ric Mason
2013-02-22 20:38   ` Hugh Dickins
2013-02-24  1:39     ` Ric Mason

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=5136E91C.3020700@gmail.com \
    --to=ric.masonn@gmail.com \
    --cc=aarcange@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=hughd@google.com \
    --cc=izik.eidus@ravellosystems.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mgorman@suse.de \
    --cc=pholasek@redhat.com \
    /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: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).