linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Arnaldo Carvalho de Melo <acme@kernel.org>
To: Jiri Olsa <jolsa@redhat.com>
Cc: Adrian Hunter <adrian.hunter@intel.com>,
	linux-kernel@vger.kernel.org, David Ahern <dsahern@gmail.com>,
	Don Zickus <dzickus@redhat.com>,
	Frederic Weisbecker <fweisbec@gmail.com>,
	Mike Galbraith <efault@gmx.de>,
	Namhyung Kim <namhyung@kernel.org>,
	Paul Mackerras <paulus@samba.org>,
	Peter Zijlstra <peterz@infradead.org>,
	Stephane Eranian <eranian@google.com>
Subject: Re: [PATCH 04/10] perf record: Filter out POLLHUP'ed file descriptors
Date: Mon, 8 Sep 2014 12:38:24 -0300	[thread overview]
Message-ID: <20140908153824.GG2773@kernel.org> (raw)
In-Reply-To: <20140908151016.GH17728@krava.brq.redhat.com>

Em Mon, Sep 08, 2014 at 05:10:17PM +0200, Jiri Olsa escreveu:
> On Mon, Sep 08, 2014 at 11:33:17AM -0300, Arnaldo Carvalho de Melo wrote:
> > > IMO it's more clear to poll pm all event FDs.. and now with the
> > > case Adrian described it seems necessary anyway

> > I would have to check why was that we were polling just the one where
> > the mmap is done, I don't recall being the one to do it, probably who
> > did it thought that since the ring buffer is there, it was enough (and
> > possibly scaled better, dunno) to do the polling in just one of them.
 
> for read notification it's ok to poll just for one event, because they
> all share same ringbuffer and perf_poll checks if there's ANY new data
 
> for the hup notification I think we need to poll all of them

Yeah, I'm convinced of this, I'm working on a patch to make it look at
all file descriptors at poll time.

I'm doing it on top of a patch that will close the mmap when it gets a
POLLHUP, as discussed recently on this thread, in a response I gave to
Adrian.

But since multiple fds share an mmap, we can only close that mmap when
all fds are HUPed, i.e. we need to reference count struct perf_mmap,
which is what I am doing.

I.e. the fist mmaps and sets perf_mmap.nfds to 1, the next one, just
after doing that PERF_EVENT_IOC_SET_OUTPUT will bump perf_mmap.nfds,
unmap gets replaced by mmap_put, that decs and calls munmap when it
hits zero, yadda, yadda.

- Arnaldo

  reply	other threads:[~2014-09-08 15:38 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-09-03 21:59 [RFC 00/10] perf pollfd series v2 Arnaldo Carvalho de Melo
2014-09-03 21:59 ` [PATCH 01/10] perf evlist: Introduce perf_evlist__filter_pollfd method Arnaldo Carvalho de Melo
2014-09-03 21:59 ` [PATCH 02/10] perf tests: Add test for perf_evlist__filter_pollfd() Arnaldo Carvalho de Melo
2014-09-03 21:59 ` [PATCH 03/10] perf evlist: Monitor POLLERR and POLLHUP events too Arnaldo Carvalho de Melo
2014-09-03 21:59 ` [PATCH 04/10] perf record: Filter out POLLHUP'ed file descriptors Arnaldo Carvalho de Melo
2014-09-04 12:32   ` Adrian Hunter
2014-09-04 15:19     ` Arnaldo Carvalho de Melo
2014-09-05  8:42       ` Adrian Hunter
2014-09-05 14:07         ` Arnaldo Carvalho de Melo
2014-09-06 20:39           ` Jiri Olsa
2014-09-08 13:46             ` Arnaldo Carvalho de Melo
2014-09-08 14:04               ` Jiri Olsa
2014-09-08 14:33                 ` Arnaldo Carvalho de Melo
2014-09-08 15:10                   ` Jiri Olsa
2014-09-08 15:38                     ` Arnaldo Carvalho de Melo [this message]
2014-09-08 17:07                       ` Arnaldo Carvalho de Melo
2014-09-26  9:21                       ` [tip:perf/core] perf evlist: Refcount mmaps tip-bot for Arnaldo Carvalho de Melo
2014-09-26  9:20                     ` [tip:perf/core] perf evlist: We need to poll all event file descriptors tip-bot for Arnaldo Carvalho de Melo
2014-09-03 21:59 ` [PATCH 05/10] perf trace: Filter out POLLHUP'ed " Arnaldo Carvalho de Melo
2014-09-03 22:00 ` [PATCH 06/10] perf evlist: Allow growing pollfd on add method Arnaldo Carvalho de Melo
2014-09-03 22:00 ` [PATCH 07/10] perf tests: Add pollfd growing test Arnaldo Carvalho de Melo
2014-09-03 22:00 ` [PATCH 08/10] perf kvm stat live: Use perf_evlist__add_pollfd() instead of local equivalent Arnaldo Carvalho de Melo
2014-09-03 22:00 ` [PATCH 09/10] perf evlist: Introduce poll method for common code idiom Arnaldo Carvalho de Melo
2014-09-03 22:00 ` [PATCH 10/10] tools lib api: Adopt fdarray class from perf's evlist Arnaldo Carvalho de Melo
  -- strict thread matches above, loose matches on Subject: below --
2014-08-22 20:59 [RFC 00/10] perf pollfd series Arnaldo Carvalho de Melo
2014-08-22 20:59 ` [PATCH 04/10] perf record: Filter out POLLHUP'ed file descriptors Arnaldo Carvalho de Melo

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=20140908153824.GG2773@kernel.org \
    --to=acme@kernel.org \
    --cc=adrian.hunter@intel.com \
    --cc=dsahern@gmail.com \
    --cc=dzickus@redhat.com \
    --cc=efault@gmx.de \
    --cc=eranian@google.com \
    --cc=fweisbec@gmail.com \
    --cc=jolsa@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=namhyung@kernel.org \
    --cc=paulus@samba.org \
    --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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).