linux-kernel - Re: [PATCH 04/10] perf record: Filter out POLLHUP'ed file descriptors

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 8 Sep 2014 17:10:17 +0200
From:	Jiri Olsa <jolsa@...hat.com>
To:	Arnaldo Carvalho de Melo <acme@...nel.org>
Cc:	Adrian Hunter <adrian.hunter@...el.com>,
	linux-kernel@...r.kernel.org, David Ahern <dsahern@...il.com>,
	Don Zickus <dzickus@...hat.com>,
	Frederic Weisbecker <fweisbec@...il.com>,
	Mike Galbraith <efault@....de>,
	Namhyung Kim <namhyung@...nel.org>,
	Paul Mackerras <paulus@...ba.org>,
	Peter Zijlstra <peterz@...radead.org>,
	Stephane Eranian <eranian@...gle.com>
Subject: Re: [PATCH 04/10] perf record: Filter out POLLHUP'ed file descriptors

On Mon, Sep 08, 2014 at 11:33:17AM -0300, Arnaldo Carvalho de Melo wrote:

SNIP

> 
> So what you're saying is that we should have been polling all the fds
> all the time?
> 
> Because after all we end up trying to consume everything in all the ring
> buffers when just one of them gets a POLLRD anyway...
> 
> > > I'll look at the kernel code for that...
> > >  
> > > > > I.e. when we ask the kernel to point fd B to fd A's mmap (what you
> > > > > called set-output) and fd B inserts an event into fd A's mmap ring
> > > > > buffer, we get fd A poll return as POLLRD, no?
> > > 
> > > > > Have to check... Otherwise we would have to poll all fds all the time,
> > > > > not just the ones mmaping, right?
> > > 
> > > > > > I think Jiri's patchset changed pollfds to include all fds for that reason.
> > > 
> > > > hm, I did not think of that.. ;-) I needed more grained feedback
> > > > for future features like cpu hotplug
> > > 
> > > So this is good for something you didn't tried to fix (and document) but
> > > good for something that may be nice in the future? Grumpf, we have
> > > already way too much stuff that will be eventually used but is not used
> > > right now :-\
>  
> > 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

jirka
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/