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]
Date:	Tue, 21 Jun 2016 18:39:29 -0700 (PDT)
From:	Hugh Dickins <hughd@...gle.com>
To:	zhouxianrong <zhouxianrong@...wei.com>
cc:	linux-mm@...ck.org, akpm@...ux-foundation.org, hughd@...gle.com,
	aarcange@...hat.com, kirill.shutemov@...ux.intel.com,
	dave.hansen@...ux.intel.com, zhouchengming1@...wei.com,
	geliangtang@....com, linux-kernel@...r.kernel.org,
	zhouxiyu@...wei.com, wanghaijun5@...wei.com
Subject: Re: [PATCH v2] more mapcount page as kpage could reduce total
 replacement times than fewer mapcount one in probability.

On Tue, 21 Jun 2016, zhouxianrong wrote:

> hey hugh:
>     could you please give me some suggestion about this ?

I must ask you to be more patient: everyone would like me to be
quicker, but I cannot; and this does not appear to be urgent.

Your idea makes sense to me; but if your patch seems obvious to you,
sorry, it isn't obvious to me.  The two pages are not symmetrical,
the caller of try_to_merge_two_pages() thinks it knows which is which,
swapping them around underneath it like this is not obviously correct.

Your patch may be fine, but I've not had time to think it through:
will do, but not immediately.

Your idea may not make so much sense to Andrea: he has been troubled
by the difficulty in unmapping a KSM page with a very high mapcount.

And you would be maximizing a buggy case, if we think of that page
being mapped also into non-VM_MERGEABLE areas; but I think we can
ignore that aspect, it's buggy already, and I don't think anyone
really cares deeply about madvise(,,MADV_UNMERGEABLE) correctness
on forked areas.  KSM was not originally written with fork in mind.

I have never seen such a long title for a patch: maybe
"[PATCH] ksm: choose the more mapped for the KSM page".

> 
> On 2016/6/15 9:56, zhouxianrong@...wei.com wrote:
> > From: z00281421 <z00281421@...esmail.huawei.com>
> > 
> > more mapcount page as kpage could reduce total replacement times
> > than fewer mapcount one when ksmd scan and replace among
> > forked pages later.
> > 
> > Signed-off-by: z00281421 <z00281421@...esmail.huawei.com>

And I doubt that z00281421 is your real name:
see Documentation/SubmittingPatches.

Hugh

> > ---
> >  mm/ksm.c |    8 ++++++++
> >  1 file changed, 8 insertions(+)
> > 
> > diff --git a/mm/ksm.c b/mm/ksm.c
> > index 4786b41..4d530af 100644
> > --- a/mm/ksm.c
> > +++ b/mm/ksm.c
> > @@ -1094,6 +1094,14 @@ static struct page *try_to_merge_two_pages(struct
> > rmap_item *rmap_item,
> >  {
> >  	int err;
> > 
> > +	/*
> > +	 * select more mapcount page as kpage
> > +	 */
> > +	if (page_mapcount(page) < page_mapcount(tree_page)) {
> > +		swap(page, tree_page);
> > +		swap(rmap_item, tree_rmap_item);
> > +	}
> > +
> >  	err = try_to_merge_with_ksm_page(rmap_item, page, NULL);
> >  	if (!err) {
> >  		err = try_to_merge_with_ksm_page(tree_rmap_item,
> > 

Powered by blists - more mailing lists