All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ingo Molnar <mingo@elte.hu>
To: Nick Piggin <npiggin@suse.de>
Cc: Arnaldo Carvalho de Melo <acme@infradead.org>,
	linux-kernel@vger.kernel.org,
	Arnaldo Carvalho de Melo <acme@redhat.com>,
	Frederic Weisbecker <fweisbec@gmail.com>,
	Mike Galbraith <efault@gmx.de>,
	Peter Zijlstra <peterz@infradead.org>,
	Stephane Eranian <eranian@google.com>
Subject: Re: [PATCH 11/19] perf record: Release resources at exit
Date: Mon, 2 Aug 2010 11:17:14 +0200	[thread overview]
Message-ID: <20100802091714.GA6781@elte.hu> (raw)
In-Reply-To: <20100802080353.GA8713@amd>


* Nick Piggin <npiggin@suse.de> wrote:

> On Mon, Aug 02, 2010 at 09:54:22AM +0200, Ingo Molnar wrote:
> > 
> > * Nick Piggin <npiggin@suse.de> wrote:
> > 
> > > On Sun, Aug 01, 2010 at 10:08:46PM -0300, Arnaldo Carvalho de Melo wrote:
> > > > From: Arnaldo Carvalho de Melo <acme@redhat.com>
> > > > 
> > > > So that we can reduce the noise on valgrind when looking for memory
> > > > leaks.
> > > 
> > > Really? That's rather crappy of valgrind. exit is well defined to release 
> > > resources and that's often a more efficient way to do it It finds and 
> > > batches things a lot better, eg. it can avoid all TLB flushing of freeing 
> > > memory that munmap requires.
> > 
> > That's certainly true but there's no valgrind crappiness here: valgrind simply 
> > can do a better job of finding leaks if there's a well defined "all resources 
> > the app still knows about are freed now" point.
> 
> "noise" sounds like false positives though. [...]

Every predictive bug detection scheme is open to the potential of false 
positives. I've yet to see a complex one that is 100% false positive free.

> [...] Certainly if this is instead allows valgrind to run in a particular 
> mode that assumes no application resources consumed at exit(2) time, I 
> wouldn't call it crappy :)

Most apps free their stuff before they exit - i do it in all my own C apps.

That is generally useful: for example it makes it easier to thread a program 
later on - when exit() becomes pthread_exit() and a silent leak turns into a 
real leak.

Hence Valgrind checking for exit() by default looks useful to me.

> But you could equally sprinkle in other valgrind specific annotations or 
> semantics at various points in the code to improve its coverage, no?

Yeah, and exit() sounds like a pretty convenient point, right? That's the 
point where all resources are inactive hence a scan for leaks is expected to 
be the most efficient in finding real leaks.

Thanks,

	Ingo

  reply	other threads:[~2010-08-02  9:17 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <1280711334-30000-1-git-send-email-acme@infradead.org>
2010-08-02  1:08 ` [PATCH 01/19] perf hists: Mark entries filtered by parent Arnaldo Carvalho de Melo
2010-08-02  1:08 ` [PATCH 02/19] perf sort: Make column width code per hists instance Arnaldo Carvalho de Melo
2010-08-02  1:08 ` [PATCH 03/19] perf ui: Restore SPACE as an alias to PGDN in annotate Arnaldo Carvalho de Melo
2010-08-02  1:08 ` [PATCH 04/19] perf hist: Introduce routine to measure lenght of formatted entry Arnaldo Carvalho de Melo
2010-08-02  1:08 ` [PATCH 05/19] perf ui: Consider the refreshed dimensions in ui_browser__show Arnaldo Carvalho de Melo
2010-08-02  1:08 ` [PATCH 06/19] perf ui: Show the scroll bar over the left window frame Arnaldo Carvalho de Melo
2010-08-02  1:08 ` [PATCH 07/19] perf ui: New hists tree widget Arnaldo Carvalho de Melo
2010-08-02  1:08 ` [PATCH 08/19] perf report: Don't abbreviate file paths relative to the cwd Arnaldo Carvalho de Melo
2010-08-02  1:08 ` [PATCH 09/19] perf tools: Remove unneeded code for tracking the cwd in perf sessions Arnaldo Carvalho de Melo
2010-08-02  1:08 ` [PATCH 10/19] perf man pages: Fix cut'n'paste error Arnaldo Carvalho de Melo
2010-08-02  1:08 ` [PATCH 11/19] perf record: Release resources at exit Arnaldo Carvalho de Melo
2010-08-02  7:30   ` Nick Piggin
2010-08-02  7:54     ` Ingo Molnar
2010-08-02  8:03       ` Nick Piggin
2010-08-02  9:17         ` Ingo Molnar [this message]
2010-08-02  9:59           ` Nick Piggin
2010-08-02 14:49             ` Arnaldo Carvalho de Melo
2010-08-02  8:08   ` Pekka Enberg
2010-08-02  1:08 ` [PATCH 12/19] perf symbols: Precisely specify if dso->{long,short}_name should be freed Arnaldo Carvalho de Melo
2010-08-02  1:08 ` [PATCH 13/19] perf tools: Factor out buildid reading and make it implicit in dso__load Arnaldo Carvalho de Melo
2010-08-02  1:08 ` [PATCH 14/19] perf tools: remove extra build-id check factored into dso__load Arnaldo Carvalho de Melo
2010-08-02  1:08 ` [PATCH 15/19] perf symbols: Improve debug image search when loading symbols Arnaldo Carvalho de Melo
2010-08-02  1:08 ` [PATCH 16/19] perf tui: Make CTRL+Z suspend perf Arnaldo Carvalho de Melo
2010-08-02  1:08 ` [PATCH 17/19] perf probe: Rename common fields/functions from kprobe to probe Arnaldo Carvalho de Melo
2010-08-02  1:08 ` [PATCH 18/19] perf tools: Release thread resources on PERF_RECORD_EXIT Arnaldo Carvalho de Melo
2010-08-02  1:08 ` [PATCH 19/19] perf tools: Release session and symbol resources on exit Arnaldo Carvalho de Melo
2010-08-02  6:26 ` [PATCH 00/19] perf/core improvements and fixes Ingo Molnar

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=20100802091714.GA6781@elte.hu \
    --to=mingo@elte.hu \
    --cc=acme@infradead.org \
    --cc=acme@redhat.com \
    --cc=efault@gmx.de \
    --cc=eranian@google.com \
    --cc=fweisbec@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=npiggin@suse.de \
    --cc=peterz@infradead.org \
    /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.