All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] mm/mmap.c: Remove useless statement "vma = NULL" in find_vma()
@ 2015-09-05 14:03 Chen Gang
  2015-09-07 12:36   ` Oleg Nesterov
  2015-09-08 23:14   ` David Rientjes
  0 siblings, 2 replies; 17+ messages in thread
From: Chen Gang @ 2015-09-05 14:03 UTC (permalink / raw)
  To: Andrew Morton, kirill.shutemov, riel, Michal Hocko, oleg,
	sasha.levin, pfeiner, aarcange, vishnu.ps, Linux Memory,
	kernel mailing list

[-- Attachment #1: Type: text/plain, Size: 786 bytes --]


>From b12fa5a9263cf4c044988e59f0071f4bcc132215 Mon Sep 17 00:00:00 2001
From: Chen Gang <gang.chen.5i5j@gmail.com>
Date: Sat, 5 Sep 2015 21:49:56 +0800
Subject: [PATCH] mm/mmap.c: Remove useless statement "vma = NULL" in
 find_vma()

Before the main looping, vma is already is NULL, so need not set it to
NULL, again.

Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com>
---
 mm/mmap.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/mm/mmap.c b/mm/mmap.c
index df6d5f0..4db7cf0 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -2054,7 +2054,6 @@ struct vm_area_struct *find_vma(struct mm_struct *mm, unsigned long addr)
 		return vma;
 
 	rb_node = mm->mm_rb.rb_node;
-	vma = NULL;
 
 	while (rb_node) {
 		struct vm_area_struct *tmp;
-- 
1.9.3

 		 	   		  

[-- Attachment #2: 0001-mm-mmap.c-Remove-useless-statement-vma-NULL-in-find_.patch --]
[-- Type: application/octet-stream, Size: 746 bytes --]

From b12fa5a9263cf4c044988e59f0071f4bcc132215 Mon Sep 17 00:00:00 2001
From: Chen Gang <gang.chen.5i5j@gmail.com>
Date: Sat, 5 Sep 2015 21:49:56 +0800
Subject: [PATCH 1/2] mm/mmap.c: Remove useless statement "vma = NULL" in
 find_vma()

Before the main looping, vma is already is NULL, so need not set it to
NULL, again.

Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com>
---
 mm/mmap.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/mm/mmap.c b/mm/mmap.c
index df6d5f0..4db7cf0 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -2054,7 +2054,6 @@ struct vm_area_struct *find_vma(struct mm_struct *mm, unsigned long addr)
 		return vma;
 
 	rb_node = mm->mm_rb.rb_node;
-	vma = NULL;
 
 	while (rb_node) {
 		struct vm_area_struct *tmp;
-- 
1.9.3


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

* Re: [PATCH] mm/mmap.c: Remove useless statement "vma = NULL" in find_vma()
  2015-09-05 14:03 [PATCH] mm/mmap.c: Remove useless statement "vma = NULL" in find_vma() Chen Gang
@ 2015-09-07 12:36   ` Oleg Nesterov
  2015-09-08 23:14   ` David Rientjes
  1 sibling, 0 replies; 17+ messages in thread
From: Oleg Nesterov @ 2015-09-07 12:36 UTC (permalink / raw)
  To: Chen Gang
  Cc: Andrew Morton, kirill.shutemov, riel, Michal Hocko, sasha.levin,
	pfeiner, aarcange, vishnu.ps, Linux Memory, kernel mailing list

On 09/05, Chen Gang wrote:
>
> From b12fa5a9263cf4c044988e59f0071f4bcc132215 Mon Sep 17 00:00:00 2001
> From: Chen Gang <gang.chen.5i5j@gmail.com>
> Date: Sat, 5 Sep 2015 21:49:56 +0800
> Subject: [PATCH] mm/mmap.c: Remove useless statement "vma = NULL" in
>  find_vma()
>
> Before the main looping, vma is already is NULL, so need not set it to
> NULL, again.
>
> Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com>

Reviewed-by: Oleg Nesterov <oleg@redhat.com>

> ---
>  mm/mmap.c | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/mm/mmap.c b/mm/mmap.c
> index df6d5f0..4db7cf0 100644
> --- a/mm/mmap.c
> +++ b/mm/mmap.c
> @@ -2054,7 +2054,6 @@ struct vm_area_struct *find_vma(struct mm_struct *mm, unsigned long addr)
>  		return vma;
>  
>  	rb_node = mm->mm_rb.rb_node;
> -	vma = NULL;
>  
>  	while (rb_node) {
>  		struct vm_area_struct *tmp;
> -- 
> 1.9.3
>
>



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

* Re: [PATCH] mm/mmap.c: Remove useless statement "vma = NULL" in find_vma()
@ 2015-09-07 12:36   ` Oleg Nesterov
  0 siblings, 0 replies; 17+ messages in thread
From: Oleg Nesterov @ 2015-09-07 12:36 UTC (permalink / raw)
  To: Chen Gang
  Cc: Andrew Morton, kirill.shutemov, riel, Michal Hocko, sasha.levin,
	pfeiner, aarcange, vishnu.ps, Linux Memory, kernel mailing list

On 09/05, Chen Gang wrote:
>
> From b12fa5a9263cf4c044988e59f0071f4bcc132215 Mon Sep 17 00:00:00 2001
> From: Chen Gang <gang.chen.5i5j@gmail.com>
> Date: Sat, 5 Sep 2015 21:49:56 +0800
> Subject: [PATCH] mm/mmap.c: Remove useless statement "vma = NULL" in
>  find_vma()
>
> Before the main looping, vma is already is NULL, so need not set it to
> NULL, again.
>
> Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com>

Reviewed-by: Oleg Nesterov <oleg@redhat.com>

> ---
>  mm/mmap.c | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/mm/mmap.c b/mm/mmap.c
> index df6d5f0..4db7cf0 100644
> --- a/mm/mmap.c
> +++ b/mm/mmap.c
> @@ -2054,7 +2054,6 @@ struct vm_area_struct *find_vma(struct mm_struct *mm, unsigned long addr)
>  		return vma;
>  
>  	rb_node = mm->mm_rb.rb_node;
> -	vma = NULL;
>  
>  	while (rb_node) {
>  		struct vm_area_struct *tmp;
> -- 
> 1.9.3
>
>


--
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] 17+ messages in thread

* Re: [PATCH] mm/mmap.c: Remove useless statement "vma = NULL" in find_vma()
       [not found]   ` <55EEED66.6090509@hotmail.com>
@ 2015-09-08 14:14       ` Chen Gang
  0 siblings, 0 replies; 17+ messages in thread
From: Chen Gang @ 2015-09-08 14:14 UTC (permalink / raw)
  To: oleg
  Cc: Andrew Morton, kirill.shutemov, riel, Michal Hocko, sasha.levin,
	pfeiner, aarcange, vishnu.ps, Linux Memory, kernel mailing list

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="gb2312", Size: 1539 bytes --]

On 9/7/15 20:36, Oleg Nesterov wrote:
> On 09/05, Chen Gang wrote:
>>
>> From b12fa5a9263cf4c044988e59f0071f4bcc132215 Mon Sep 17 00:00:00 2001
>> From: Chen Gang <gang.chen.5i5j@gmail.com>
>> Date: Sat, 5 Sep 2015 21:49:56 +0800
>> Subject: [PATCH] mm/mmap.c: Remove useless statement "vma = NULL" in
>> find_vma()
>>
>> Before the main looping, vma is already is NULL, so need not set it to
>> NULL, again.
>>
>> Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com>
>
> Reviewed-by: Oleg Nesterov <oleg@redhat.com>
>

OK, thanks.


I also want to consult: the comments of find_vma() says:

"Look up the first VMA which satisfies addr < vm_end, ..."

Is it OK? (why not "vm_start <= addr < vm_end"), need we let "vma = tmp"
in "if (tmp->vm_start <= addr)"? -- it looks the comments is not match
the implementation, precisely (maybe not 1st VMA).


Thanks.


>> ---
>> mm/mmap.c | 1 -
>> 1 file changed, 1 deletion(-)
>>
>> diff --git a/mm/mmap.c b/mm/mmap.c
>> index df6d5f0..4db7cf0 100644
>> --- a/mm/mmap.c
>> +++ b/mm/mmap.c
>> @@ -2054,7 +2054,6 @@ struct vm_area_struct *find_vma(struct mm_struct *mm, unsigned long addr)
>> return vma;
>>
>> rb_node = mm->mm_rb.rb_node;
>> - vma = NULL;
>>
>> while (rb_node) {
>> struct vm_area_struct *tmp;
>> --
>> 1.9.3
>>
>>
>
>

--
Chen Gang (³Â¸Õ)

Open, share, and attitude like air, water, and life which God blessed
 		 	   		  ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

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

* Re: [PATCH] mm/mmap.c: Remove useless statement "vma = NULL" in find_vma()
@ 2015-09-08 14:14       ` Chen Gang
  0 siblings, 0 replies; 17+ messages in thread
From: Chen Gang @ 2015-09-08 14:14 UTC (permalink / raw)
  To: oleg
  Cc: Andrew Morton, kirill.shutemov, riel, Michal Hocko, sasha.levin,
	pfeiner, aarcange, vishnu.ps, Linux Memory, kernel mailing list

On 9/7/15 20:36, Oleg Nesterov wrote:
> On 09/05, Chen Gang wrote:
>>
>> From b12fa5a9263cf4c044988e59f0071f4bcc132215 Mon Sep 17 00:00:00 2001
>> From: Chen Gang <gang.chen.5i5j@gmail.com>
>> Date: Sat, 5 Sep 2015 21:49:56 +0800
>> Subject: [PATCH] mm/mmap.c: Remove useless statement "vma = NULL" in
>> find_vma()
>>
>> Before the main looping, vma is already is NULL, so need not set it to
>> NULL, again.
>>
>> Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com>
>
> Reviewed-by: Oleg Nesterov <oleg@redhat.com>
>

OK, thanks.


I also want to consult: the comments of find_vma() says:

"Look up the first VMA which satisfies addr < vm_end, ..."

Is it OK? (why not "vm_start <= addr < vm_end"), need we let "vma = tmp"
in "if (tmp->vm_start <= addr)"? -- it looks the comments is not match
the implementation, precisely (maybe not 1st VMA).


Thanks.


>> ---
>> mm/mmap.c | 1 -
>> 1 file changed, 1 deletion(-)
>>
>> diff --git a/mm/mmap.c b/mm/mmap.c
>> index df6d5f0..4db7cf0 100644
>> --- a/mm/mmap.c
>> +++ b/mm/mmap.c
>> @@ -2054,7 +2054,6 @@ struct vm_area_struct *find_vma(struct mm_struct *mm, unsigned long addr)
>> return vma;
>>
>> rb_node = mm->mm_rb.rb_node;
>> - vma = NULL;
>>
>> while (rb_node) {
>> struct vm_area_struct *tmp;
>> --
>> 1.9.3
>>
>>
>
>

--
Chen Gang (陈刚)

Open, share, and attitude like air, water, and life which God blessed
 		 	   		  

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

* Re: [PATCH] mm/mmap.c: Remove useless statement "vma = NULL" in  find_vma()
  2015-09-05 14:03 [PATCH] mm/mmap.c: Remove useless statement "vma = NULL" in find_vma() Chen Gang
@ 2015-09-08 23:14   ` David Rientjes
  2015-09-08 23:14   ` David Rientjes
  1 sibling, 0 replies; 17+ messages in thread
From: David Rientjes @ 2015-09-08 23:14 UTC (permalink / raw)
  To: Chen Gang
  Cc: Andrew Morton, kirill.shutemov, riel, Michal Hocko, oleg,
	sasha.levin, pfeiner, aarcange, vishnu.ps, Linux Memory,
	kernel mailing list

[-- Attachment #1: Type: TEXT/PLAIN, Size: 478 bytes --]

On Sat, 5 Sep 2015, Chen Gang wrote:

> 
> From b12fa5a9263cf4c044988e59f0071f4bcc132215 Mon Sep 17 00:00:00 2001
> From: Chen Gang <gang.chen.5i5j@gmail.com>
> Date: Sat, 5 Sep 2015 21:49:56 +0800
> Subject: [PATCH] mm/mmap.c: Remove useless statement "vma = NULL" in
>  find_vma()
> 
> Before the main looping, vma is already is NULL, so need not set it to
> NULL, again.
> 
> Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com>

Acked-by: David Rientjes <rientjes@google.com>

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

* Re: [PATCH] mm/mmap.c: Remove useless statement "vma = NULL" in find_vma()
@ 2015-09-08 23:14   ` David Rientjes
  0 siblings, 0 replies; 17+ messages in thread
From: David Rientjes @ 2015-09-08 23:14 UTC (permalink / raw)
  To: Chen Gang
  Cc: Andrew Morton, kirill.shutemov, riel, Michal Hocko, oleg,
	sasha.levin, pfeiner, aarcange, vishnu.ps, Linux Memory,
	kernel mailing list

[-- Attachment #1: Type: TEXT/PLAIN, Size: 478 bytes --]

On Sat, 5 Sep 2015, Chen Gang wrote:

> 
> From b12fa5a9263cf4c044988e59f0071f4bcc132215 Mon Sep 17 00:00:00 2001
> From: Chen Gang <gang.chen.5i5j@gmail.com>
> Date: Sat, 5 Sep 2015 21:49:56 +0800
> Subject: [PATCH] mm/mmap.c: Remove useless statement "vma = NULL" in
>  find_vma()
> 
> Before the main looping, vma is already is NULL, so need not set it to
> NULL, again.
> 
> Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com>

Acked-by: David Rientjes <rientjes@google.com>

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

* Re: [PATCH] mm/mmap.c: Remove useless statement "vma = NULL" in find_vma()
  2015-09-08 14:14       ` Chen Gang
@ 2015-09-09 16:26         ` Oleg Nesterov
  -1 siblings, 0 replies; 17+ messages in thread
From: Oleg Nesterov @ 2015-09-09 16:26 UTC (permalink / raw)
  To: Chen Gang
  Cc: Andrew Morton, kirill.shutemov, riel, Michal Hocko, sasha.levin,
	pfeiner, aarcange, vishnu.ps, Linux Memory, kernel mailing list

On 09/08, Chen Gang wrote:
>
> I also want to consult: the comments of find_vma() says:

Sorry, I don't understand the question ;)

> "Look up the first VMA which satisfies addr < vm_end, ..."
>
> Is it OK?

Why not?

> (why not "vm_start <= addr < vm_end"),

Because this some callers actually want to find the 1st vma which
satisfies addr < vm_end? For example, shift_arg_pages().

OTOH, I think that another helper,

	find_vma_xxx(mm, addr)
	{
		vma = find_vma(...)
		if (vma && vma->vm_start > addr)
			vma = NULL;
		return vma;
	}

makes sense. It can have a lot of users.

> need we let "vma = tmp"
> in "if (tmp->vm_start <= addr)"? -- it looks the comments is not match
> the implementation, precisely (maybe not 1st VMA).

This contradicts with above... I mean, it is not clear what exactly do
you blame, semantics or implementation.

The implementation looks correct. Why do you think it can be not 1st vma?

Oleg.


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

* Re: [PATCH] mm/mmap.c: Remove useless statement "vma = NULL" in find_vma()
@ 2015-09-09 16:26         ` Oleg Nesterov
  0 siblings, 0 replies; 17+ messages in thread
From: Oleg Nesterov @ 2015-09-09 16:26 UTC (permalink / raw)
  To: Chen Gang
  Cc: Andrew Morton, kirill.shutemov, riel, Michal Hocko, sasha.levin,
	pfeiner, aarcange, vishnu.ps, Linux Memory, kernel mailing list

On 09/08, Chen Gang wrote:
>
> I also want to consult: the comments of find_vma() says:

Sorry, I don't understand the question ;)

> "Look up the first VMA which satisfies addr < vm_end, ..."
>
> Is it OK?

Why not?

> (why not "vm_start <= addr < vm_end"),

Because this some callers actually want to find the 1st vma which
satisfies addr < vm_end? For example, shift_arg_pages().

OTOH, I think that another helper,

	find_vma_xxx(mm, addr)
	{
		vma = find_vma(...)
		if (vma && vma->vm_start > addr)
			vma = NULL;
		return vma;
	}

makes sense. It can have a lot of users.

> need we let "vma = tmp"
> in "if (tmp->vm_start <= addr)"? -- it looks the comments is not match
> the implementation, precisely (maybe not 1st VMA).

This contradicts with above... I mean, it is not clear what exactly do
you blame, semantics or implementation.

The implementation looks correct. Why do you think it can be not 1st vma?

Oleg.

--
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] 17+ messages in thread

* Re: [PATCH] mm/mmap.c: Remove useless statement "vma = NULL" in find_vma()
       [not found]         ` <55F0B6C2.2000706@hotmail.com>
@ 2015-09-09 22:44             ` Chen Gang
  0 siblings, 0 replies; 17+ messages in thread
From: Chen Gang @ 2015-09-09 22:44 UTC (permalink / raw)
  To: oleg
  Cc: Andrew Morton, kirill.shutemov, riel, Michal Hocko, sasha.levin,
	pfeiner, aarcange, vishnu.ps, Linux Memory, kernel mailing list

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="gb2312", Size: 2049 bytes --]


On 9/10/15 00:26, Oleg Nesterov wrote:
> On 09/08, Chen Gang wrote:
>>
>> I also want to consult: the comments of find_vma() says:
>
> Sorry, I don't understand the question ;)
>
>> "Look up the first VMA which satisfies addr < vm_end, ..."
>>
>> Is it OK?
>
> Why not?
>

We will continue discuss about it below. Please help check, thanks.

>> (why not "vm_start <= addr < vm_end"),
>
> Because this some callers actually want to find the 1st vma which
> satisfies addr < vm_end? For example, shift_arg_pages().
>
> OTOH, I think that another helper,
>
> find_vma_xxx(mm, addr)
> {
> vma = find_vma(...)
> if (vma && vma->vm_start> addr)
> vma = NULL;
> return vma;
> }
>
> makes sense. It can have a lot of users.
>

OK. thank you very much. :-)

>> need we let "vma = tmp"
>> in "if (tmp->vm_start <= addr)"? -- it looks the comments is not match
>> the implementation, precisely (maybe not 1st VMA).
>
> This contradicts with above... I mean, it is not clear what exactly do
> you blame, semantics or implementation.
>
> The implementation looks correct. Why do you think it can be not 1st vma?
>

It is in while (rb_node) {...}.

- When we set "vma = tmp", it is alreay match "addr < vm_end".

- If "addr>= vm_start", we return this vma (else continue searching).

If "the first left" is the real first, when "addr>= vm_start", it
will return (may not return 1st left matched vma).

If "the first find" is the real first, when "addr < vm_start", it
will continue searching (may not return 1st find matched vma).

For me, if we only focus on "addr < vm_end", we need remove "vm_start <=
addr" checking). If we have to consider about "addr>= vm_start", we may
need additional parameter or implement a new function for it.


Welcome any ideas, suggestions and completions.


Thanks.
--
Chen Gang (³Â¸Õ)

Open, share, and attitude like air, water, and life which God blessed
 		 	   		  ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

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

* Re: [PATCH] mm/mmap.c: Remove useless statement "vma = NULL" in find_vma()
@ 2015-09-09 22:44             ` Chen Gang
  0 siblings, 0 replies; 17+ messages in thread
From: Chen Gang @ 2015-09-09 22:44 UTC (permalink / raw)
  To: oleg
  Cc: Andrew Morton, kirill.shutemov, riel, Michal Hocko, sasha.levin,
	pfeiner, aarcange, vishnu.ps, Linux Memory, kernel mailing list


On 9/10/15 00:26, Oleg Nesterov wrote:
> On 09/08, Chen Gang wrote:
>>
>> I also want to consult: the comments of find_vma() says:
>
> Sorry, I don't understand the question ;)
>
>> "Look up the first VMA which satisfies addr < vm_end, ..."
>>
>> Is it OK?
>
> Why not?
>

We will continue discuss about it below. Please help check, thanks.

>> (why not "vm_start <= addr < vm_end"),
>
> Because this some callers actually want to find the 1st vma which
> satisfies addr < vm_end? For example, shift_arg_pages().
>
> OTOH, I think that another helper,
>
> find_vma_xxx(mm, addr)
> {
> vma = find_vma(...)
> if (vma && vma->vm_start> addr)
> vma = NULL;
> return vma;
> }
>
> makes sense. It can have a lot of users.
>

OK. thank you very much. :-)

>> need we let "vma = tmp"
>> in "if (tmp->vm_start <= addr)"? -- it looks the comments is not match
>> the implementation, precisely (maybe not 1st VMA).
>
> This contradicts with above... I mean, it is not clear what exactly do
> you blame, semantics or implementation.
>
> The implementation looks correct. Why do you think it can be not 1st vma?
>

It is in while (rb_node) {...}.

- When we set "vma = tmp", it is alreay match "addr < vm_end".

- If "addr>= vm_start", we return this vma (else continue searching).

If "the first left" is the real first, when "addr>= vm_start", it
will return (may not return 1st left matched vma).

If "the first find" is the real first, when "addr < vm_start", it
will continue searching (may not return 1st find matched vma).

For me, if we only focus on "addr < vm_end", we need remove "vm_start <=
addr" checking). If we have to consider about "addr>= vm_start", we may
need additional parameter or implement a new function for it.


Welcome any ideas, suggestions and completions.


Thanks.
--
Chen Gang (陈刚)

Open, share, and attitude like air, water, and life which God blessed
 		 	   		  

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

* Re: [PATCH] mm/mmap.c: Remove useless statement "vma = NULL" in find_vma()
  2015-09-09 22:44             ` Chen Gang
@ 2015-09-10 18:19               ` Oleg Nesterov
  -1 siblings, 0 replies; 17+ messages in thread
From: Oleg Nesterov @ 2015-09-10 18:19 UTC (permalink / raw)
  To: Chen Gang
  Cc: Andrew Morton, kirill.shutemov, riel, Michal Hocko, sasha.levin,
	pfeiner, aarcange, vishnu.ps, Linux Memory, kernel mailing list

On 09/10, Chen Gang wrote:
>
> On 9/10/15 00:26, Oleg Nesterov wrote:
> >
> > The implementation looks correct. Why do you think it can be not 1st vma?
> >
>
> It is in while (rb_node) {...}.
>
> - When we set "vma = tmp", it is alreay match "addr < vm_end".

Yes,

> - If "addr>= vm_start", we return this vma (else continue searching).

This is optimization, we can stop the search because in this case
vma == tmp is obviously the 1st vma with "addr < vm_end".

I simply can't understand your concerns. Perhaps you can make a
patch, then it will be more clear what me-or-you have missed.

Oleg.


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

* Re: [PATCH] mm/mmap.c: Remove useless statement "vma = NULL" in find_vma()
@ 2015-09-10 18:19               ` Oleg Nesterov
  0 siblings, 0 replies; 17+ messages in thread
From: Oleg Nesterov @ 2015-09-10 18:19 UTC (permalink / raw)
  To: Chen Gang
  Cc: Andrew Morton, kirill.shutemov, riel, Michal Hocko, sasha.levin,
	pfeiner, aarcange, vishnu.ps, Linux Memory, kernel mailing list

On 09/10, Chen Gang wrote:
>
> On 9/10/15 00:26, Oleg Nesterov wrote:
> >
> > The implementation looks correct. Why do you think it can be not 1st vma?
> >
>
> It is in while (rb_node) {...}.
>
> - When we set "vma = tmp", it is alreay match "addr < vm_end".

Yes,

> - If "addr>= vm_start", we return this vma (else continue searching).

This is optimization, we can stop the search because in this case
vma == tmp is obviously the 1st vma with "addr < vm_end".

I simply can't understand your concerns. Perhaps you can make a
patch, then it will be more clear what me-or-you have missed.

Oleg.

--
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] 17+ messages in thread

* Re: [PATCH] mm/mmap.c: Remove useless statement "vma = NULL" in find_vma()
  2015-09-10 18:19               ` Oleg Nesterov
@ 2015-09-10 22:20                 ` Chen Gang
  -1 siblings, 0 replies; 17+ messages in thread
From: Chen Gang @ 2015-09-10 22:20 UTC (permalink / raw)
  To: Oleg Nesterov
  Cc: Andrew Morton, kirill.shutemov, riel, Michal Hocko, sasha.levin,
	pfeiner, aarcange, vishnu.ps, Linux Memory, kernel mailing list

On 9/11/15 02:19, Oleg Nesterov wrote:
> On 09/10, Chen Gang wrote:
>> - If "addr>= vm_start", we return this vma (else continue searching).
> 
> This is optimization, we can stop the search because in this case
> vma == tmp is obviously the 1st vma with "addr < vm_end".
> 

OK, thanks.

I guess if we have additional comments for "if (tmp->vm_start <= addr)",
the code will be more readable for readers (especially for newbies).

@@ -2064,7 +2064,7 @@ struct vm_area_struct *find_vma(struct mm_struct *mm, unsigned long addr)
                if (tmp->vm_end > addr) {
                        vma = tmp;
                        if (tmp->vm_start <= addr)
-                               break;
+                               break; /* It must be 1st "addr < vm_end" */
                        rb_node = rb_node->rb_left;
                } else
                        rb_node = rb_node->rb_right;


> I simply can't understand your concerns. Perhaps you can make a
> patch, then it will be more clear what me-or-you have missed.
> 

I guess, we need not (it is my missing). :-)


Thanks.
-- 
Chen Gang (陈刚)

Open, share, and attitude like air, water, and life which God blessed

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

* Re: [PATCH] mm/mmap.c: Remove useless statement "vma = NULL" in find_vma()
@ 2015-09-10 22:20                 ` Chen Gang
  0 siblings, 0 replies; 17+ messages in thread
From: Chen Gang @ 2015-09-10 22:20 UTC (permalink / raw)
  To: Oleg Nesterov
  Cc: Andrew Morton, kirill.shutemov, riel, Michal Hocko, sasha.levin,
	pfeiner, aarcange, vishnu.ps, Linux Memory, kernel mailing list

On 9/11/15 02:19, Oleg Nesterov wrote:
> On 09/10, Chen Gang wrote:
>> - If "addr>= vm_start", we return this vma (else continue searching).
> 
> This is optimization, we can stop the search because in this case
> vma == tmp is obviously the 1st vma with "addr < vm_end".
> 

OK, thanks.

I guess if we have additional comments for "if (tmp->vm_start <= addr)",
the code will be more readable for readers (especially for newbies).

@@ -2064,7 +2064,7 @@ struct vm_area_struct *find_vma(struct mm_struct *mm, unsigned long addr)
                if (tmp->vm_end > addr) {
                        vma = tmp;
                        if (tmp->vm_start <= addr)
-                               break;
+                               break; /* It must be 1st "addr < vm_end" */
                        rb_node = rb_node->rb_left;
                } else
                        rb_node = rb_node->rb_right;


> I simply can't understand your concerns. Perhaps you can make a
> patch, then it will be more clear what me-or-you have missed.
> 

I guess, we need not (it is my missing). :-)


Thanks.
-- 
Chen Gang (陈刚)

Open, share, and attitude like air, water, and life which God blessed

--
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] 17+ messages in thread

* Re: [PATCH] mm/mmap.c: Remove useless statement "vma = NULL" in find_vma()
  2015-09-03  3:52 gang.chen.5i5j
@ 2015-09-03  4:02 ` Chen Gang
  0 siblings, 0 replies; 17+ messages in thread
From: Chen Gang @ 2015-09-03  4:02 UTC (permalink / raw)
  To: gang.chen.5i5j, akpm, mhocko; +Cc: linux-mm, linux-kernel

Hello all:

I also want to consult: the comments of find_vma() says:

  "Look up the first VMA which satisfies  addr < vm_end, ..."

Is it OK? (why not "vm_start <= addr < vm_end"), need we let "vma = tmp"
in "if (tmp->vm_start <= addr)"? -- it looks the comments is not match
the implementation, precisely (maybe not 1st VMA).


Thanks.


On 9/3/15 11:52, gang.chen.5i5j@gmail.com wrote:
> From: Chen Gang <gang.chen.5i5j@gmail.com>
> 
> Before the main looping, vma is already is NULL, so need not set it to
> NULL, again.
> 
> Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com>
> ---
>  mm/mmap.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/mm/mmap.c b/mm/mmap.c
> index df6d5f0..4db7cf0 100644
> --- a/mm/mmap.c
> +++ b/mm/mmap.c
> @@ -2054,7 +2054,6 @@ struct vm_area_struct *find_vma(struct mm_struct *mm, unsigned long addr)
>  		return vma;
>  
>  	rb_node = mm->mm_rb.rb_node;
> -	vma = NULL;
>  
>  	while (rb_node) {
>  		struct vm_area_struct *tmp;
> 

-- 
Chen Gang (陈刚)

Open, share, and attitude like air, water, and life which God blessed

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

* [PATCH] mm/mmap.c: Remove useless statement "vma = NULL" in find_vma()
@ 2015-09-03  3:52 gang.chen.5i5j
  2015-09-03  4:02 ` Chen Gang
  0 siblings, 1 reply; 17+ messages in thread
From: gang.chen.5i5j @ 2015-09-03  3:52 UTC (permalink / raw)
  To: akpm, mhocko; +Cc: linux-mm, linux-kernel, gchen_5i5j, Chen Gang

From: Chen Gang <gang.chen.5i5j@gmail.com>

Before the main looping, vma is already is NULL, so need not set it to
NULL, again.

Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com>
---
 mm/mmap.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/mm/mmap.c b/mm/mmap.c
index df6d5f0..4db7cf0 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -2054,7 +2054,6 @@ struct vm_area_struct *find_vma(struct mm_struct *mm, unsigned long addr)
 		return vma;
 
 	rb_node = mm->mm_rb.rb_node;
-	vma = NULL;
 
 	while (rb_node) {
 		struct vm_area_struct *tmp;
-- 
1.9.3

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

end of thread, other threads:[~2015-09-10 22:18 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-09-05 14:03 [PATCH] mm/mmap.c: Remove useless statement "vma = NULL" in find_vma() Chen Gang
2015-09-07 12:36 ` Oleg Nesterov
2015-09-07 12:36   ` Oleg Nesterov
     [not found]   ` <55EEED66.6090509@hotmail.com>
2015-09-08 14:14     ` Chen Gang
2015-09-08 14:14       ` Chen Gang
2015-09-09 16:26       ` Oleg Nesterov
2015-09-09 16:26         ` Oleg Nesterov
     [not found]         ` <55F0B6C2.2000706@hotmail.com>
2015-09-09 22:44           ` Chen Gang
2015-09-09 22:44             ` Chen Gang
2015-09-10 18:19             ` Oleg Nesterov
2015-09-10 18:19               ` Oleg Nesterov
2015-09-10 22:20               ` Chen Gang
2015-09-10 22:20                 ` Chen Gang
2015-09-08 23:14 ` David Rientjes
2015-09-08 23:14   ` David Rientjes
  -- strict thread matches above, loose matches on Subject: below --
2015-09-03  3:52 gang.chen.5i5j
2015-09-03  4:02 ` Chen Gang

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.