All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ingo Molnar <mingo@kernel.org>
To: Davidlohr Bueso <davidlohr@hp.com>
Cc: Al Viro <viro@zeniv.linux.org.uk>,
	Andrew Morton <akpm@linux-foundation.org>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Michel Lespinasse <walken@google.com>,
	Peter Zijlstra <a.p.zijlstra@chello.nl>,
	Rik van Riel <riel@redhat.com>,
	Tim Chen <tim.c.chen@linux.intel.com>,
	aswin@hp.com, linux-mm <linux-mm@kvack.org>,
	linux-kernel@vger.kernel.org,
	Russell King <linux@arm.linux.org.uk>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will.deacon@arm.com>,
	Richard Kuo <rkuo@codeaurora.org>,
	Ralf Baechle <ralf@linux-mips.org>,
	Benjamin Herrenschmidt <benh@kernel.crashing.org>,
	Paul Mackerras <paulus@samba.org>,
	Martin Schwidefsky <schwidefsky@de.ibm.com>,
	Heiko Carstens <heiko.carstens@de.ibm.com>,
	Paul Mundt <lethal@linux-sh.org>,
	Chris Metcalf <cmetcalf@tilera.com>,
	Jeff Dike <jdike@addtoit.com>,
	Richard Weinberger <richard@nod.at>,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>, "H. Peter Anvin" <hpa@zytor.com>
Subject: Re: [PATCH 4/3] x86/vdso: Optimize setup_additional_pages()
Date: Mon, 21 Oct 2013 07:27:31 +0200	[thread overview]
Message-ID: <20131021052731.GA14476@gmail.com> (raw)
In-Reply-To: <1382327556.2402.23.camel@buesod1.americas.hpqcorp.net>


* Davidlohr Bueso <davidlohr@hp.com> wrote:

> > 2)
> > 
> > I don't see the justification: this code gets executed in exec() where 
> > a new mm has just been allocated. There's only a single user of the mm 
> > and thus the critical section width of mmap_sem is more or less 
> > irrelevant.
> > 
> > mmap_sem critical section size only matters for codepaths that 
> > threaded programs can hit.
> 
> Yes, I was surprised by the performance boost I noticed when running 
> this patch. This weekend I re-ran the tests (including your 4/3 patch) 
> and noticed that while we're still getting some benefits (more like in 
> the +5% throughput range), it's not as good as I originally reported. I 
> believe the reason is because I had ran the tests on the vanilla kernel 
> without the max clock frequency, so the comparison was obviously not 
> fair. That said, I still think it's worth adding this patch, as it does 
> help at a micro-optimization level, and it's one less mmap_sem user we 
> have to worry about.

But it's a mmap_sem user that is essentially _guaranteed_ to have only a 
single user at that point, in the exec() path!

So I don't see how this can show _any_ measurable speedup, let alone a 5% 
speedup in a macro test. If our understanding is correct then the patch 
does nothing but shuffle around a flag setting operation. (the mmap_sem is 
equivalent to setting a single flag, in the single-user case.)

Now, if our understanding is incorrect then we need to improve our 
understanding.

Thanks,

	Ingo

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

WARNING: multiple messages have this Message-ID (diff)
From: Ingo Molnar <mingo@kernel.org>
To: Davidlohr Bueso <davidlohr@hp.com>
Cc: Al Viro <viro@zeniv.linux.org.uk>,
	Andrew Morton <akpm@linux-foundation.org>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Michel Lespinasse <walken@google.com>,
	Peter Zijlstra <a.p.zijlstra@chello.nl>,
	Rik van Riel <riel@redhat.com>,
	Tim Chen <tim.c.chen@linux.intel.com>,
	aswin@hp.com, linux-mm <linux-mm@kvack.org>,
	linux-kernel@vger.kernel.org,
	Russell King <linux@arm.linux.org.uk>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will.deacon@arm.com>,
	Richard Kuo <rkuo@codeaurora.org>,
	Ralf Baechle <ralf@linux-mips.org>,
	Benjamin Herrenschmidt <benh@kernel.crashing.org>,
	Paul Mackerras <paulus@samba.org>,
	Martin Schwidefsky <schwidefsky@de.ibm.com>,
	Heiko Carstens <heiko.carstens@de.ibm.com>,
	Paul Mundt <lethal@linux-sh.org>,
	Chris Metcalf <cmetcalf@tilera.com>,
	Jeff Dike <jdike@addtoit.com>,
	Richard Weinberger <richard@nod.at>,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>, "H. Peter Anvin" <hpa@zytor.com>
Subject: Re: [PATCH 4/3] x86/vdso: Optimize setup_additional_pages()
Date: Mon, 21 Oct 2013 07:27:31 +0200	[thread overview]
Message-ID: <20131021052731.GA14476@gmail.com> (raw)
In-Reply-To: <1382327556.2402.23.camel@buesod1.americas.hpqcorp.net>


* Davidlohr Bueso <davidlohr@hp.com> wrote:

> > 2)
> > 
> > I don't see the justification: this code gets executed in exec() where 
> > a new mm has just been allocated. There's only a single user of the mm 
> > and thus the critical section width of mmap_sem is more or less 
> > irrelevant.
> > 
> > mmap_sem critical section size only matters for codepaths that 
> > threaded programs can hit.
> 
> Yes, I was surprised by the performance boost I noticed when running 
> this patch. This weekend I re-ran the tests (including your 4/3 patch) 
> and noticed that while we're still getting some benefits (more like in 
> the +5% throughput range), it's not as good as I originally reported. I 
> believe the reason is because I had ran the tests on the vanilla kernel 
> without the max clock frequency, so the comparison was obviously not 
> fair. That said, I still think it's worth adding this patch, as it does 
> help at a micro-optimization level, and it's one less mmap_sem user we 
> have to worry about.

But it's a mmap_sem user that is essentially _guaranteed_ to have only a 
single user at that point, in the exec() path!

So I don't see how this can show _any_ measurable speedup, let alone a 5% 
speedup in a macro test. If our understanding is correct then the patch 
does nothing but shuffle around a flag setting operation. (the mmap_sem is 
equivalent to setting a single flag, in the single-user case.)

Now, if our understanding is incorrect then we need to improve our 
understanding.

Thanks,

	Ingo

  reply	other threads:[~2013-10-21  5:27 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-10-18  0:50 [PATCH 0/3] mm,vdso: preallocate new vmas Davidlohr Bueso
2013-10-18  0:50 ` Davidlohr Bueso
2013-10-18  0:50 ` [PATCH 1/3] mm: add mlock_future_check helper Davidlohr Bueso
2013-10-18  0:50   ` Davidlohr Bueso
2013-10-23  9:30   ` walken
2013-10-23  9:30     ` walken
2013-10-18  0:50 ` [PATCH 2/3] mm/mlock: prepare params outside critical region Davidlohr Bueso
2013-10-18  0:50   ` Davidlohr Bueso
2013-10-23  9:33   ` walken
2013-10-23  9:33     ` walken
2013-10-23  9:46   ` Vlastimil Babka
2013-10-23  9:46     ` Vlastimil Babka
2013-10-18  0:50 ` [PATCH 3/3] vdso: preallocate new vmas Davidlohr Bueso
2013-10-18  0:50   ` Davidlohr Bueso
2013-10-18  1:17   ` Linus Torvalds
2013-10-18  1:17     ` Linus Torvalds
2013-10-18  5:59   ` Richard Weinberger
2013-10-18  5:59     ` Richard Weinberger
2013-10-18  6:05   ` [PATCH 4/3] x86/vdso: Optimize setup_additional_pages() Ingo Molnar
2013-10-18  6:05     ` Ingo Molnar
2013-10-21  3:52     ` Davidlohr Bueso
2013-10-21  3:52       ` Davidlohr Bueso
2013-10-21  5:27       ` Ingo Molnar [this message]
2013-10-21  5:27         ` Ingo Molnar
2013-10-21  3:26   ` [PATCH 3/3] vdso: preallocate new vmas Davidlohr Bueso
2013-10-21  3:26     ` Davidlohr Bueso
2013-10-23  9:53     ` walken
2013-10-23  9:53       ` walken
2013-10-25  0:55       ` Davidlohr Bueso
2013-10-25  0:55         ` Davidlohr Bueso
2013-10-22 15:48 ` [PATCH 0/3] mm,vdso: " walken
2013-10-22 15:48   ` walken
2013-10-22 16:20   ` Linus Torvalds
2013-10-22 16:20     ` Linus Torvalds
2013-10-22 17:04     ` Michel Lespinasse
2013-10-22 17:04       ` Michel Lespinasse
2013-10-22 17:54   ` Andy Lutomirski
2013-10-22 17:54     ` Andy Lutomirski
2013-10-23 10:13     ` Michel Lespinasse
2013-10-23 10:13       ` Michel Lespinasse
2013-10-23 21:42       ` Andy Lutomirski
2013-10-23 21:42         ` Andy Lutomirski
2013-10-23  2:46   ` Davidlohr Bueso
2013-10-23  2:46     ` Davidlohr Bueso
2013-11-05  0:39 ` Davidlohr Bueso
2013-11-05  0:39   ` Davidlohr Bueso

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20131021052731.GA14476@gmail.com \
    --to=mingo@kernel.org \
    --cc=a.p.zijlstra@chello.nl \
    --cc=akpm@linux-foundation.org \
    --cc=aswin@hp.com \
    --cc=benh@kernel.crashing.org \
    --cc=catalin.marinas@arm.com \
    --cc=cmetcalf@tilera.com \
    --cc=davidlohr@hp.com \
    --cc=heiko.carstens@de.ibm.com \
    --cc=hpa@zytor.com \
    --cc=jdike@addtoit.com \
    --cc=lethal@linux-sh.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=linux@arm.linux.org.uk \
    --cc=mingo@redhat.com \
    --cc=paulus@samba.org \
    --cc=ralf@linux-mips.org \
    --cc=richard@nod.at \
    --cc=riel@redhat.com \
    --cc=rkuo@codeaurora.org \
    --cc=schwidefsky@de.ibm.com \
    --cc=tglx@linutronix.de \
    --cc=tim.c.chen@linux.intel.com \
    --cc=torvalds@linux-foundation.org \
    --cc=viro@zeniv.linux.org.uk \
    --cc=walken@google.com \
    --cc=will.deacon@arm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.