All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] more mapcount page as kpage could reduce total replacement times than fewer mapcount one in probability.
@ 2016-06-15  1:56 ` zhouxianrong
  0 siblings, 0 replies; 10+ messages in thread
From: zhouxianrong @ 2016-06-15  1:56 UTC (permalink / raw)
  To: linux-mm
  Cc: akpm, hughd, aarcange, kirill.shutemov, dave.hansen,
	zhouchengming1, geliangtang, zhouxianrong, linux-kernel,
	zhouxiyu, wanghaijun5

From: z00281421 <z00281421@notesmail.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@notesmail.huawei.com>
---
 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,
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2] more mapcount page as kpage could reduce total replacement times than fewer mapcount one in probability.
@ 2016-06-15  1:56 ` zhouxianrong
  0 siblings, 0 replies; 10+ messages in thread
From: zhouxianrong @ 2016-06-15  1:56 UTC (permalink / raw)
  To: linux-mm
  Cc: akpm, hughd, aarcange, kirill.shutemov, dave.hansen,
	zhouchengming1, geliangtang, zhouxianrong, linux-kernel,
	zhouxiyu, wanghaijun5

From: z00281421 <z00281421@notesmail.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@notesmail.huawei.com>
---
 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,
-- 
1.7.9.5

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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] more mapcount page as kpage could reduce total replacement times than fewer mapcount one in probability.
  2016-06-15  1:56 ` zhouxianrong
@ 2016-06-21  1:57   ` zhouxianrong
  -1 siblings, 0 replies; 10+ messages in thread
From: zhouxianrong @ 2016-06-21  1:57 UTC (permalink / raw)
  To: linux-mm
  Cc: akpm, hughd, aarcange, kirill.shutemov, dave.hansen,
	zhouchengming1, geliangtang, linux-kernel, zhouxiyu, wanghaijun5

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

On 2016/6/15 9:56, zhouxianrong@huawei.com wrote:
> From: z00281421 <z00281421@notesmail.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@notesmail.huawei.com>
> ---
>  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,
>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] more mapcount page as kpage could reduce total replacement times than fewer mapcount one in probability.
@ 2016-06-21  1:57   ` zhouxianrong
  0 siblings, 0 replies; 10+ messages in thread
From: zhouxianrong @ 2016-06-21  1:57 UTC (permalink / raw)
  To: linux-mm
  Cc: akpm, hughd, aarcange, kirill.shutemov, dave.hansen,
	zhouchengming1, geliangtang, linux-kernel, zhouxiyu, wanghaijun5

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

On 2016/6/15 9:56, zhouxianrong@huawei.com wrote:
> From: z00281421 <z00281421@notesmail.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@notesmail.huawei.com>
> ---
>  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,
>

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] more mapcount page as kpage could reduce total replacement times than fewer mapcount one in probability.
  2016-06-21  1:57   ` zhouxianrong
@ 2016-06-22  1:39     ` Hugh Dickins
  -1 siblings, 0 replies; 10+ messages in thread
From: Hugh Dickins @ 2016-06-22  1:39 UTC (permalink / raw)
  To: zhouxianrong
  Cc: linux-mm, akpm, hughd, aarcange, kirill.shutemov, dave.hansen,
	zhouchengming1, geliangtang, linux-kernel, zhouxiyu, wanghaijun5

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@huawei.com wrote:
> > From: z00281421 <z00281421@notesmail.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@notesmail.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,
> > 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] more mapcount page as kpage could reduce total replacement times than fewer mapcount one in probability.
@ 2016-06-22  1:39     ` Hugh Dickins
  0 siblings, 0 replies; 10+ messages in thread
From: Hugh Dickins @ 2016-06-22  1:39 UTC (permalink / raw)
  To: zhouxianrong
  Cc: linux-mm, akpm, hughd, aarcange, kirill.shutemov, dave.hansen,
	zhouchengming1, geliangtang, linux-kernel, zhouxiyu, wanghaijun5

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@huawei.com wrote:
> > From: z00281421 <z00281421@notesmail.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@notesmail.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,
> > 

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] more mapcount page as kpage could reduce total replacement times than fewer mapcount one in probability.
  2016-06-22  1:39     ` Hugh Dickins
@ 2016-06-22 11:00       ` zhouxianrong
  -1 siblings, 0 replies; 10+ messages in thread
From: zhouxianrong @ 2016-06-22 11:00 UTC (permalink / raw)
  To: Hugh Dickins
  Cc: linux-mm, akpm, aarcange, kirill.shutemov, dave.hansen,
	zhouchengming1, geliangtang, linux-kernel, zhouxiyu, wanghaijun5

hey hugh:
	thank you for your reply and supplied more information.
i spent lots of time to study your questions mentioned.
i state my idea in detail, for exmple,
page1 with map count 7 and page2 with map count 9 are equal.
choosing page1 for ksm page will do 9 times heavily merg and 6 times lightly merg
choosing page2 for ksm page will do 7 times heavily merg and 8 times lightly merg
so choose page1 for ksm page.

the modification that swapping them around underneath try_to_merge_two_pages still has problem.
i am not going to modify the patch but i hope you would consider the idea later.

i have watched the problem about unmapping the more mapcount ksm page.
another case is that, for a more mapcount ksm page, not all of the forked page has been lightly merg
only with a rmap_item in diffrent mm and vma for now, so we can not find all the forked vmas via reverse map,
in which case we can not unmap the ksm page completely. so such ksm page could not be reclaimed or migrated.


i think the page being mapped also into non-VM_MERGEABLE areas due to MADV_MERGEABLE would meet same problem mentioned above.
that is we can not find all the forked vmas via reverse map.

is this a problem ? do you think about this ?



On 2016/6/22 9:39, Hugh Dickins wrote:
> 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@huawei.com wrote:
>>> From: z00281421 <z00281421@notesmail.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@notesmail.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,
>>>
>
> .
>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] more mapcount page as kpage could reduce total replacement times than fewer mapcount one in probability.
@ 2016-06-22 11:00       ` zhouxianrong
  0 siblings, 0 replies; 10+ messages in thread
From: zhouxianrong @ 2016-06-22 11:00 UTC (permalink / raw)
  To: Hugh Dickins
  Cc: linux-mm, akpm, aarcange, kirill.shutemov, dave.hansen,
	zhouchengming1, geliangtang, linux-kernel, zhouxiyu, wanghaijun5

hey hugh:
	thank you for your reply and supplied more information.
i spent lots of time to study your questions mentioned.
i state my idea in detail, for exmple,
page1 with map count 7 and page2 with map count 9 are equal.
choosing page1 for ksm page will do 9 times heavily merg and 6 times lightly merg
choosing page2 for ksm page will do 7 times heavily merg and 8 times lightly merg
so choose page1 for ksm page.

the modification that swapping them around underneath try_to_merge_two_pages still has problem.
i am not going to modify the patch but i hope you would consider the idea later.

i have watched the problem about unmapping the more mapcount ksm page.
another case is that, for a more mapcount ksm page, not all of the forked page has been lightly merg
only with a rmap_item in diffrent mm and vma for now, so we can not find all the forked vmas via reverse map,
in which case we can not unmap the ksm page completely. so such ksm page could not be reclaimed or migrated.


i think the page being mapped also into non-VM_MERGEABLE areas due to MADV_MERGEABLE would meet same problem mentioned above.
that is we can not find all the forked vmas via reverse map.

is this a problem ? do you think about this ?



On 2016/6/22 9:39, Hugh Dickins wrote:
> 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@huawei.com wrote:
>>> From: z00281421 <z00281421@notesmail.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@notesmail.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,
>>>
>
> .
>

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] more mapcount page as kpage could reduce total replacement times than fewer mapcount one in probability.
  2016-06-22  1:39     ` Hugh Dickins
@ 2016-07-20  1:03       ` Hugh Dickins
  -1 siblings, 0 replies; 10+ messages in thread
From: Hugh Dickins @ 2016-07-20  1:03 UTC (permalink / raw)
  To: zhouxianrong
  Cc: akpm, hughd, aarcange, kirill.shutemov, dave.hansen,
	zhouchengming1, geliangtang, zhouxiyu, wanghaijun5, linux-kernel,
	linux-mm

On Tue, 21 Jun 2016, Hugh Dickins wrote:
> 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@huawei.com wrote:
> > > From: z00281421 <z00281421@notesmail.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@notesmail.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);
> > > +	}
> > > +

I gave this a try, but commenting out the condition to make it exchange
the pages every time, to make it more likely to generate problems if any.

It very soon gave me lots of "BUG: Bad page" messages: presumably because
of the point I already made, that cmp_and_merge_page() knows which page
is which, but you've done nothing to tell it of the exchange.

So NAK to this patch as it stands.  No doubt easily corrected, but I
think this can only ever be a minor optimization - the very next page
to be merged with these two may turn out to have a much higher mapcount
than either of the first two.

I think it's better to drop this patch for now: as our other mail thread
indicates, there are more important things for us to worry about in KSM,
and their fixes may change around what's required here anyway.

Hugh

> > >  	err = try_to_merge_with_ksm_page(rmap_item, page, NULL);
> > >  	if (!err) {
> > >  		err = try_to_merge_with_ksm_page(tree_rmap_item,

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] more mapcount page as kpage could reduce total replacement times than fewer mapcount one in probability.
@ 2016-07-20  1:03       ` Hugh Dickins
  0 siblings, 0 replies; 10+ messages in thread
From: Hugh Dickins @ 2016-07-20  1:03 UTC (permalink / raw)
  To: zhouxianrong
  Cc: akpm, hughd, aarcange, kirill.shutemov, dave.hansen,
	zhouchengming1, geliangtang, zhouxiyu, wanghaijun5, linux-kernel,
	linux-mm

On Tue, 21 Jun 2016, Hugh Dickins wrote:
> 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@huawei.com wrote:
> > > From: z00281421 <z00281421@notesmail.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@notesmail.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);
> > > +	}
> > > +

I gave this a try, but commenting out the condition to make it exchange
the pages every time, to make it more likely to generate problems if any.

It very soon gave me lots of "BUG: Bad page" messages: presumably because
of the point I already made, that cmp_and_merge_page() knows which page
is which, but you've done nothing to tell it of the exchange.

So NAK to this patch as it stands.  No doubt easily corrected, but I
think this can only ever be a minor optimization - the very next page
to be merged with these two may turn out to have a much higher mapcount
than either of the first two.

I think it's better to drop this patch for now: as our other mail thread
indicates, there are more important things for us to worry about in KSM,
and their fixes may change around what's required here anyway.

Hugh

> > >  	err = try_to_merge_with_ksm_page(rmap_item, page, NULL);
> > >  	if (!err) {
> > >  		err = try_to_merge_with_ksm_page(tree_rmap_item,

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2016-07-20  1:03 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-15  1:56 [PATCH v2] more mapcount page as kpage could reduce total replacement times than fewer mapcount one in probability zhouxianrong
2016-06-15  1:56 ` zhouxianrong
2016-06-21  1:57 ` zhouxianrong
2016-06-21  1:57   ` zhouxianrong
2016-06-22  1:39   ` Hugh Dickins
2016-06-22  1:39     ` Hugh Dickins
2016-06-22 11:00     ` zhouxianrong
2016-06-22 11:00       ` zhouxianrong
2016-07-20  1:03     ` Hugh Dickins
2016-07-20  1:03       ` Hugh Dickins

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.