linux-kernel - Re: [PATCH 2/7] ksm: treat unstable nid like in stable tree

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <5136ABEE.8000501@gmail.com>
Date:	Wed, 06 Mar 2013 10:37:34 +0800
From:	Ric Mason <ric.masonn@...il.com>
To:	Hugh Dickins <hughd@...gle.com>
CC:	Andrew Morton <akpm@...ux-foundation.org>,
	Mel Gorman <mgorman@...e.de>,
	Petr Holasek <pholasek@...hat.com>,
	Andrea Arcangeli <aarcange@...hat.com>,
	Izik Eidus <izik.eidus@...ellosystems.com>,
	linux-kernel@...r.kernel.org, linux-mm@...ck.org
Subject: Re: [PATCH 2/7] ksm: treat unstable nid like in stable tree

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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/