Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1220824 > unrolled thread

Re: [PATCH] mm/mmap.c: Remove useless statement "vma = NULL" in find_vma()

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH] mm/mmap.c: Remove useless statement "vma = NULL" in  find_vma() Chen Gang <xili_gchen_5257@hotmail.com> - 2015-09-08 16:20 +0200
    Re: [PATCH] mm/mmap.c: Remove useless statement "vma = NULL" in  find_vma() Oleg Nesterov <oleg@redhat.com> - 2015-09-09 18:30 +0200
    Re: [PATCH] mm/mmap.c: Remove useless statement "vma = NULL" in  find_vma() Chen Gang <xili_gchen_5257@hotmail.com> - 2015-09-10 00:50 +0200
      Re: [PATCH] mm/mmap.c: Remove useless statement "vma = NULL" in  find_vma() Oleg Nesterov <oleg@redhat.com> - 2015-09-10 20:30 +0200
        Re: [PATCH] mm/mmap.c: Remove useless statement "vma = NULL" in find_vma() Chen Gang <xili_gchen_5257@hotmail.com> - 2015-09-11 00:20 +0200

#1220824 — Re: [PATCH] mm/mmap.c: Remove useless statement "vma = NULL" in find_vma()

T24gOS83LzE1IDIwOjM2LCBPbGVnIE5lc3Rlcm92IHdyb3RlOgo+IE9uIDA5LzA1LCBDaGVuIEdh
bmcgd3JvdGU6Cj4+Cj4+IEZyb20gYjEyZmE1YTkyNjNjZjRjMDQ0OTg4ZTU5ZjAwNzFmNGJjYzEz
MjIxNSBNb24gU2VwIDE3IDAwOjAwOjAwIDIwMDEKPj4gRnJvbTogQ2hlbiBHYW5nIDxnYW5nLmNo
ZW4uNWk1akBnbWFpbC5jb20+Cj4+IERhdGU6IFNhdCwgNSBTZXAgMjAxNSAyMTo0OTo1NiArMDgw
MAo+PiBTdWJqZWN0OiBbUEFUQ0hdIG1tL21tYXAuYzogUmVtb3ZlIHVzZWxlc3Mgc3RhdGVtZW50
ICJ2bWEgPSBOVUxMIiBpbgo+PiBmaW5kX3ZtYSgpCj4+Cj4+IEJlZm9yZSB0aGUgbWFpbiBsb29w
aW5nLCB2bWEgaXMgYWxyZWFkeSBpcyBOVUxMLCBzbyBuZWVkIG5vdCBzZXQgaXQgdG8KPj4gTlVM
TCwgYWdhaW4uCj4+Cj4+IFNpZ25lZC1vZmYtYnk6IENoZW4gR2FuZyA8Z2FuZy5jaGVuLjVpNWpA
Z21haWwuY29tPgo+Cj4gUmV2aWV3ZWQtYnk6IE9sZWcgTmVzdGVyb3YgPG9sZWdAcmVkaGF0LmNv
bT4KPgoKT0ssIHRoYW5rcy4KCgpJIGFsc28gd2FudCB0byBjb25zdWx0OiB0aGUgY29tbWVudHMg
b2YgZmluZF92bWEoKSBzYXlzOgoKIkxvb2sgdXAgdGhlIGZpcnN0IFZNQSB3aGljaCBzYXRpc2Zp
ZXMgYWRkciA8IHZtX2VuZCwgLi4uIgoKSXMgaXQgT0s/ICh3aHkgbm90ICJ2bV9zdGFydCA8PSBh
ZGRyIDwgdm1fZW5kIiksIG5lZWQgd2UgbGV0ICJ2bWEgPSB0bXAiCmluICJpZiAodG1wLT52bV9z
dGFydCA8PSBhZGRyKSI/IC0tIGl0IGxvb2tzIHRoZSBjb21tZW50cyBpcyBub3QgbWF0Y2gKdGhl
IGltcGxlbWVudGF0aW9uLCBwcmVjaXNlbHkgKG1heWJlIG5vdCAxc3QgVk1BKS4KCgpUaGFua3Mu
CgoKPj4gLS0tCj4+IG1tL21tYXAuYyB8IDEgLQo+PiAxIGZpbGUgY2hhbmdlZCwgMSBkZWxldGlv
bigtKQo+Pgo+PiBkaWZmIC0tZ2l0IGEvbW0vbW1hcC5jIGIvbW0vbW1hcC5jCj4+IGluZGV4IGRm
NmQ1ZjAuLjRkYjdjZjAgMTAwNjQ0Cj4+IC0tLSBhL21tL21tYXAuYwo+PiArKysgYi9tbS9tbWFw
LmMKPj4gQEAgLTIwNTQsNyArMjA1NCw2IEBAIHN0cnVjdCB2bV9hcmVhX3N0cnVjdCAqZmluZF92
bWEoc3RydWN0IG1tX3N0cnVjdCAqbW0sIHVuc2lnbmVkIGxvbmcgYWRkcikKPj4gcmV0dXJuIHZt
YTsKPj4KPj4gcmJfbm9kZSA9IG1tLT5tbV9yYi5yYl9ub2RlOwo+PiAtIHZtYSA9IE5VTEw7Cj4+
Cj4+IHdoaWxlIChyYl9ub2RlKSB7Cj4+IHN0cnVjdCB2bV9hcmVhX3N0cnVjdCAqdG1wOwo+PiAt
LQo+PiAxLjkuMwo+Pgo+Pgo+Cj4KCi0tCkNoZW4gR2FuZyAos8K41SkKCk9wZW4sIHNoYXJlLCBh
bmQgYXR0aXR1ZGUgbGlrZSBhaXIsIHdhdGVyLCBhbmQgbGlmZSB3aGljaCBHb2QgYmxlc3NlZAog
CQkgCSAgIAkJICA=
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1221554

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 from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1221779

Ck9uIDkvMTAvMTUgMDA6MjYsIE9sZWcgTmVzdGVyb3Ygd3JvdGU6Cj4gT24gMDkvMDgsIENoZW4g
R2FuZyB3cm90ZToKPj4KPj4gSSBhbHNvIHdhbnQgdG8gY29uc3VsdDogdGhlIGNvbW1lbnRzIG9m
IGZpbmRfdm1hKCkgc2F5czoKPgo+IFNvcnJ5LCBJIGRvbid0IHVuZGVyc3RhbmQgdGhlIHF1ZXN0
aW9uIDspCj4KPj4gIkxvb2sgdXAgdGhlIGZpcnN0IFZNQSB3aGljaCBzYXRpc2ZpZXMgYWRkciA8
IHZtX2VuZCwgLi4uIgo+Pgo+PiBJcyBpdCBPSz8KPgo+IFdoeSBub3Q/Cj4KCldlIHdpbGwgY29u
dGludWUgZGlzY3VzcyBhYm91dCBpdCBiZWxvdy4gUGxlYXNlIGhlbHAgY2hlY2ssIHRoYW5rcy4K
Cj4+ICh3aHkgbm90ICJ2bV9zdGFydCA8PSBhZGRyIDwgdm1fZW5kIiksCj4KPiBCZWNhdXNlIHRo
aXMgc29tZSBjYWxsZXJzIGFjdHVhbGx5IHdhbnQgdG8gZmluZCB0aGUgMXN0IHZtYSB3aGljaAo+
IHNhdGlzZmllcyBhZGRyIDwgdm1fZW5kPyBGb3IgZXhhbXBsZSwgc2hpZnRfYXJnX3BhZ2VzKCku
Cj4KPiBPVE9ILCBJIHRoaW5rIHRoYXQgYW5vdGhlciBoZWxwZXIsCj4KPiBmaW5kX3ZtYV94eHgo
bW0sIGFkZHIpCj4gewo+IHZtYSA9IGZpbmRfdm1hKC4uLikKPiBpZiAodm1hICYmIHZtYS0+dm1f
c3RhcnQ+IGFkZHIpCj4gdm1hID0gTlVMTDsKPiByZXR1cm4gdm1hOwo+IH0KPgo+IG1ha2VzIHNl
bnNlLiBJdCBjYW4gaGF2ZSBhIGxvdCBvZiB1c2Vycy4KPgoKT0suIHRoYW5rIHlvdSB2ZXJ5IG11
Y2guIDotKQoKPj4gbmVlZCB3ZSBsZXQgInZtYSA9IHRtcCIKPj4gaW4gImlmICh0bXAtPnZtX3N0
YXJ0IDw9IGFkZHIpIj8gLS0gaXQgbG9va3MgdGhlIGNvbW1lbnRzIGlzIG5vdCBtYXRjaAo+PiB0
aGUgaW1wbGVtZW50YXRpb24sIHByZWNpc2VseSAobWF5YmUgbm90IDFzdCBWTUEpLgo+Cj4gVGhp
cyBjb250cmFkaWN0cyB3aXRoIGFib3ZlLi4uIEkgbWVhbiwgaXQgaXMgbm90IGNsZWFyIHdoYXQg
ZXhhY3RseSBkbwo+IHlvdSBibGFtZSwgc2VtYW50aWNzIG9yIGltcGxlbWVudGF0aW9uLgo+Cj4g
VGhlIGltcGxlbWVudGF0aW9uIGxvb2tzIGNvcnJlY3QuIFdoeSBkbyB5b3UgdGhpbmsgaXQgY2Fu
IGJlIG5vdCAxc3Qgdm1hPwo+CgpJdCBpcyBpbiB3aGlsZSAocmJfbm9kZSkgey4uLn0uCgotIFdo
ZW4gd2Ugc2V0ICJ2bWEgPSB0bXAiLCBpdCBpcyBhbHJlYXkgbWF0Y2ggImFkZHIgPCB2bV9lbmQi
LgoKLSBJZiAiYWRkcj49IHZtX3N0YXJ0Iiwgd2UgcmV0dXJuIHRoaXMgdm1hIChlbHNlIGNvbnRp
bnVlIHNlYXJjaGluZykuCgpJZiAidGhlIGZpcnN0IGxlZnQiIGlzIHRoZSByZWFsIGZpcnN0LCB3
aGVuICJhZGRyPj0gdm1fc3RhcnQiLCBpdAp3aWxsIHJldHVybiAobWF5IG5vdCByZXR1cm4gMXN0
IGxlZnQgbWF0Y2hlZCB2bWEpLgoKSWYgInRoZSBmaXJzdCBmaW5kIiBpcyB0aGUgcmVhbCBmaXJz
dCwgd2hlbiAiYWRkciA8IHZtX3N0YXJ0IiwgaXQKd2lsbCBjb250aW51ZSBzZWFyY2hpbmcgKG1h
eSBub3QgcmV0dXJuIDFzdCBmaW5kIG1hdGNoZWQgdm1hKS4KCkZvciBtZSwgaWYgd2Ugb25seSBm
b2N1cyBvbiAiYWRkciA8IHZtX2VuZCIsIHdlIG5lZWQgcmVtb3ZlICJ2bV9zdGFydCA8PQphZGRy
IiBjaGVja2luZykuIElmIHdlIGhhdmUgdG8gY29uc2lkZXIgYWJvdXQgImFkZHI+PSB2bV9zdGFy
dCIsIHdlIG1heQpuZWVkIGFkZGl0aW9uYWwgcGFyYW1ldGVyIG9yIGltcGxlbWVudCBhIG5ldyBm
dW5jdGlvbiBmb3IgaXQuCgoKV2VsY29tZSBhbnkgaWRlYXMsIHN1Z2dlc3Rpb25zIGFuZCBjb21w
bGV0aW9ucy4KCgpUaGFua3MuCi0tCkNoZW4gR2FuZyAos8K41SkKCk9wZW4sIHNoYXJlLCBhbmQg
YXR0aXR1ZGUgbGlrZSBhaXIsIHdhdGVyLCBhbmQgbGlmZSB3aGljaCBHb2QgYmxlc3NlZAogCQkg
CSAgIAkJICA=
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1222347

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 from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1222411 — Re: [PATCH] mm/mmap.c: Remove useless statement "vma = NULL" in find_vma()

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 from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web