* [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.