Hi Hugh,
On 02/23/2013 05:03 AM, Hugh Dickins wrote:
> On Fri, 22 Feb 2013, Ric Mason wrote:
>> On 02/21/2013 04:20 PM, Hugh Dickins wrote:
>>> An inconsistency emerged in reviewing the NUMA node changes to KSM:
>>> when meeting a page from the wrong NUMA node in a stable tree, we say
>>> that it's okay for comparisons, but not as a leaf for merging; whereas
>>> when meeting a page from the wrong NUMA node in an unstable tree, we
>>> bail out immediately.
>> IIUC
>> - ksm page from the wrong NUMA node will be add to current node's stable tree
Please forgive my late response.
> That should never happen (and when I was checking with a WARN_ON it did
> not happen). What can happen is that a node already in a stable tree
> has its page migrated away to another NUMA node.
>
>> - normal page from the wrong NUMA node will be merged to current node's
>> stable tree <- where I miss here? I didn't see any special handling in
>> function stable_tree_search for this case.
> nid = get_kpfn_nid(page_to_pfn(page));
> root = root_stable_tree + nid;
>
> to choose the right tree for the page, and
>
> if (get_kpfn_nid(stable_node->kpfn) !=
> NUMA(stable_node->nid)) {
> put_page(tree_page);
> goto replace;
> }
>
> to make sure that we don't latch on to a node whose page got migrated away.
I think the ksm implementation for num awareness is buggy.
For page migratyion stuff, new page is allocated from node *which page
is migrated to*.
- when meeting a page from the wrong NUMA node in an unstable tree
get_kpfn_nid(page_to_pfn(page)) *==* page_to_nid(tree_page)
How can say it's okay for comparisons, but not as a leaf for merging?
- when meeting a page from the wrong NUMA node in an stable tree
- meeting a normal page
- meeting a page which is ksm page before migration
get_kpfn_nid(stable_node->kpfn) != NUMA(stable_node->nid) can't
capture them since stable_node is for tree page in current stable tree.
They are always equal.
>
>> - normal page from the wrong NUMA node will compare but not as a leaf for
>> merging after the patch
> I don't understand you there, but hope my remarks above resolve it.
>
> Hugh
On Fri, 1 Mar 2013, Ric Mason wrote:
>
> I think the ksm implementation for num awareness is buggy.
Sorry, I just don't understand your comments below,
but will try to answer or question them as best I can.
>
> For page migratyion stuff, new page is allocated from node *which page is
> migrated to*.
Yes, by definition.
> - when meeting a page from the wrong NUMA node in an unstable tree
> get_kpfn_nid(page_to_pfn(page)) *==* page_to_nid(tree_page)
I thought you were writing of the wrong NUMA node case,
but now you emphasize "*==*", which means the right NUMA node.
> How can say it's okay for comparisons, but not as a leaf for merging?
Pages in the unstable tree are unstable (and it's not even accurate to
say "pages in the unstable tree"), they and their content can change at
any moment, so I cannot assert anything of them for sure.
But if we suppose, as an approximation, that they are somewhat likely
to remain stable (and the unstable tree would be useless without that
assumption: it tends to work out), but subject to migration, then it makes
sense to compare content, no matter what NUMA node it is on, in order to
locate a page of the same content; but wrong to merge with that page if
it's on the wrong NUMA node, if !merge_across_nodes tells us not to.
> - when meeting a page from the wrong NUMA node in an stable tree
> - meeting a normal page
What does that line mean, and where does it fit in your argument?
> - meeting a page which is ksm page before migration
> get_kpfn_nid(stable_node->kpfn) != NUMA(stable_node->nid) can't capture
> them since stable_node is for tree page in current stable tree. They are
> always equal.
When we meet a ksm page in the stable tree before it's migrated to another
NUMA node, yes, it will be on the right NUMA node (because we were careful
only to merge pages from the right NUMA node there), and that test will not
capture them. It's for capturng a ksm page in the stable tree after it has
been migrated to another NUMA node.
Hugh
Hi Hugh,
On 03/02/2013 04:03 AM, Hugh Dickins wrote:
> On Fri, 1 Mar 2013, Ric Mason wrote:
>> I think the ksm implementation for num awareness is buggy.
> Sorry, I just don't understand your comments below,
> but will try to answer or question them as best I can.
>
>> For page migratyion stuff, new page is allocated from node *which page is
>> migrated to*.
> Yes, by definition.
>
>> - when meeting a page from the wrong NUMA node in an unstable tree
>> get_kpfn_nid(page_to_pfn(page)) *==* page_to_nid(tree_page)
> I thought you were writing of the wrong NUMA node case,
> but now you emphasize "*==*", which means the right NUMA node.
Yes, I mean the wrong NUMA node. During page migration, new page has
already been allocated in new node and old page maybe freed. So
tree_page is the page in new node's unstable tree, page is also new node
page, so get_kpfn_nid(page_to_pfn(page)) *==* page_to_nid(tree_page).
>
>> How can say it's okay for comparisons, but not as a leaf for merging?
> Pages in the unstable tree are unstable (and it's not even accurate to
> say "pages in the unstable tree"), they and their content can change at
> any moment, so I cannot assert anything of them for sure.
>
> But if we suppose, as an approximation, that they are somewhat likely
> to remain stable (and the unstable tree would be useless without that
> assumption: it tends to work out), but subject to migration, then it makes
> sense to compare content, no matter what NUMA node it is on, in order to
> locate a page of the same content; but wrong to merge with that page if
> it's on the wrong NUMA node, if !merge_across_nodes tells us not to.
>
>
>> - when meeting a page from the wrong NUMA node in an stable tree
>> - meeting a normal page
> What does that line mean, and where does it fit in your argument?
I distinguish pages in three kinds.
- ksm page which already in stable tree in old node
- page in unstable tree in old node
- page not in any trees in old node
So normal page here I mean page not in any trees in old node.
>
>> - meeting a page which is ksm page before migration
>> get_kpfn_nid(stable_node->kpfn) != NUMA(stable_node->nid) can't capture
>> them since stable_node is for tree page in current stable tree. They are
>> always equal.
> When we meet a ksm page in the stable tree before it's migrated to another
> NUMA node, yes, it will be on the right NUMA node (because we were careful
> only to merge pages from the right NUMA node there), and that test will not
> capture them. It's for capturng a ksm page in the stable tree after it has
> been migrated to another NUMA node.
ksm page migrated to another NUMA node still not freed, why? Who take
page count of it? If not freed, since new page is allocated in new
node, it is the copy of current ksm page, so current ksm doesn't change,
get_kpfn_nid(stable_node->kpfn) *==* NUMA(stable_node->nid).
>
> Hugh
On Sat, 2 Mar 2013, Ric Mason wrote:
> On 03/02/2013 04:03 AM, Hugh Dickins wrote:
> > On Fri, 1 Mar 2013, Ric Mason wrote:
> > > I think the ksm implementation for num awareness is buggy.
> > Sorry, I just don't understand your comments below,
> > but will try to answer or question them as best I can.
> >
> > > For page migratyion stuff, new page is allocated from node *which page is
> > > migrated to*.
> > Yes, by definition.
> >
> > > - when meeting a page from the wrong NUMA node in an unstable tree
> > > get_kpfn_nid(page_to_pfn(page)) *==* page_to_nid(tree_page)
> > I thought you were writing of the wrong NUMA node case,
> > but now you emphasize "*==*", which means the right NUMA node.
>
> Yes, I mean the wrong NUMA node. During page migration, new page has already
> been allocated in new node and old page maybe freed. So tree_page is the
> page in new node's unstable tree, page is also new node page, so
> get_kpfn_nid(page_to_pfn(page)) *==* page_to_nid(tree_page).
I don't understand; but here you seem to be describing a case where two
pages from the same NUMA node get merged (after both have been migrated
from another NUMA node?), and there's nothing wrong with that,
so I won't worry about it further.
> > > - meeting a page which is ksm page before migration
> > > get_kpfn_nid(stable_node->kpfn) != NUMA(stable_node->nid) can't
> > > capture
> > > them since stable_node is for tree page in current stable tree. They are
> > > always equal.
> > When we meet a ksm page in the stable tree before it's migrated to another
> > NUMA node, yes, it will be on the right NUMA node (because we were careful
> > only to merge pages from the right NUMA node there), and that test will not
> > capture them. It's for capturng a ksm page in the stable tree after it has
> > been migrated to another NUMA node.
>
> ksm page migrated to another NUMA node still not freed, why? Who take page
> count of it?
The old page, the one which used to be a ksm page on the old NUMA node,
should be freed very soon: since it was isolated from lru, and its page
count checked, I cannot think of anything to hold a reference to it,
apart from migration itself - so it just needs to reach putback_lru_page(),
and then may rest awhile on __lru_cache_add()'s pagevec before being freed.
But I don't see where I said the old page was still not freed.
> If not freed, since new page is allocated in new node, it is
> the copy of current ksm page, so current ksm doesn't change,
> get_kpfn_nid(stable_node->kpfn) *==* NUMA(stable_node->nid).
But ksm_migrate_page() did
VM_BUG_ON(stable_node->kpfn != page_to_pfn(oldpage));
stable_node->kpfn = page_to_pfn(newpage);
without changing stable_node->nid.
Hugh
Hi Hugh,
On 03/02/2013 10:57 AM, Hugh Dickins wrote:
How ksm treat a ksm forked page? IIUC, it's not merged in ksm stable
tree. It will just be ignore?
> On Sat, 2 Mar 2013, Ric Mason wrote:
>> On 03/02/2013 04:03 AM, Hugh Dickins wrote:
>>> On Fri, 1 Mar 2013, Ric Mason wrote:
>>>> I think the ksm implementation for num awareness is buggy.
>>> Sorry, I just don't understand your comments below,
>>> but will try to answer or question them as best I can.
>>>
>>>> For page migratyion stuff, new page is allocated from node *which page is
>>>> migrated to*.
>>> Yes, by definition.
>>>
>>>> - when meeting a page from the wrong NUMA node in an unstable tree
>>>> get_kpfn_nid(page_to_pfn(page)) *==* page_to_nid(tree_page)
>>> I thought you were writing of the wrong NUMA node case,
>>> but now you emphasize "*==*", which means the right NUMA node.
>> Yes, I mean the wrong NUMA node. During page migration, new page has already
>> been allocated in new node and old page maybe freed. So tree_page is the
>> page in new node's unstable tree, page is also new node page, so
>> get_kpfn_nid(page_to_pfn(page)) *==* page_to_nid(tree_page).
> I don't understand; but here you seem to be describing a case where two
> pages from the same NUMA node get merged (after both have been migrated
> from another NUMA node?), and there's nothing wrong with that,
> so I won't worry about it further.
>
>>>> - meeting a page which is ksm page before migration
>>>> get_kpfn_nid(stable_node->kpfn) != NUMA(stable_node->nid) can't
>>>> capture
>>>> them since stable_node is for tree page in current stable tree. They are
>>>> always equal.
>>> When we meet a ksm page in the stable tree before it's migrated to another
>>> NUMA node, yes, it will be on the right NUMA node (because we were careful
>>> only to merge pages from the right NUMA node there), and that test will not
>>> capture them. It's for capturng a ksm page in the stable tree after it has
>>> been migrated to another NUMA node.
>> ksm page migrated to another NUMA node still not freed, why? Who take page
>> count of it?
> The old page, the one which used to be a ksm page on the old NUMA node,
> should be freed very soon: since it was isolated from lru, and its page
> count checked, I cannot think of anything to hold a reference to it,
> apart from migration itself - so it just needs to reach putback_lru_page(),
> and then may rest awhile on __lru_cache_add()'s pagevec before being freed.
>
> But I don't see where I said the old page was still not freed.
>
>> If not freed, since new page is allocated in new node, it is
>> the copy of current ksm page, so current ksm doesn't change,
>> get_kpfn_nid(stable_node->kpfn) *==* NUMA(stable_node->nid).
> But ksm_migrate_page() did
> VM_BUG_ON(stable_node->kpfn != page_to_pfn(oldpage));
> stable_node->kpfn = page_to_pfn(newpage);
> without changing stable_node->nid.
>
> Hugh
>
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to [email protected]. For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
On 03/02/2013 10:57 AM, Hugh Dickins wrote:
> On Sat, 2 Mar 2013, Ric Mason wrote:
>> On 03/02/2013 04:03 AM, Hugh Dickins wrote:
>>> On Fri, 1 Mar 2013, Ric Mason wrote:
>>>> I think the ksm implementation for num awareness is buggy.
>>> Sorry, I just don't understand your comments below,
>>> but will try to answer or question them as best I can.
>>>
>>>> For page migratyion stuff, new page is allocated from node *which page is
>>>> migrated to*.
>>> Yes, by definition.
>>>
>>>> - when meeting a page from the wrong NUMA node in an unstable tree
>>>> get_kpfn_nid(page_to_pfn(page)) *==* page_to_nid(tree_page)
>>> I thought you were writing of the wrong NUMA node case,
>>> but now you emphasize "*==*", which means the right NUMA node.
>> Yes, I mean the wrong NUMA node. During page migration, new page has already
>> been allocated in new node and old page maybe freed. So tree_page is the
>> page in new node's unstable tree, page is also new node page, so
>> get_kpfn_nid(page_to_pfn(page)) *==* page_to_nid(tree_page).
> I don't understand; but here you seem to be describing a case where two
> pages from the same NUMA node get merged (after both have been migrated
> from another NUMA node?), and there's nothing wrong with that,
> so I won't worry about it further.
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 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) ?
>
>>>> - meeting a page which is ksm page before migration
>>>> get_kpfn_nid(stable_node->kpfn) != NUMA(stable_node->nid) can't
>>>> capture
>>>> them since stable_node is for tree page in current stable tree. They are
>>>> always equal.
>>> When we meet a ksm page in the stable tree before it's migrated to another
>>> NUMA node, yes, it will be on the right NUMA node (because we were careful
>>> only to merge pages from the right NUMA node there), and that test will not
>>> capture them. It's for capturng a ksm page in the stable tree after it has
>>> been migrated to another NUMA node.
>> ksm page migrated to another NUMA node still not freed, why? Who take page
>> count of it?
> The old page, the one which used to be a ksm page on the old NUMA node,
> should be freed very soon: since it was isolated from lru, and its page
> count checked, I cannot think of anything to hold a reference to it,
> apart from migration itself - so it just needs to reach putback_lru_page(),
> and then may rest awhile on __lru_cache_add()'s pagevec before being freed.
>
> But I don't see where I said the old page was still not freed.
>
>> If not freed, since new page is allocated in new node, it is
>> the copy of current ksm page, so current ksm doesn't change,
>> get_kpfn_nid(stable_node->kpfn) *==* NUMA(stable_node->nid).
> But ksm_migrate_page() did
> VM_BUG_ON(stable_node->kpfn != page_to_pfn(oldpage));
> stable_node->kpfn = page_to_pfn(newpage);
> without changing stable_node->nid.
>
> Hugh
On Wed, 6 Mar 2013, Will Huck wrote:
>
> How ksm treat a ksm forked page? IIUC, it's not merged in ksm stable tree. It
> will just be ignore?
No, it's there in the stable tree, as it was before it got forked. And
when ksmd comes around to find the new mm, it will allocate an rmap_item
for that page in the new mm, and append it to the existing stable_node.
Hugh
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
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
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
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. ]
I think I can basically understand you, please correct me if something
wrong.
For ksm page:
If one ksm page(in old node) migrate to another(new) node(ksm page is
treated as old page, one new page allocated in another node now), since
we can't get right lock in this time, we can't move stable node to its
new tree at this time, stable node still in old node and
stable_node->nid still store old node value. If ksmd scan and compare
another page in old node and search stable tree will figure out that
stable node relevant ksm page is migrated to new node, stable node will
be erased from old node's stable tree and link to migrate_nodes list.
What's the life of new page in new node? new page will be scaned by
ksmd, it will search stable tree in new node and if doesn't find matched
stable node, the new node is deleted from migrate_node list and add to
new node's table tree as a leaf, if find stable node in stable tree,
they will be merged. But in special case, the stable node relevant ksm
page can also migrated, new stable node will replace the stable node
which relevant page migrated this time.
For unstable tree page:
If search in unstable tree and find the tree page which has equal
content is migrated, just stop search and return, nothing merged. The
new page in new node for this migrated unstable tree page will be insert
to unstable tree in new node.
>> 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
> 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
Ping Hugh, :-)
On 03/06/2013 06:18 PM, Ric Mason wrote:
> 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. ]
>
> I think I can basically understand you, please correct me if something
> wrong.
>
> For ksm page:
> If one ksm page(in old node) migrate to another(new) node(ksm page is
> treated as old page, one new page allocated in another node now),
> since we can't get right lock in this time, we can't move stable node
> to its new tree at this time, stable node still in old node and
> stable_node->nid still store old node value. If ksmd scan and compare
> another page in old node and search stable tree will figure out that
> stable node relevant ksm page is migrated to new node, stable node
> will be erased from old node's stable tree and link to migrate_nodes
> list. What's the life of new page in new node? new page will be scaned
> by ksmd, it will search stable tree in new node and if doesn't find
> matched stable node, the new node is deleted from migrate_node list
> and add to new node's table tree as a leaf, if find stable node in
> stable tree, they will be merged. But in special case, the stable node
> relevant ksm page can also migrated, new stable node will replace the
> stable node which relevant page migrated this time.
> For unstable tree page:
> If search in unstable tree and find the tree page which has equal
> content is migrated, just stop search and return, nothing merged. The
> new page in new node for this migrated unstable tree page will be
> insert to unstable tree in new node.
>
>>> 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
>> 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
>