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]

Message-ID: <20140904151902.GE2997@kernel.org>
Date:	Thu, 4 Sep 2014 12:19:02 -0300
From:	Arnaldo Carvalho de Melo <acme@...nel.org>
To:	Adrian Hunter <adrian.hunter@...el.com>
Cc:	Jiri Olsa <jolsa@...hat.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

Em Thu, Sep 04, 2014 at 03:32:08PM +0300, Adrian Hunter escreveu:
> On 09/04/2014 12:59 AM, Arnaldo Carvalho de Melo wrote:
> > So that we don't continue polling on vanished file descriptors, i.e.
> > file descriptors for events monitoring threads that exited.

> > I.e. the following 'perf record' command now exits as expected, instead
> > of staying in an eternal loop:

> >   $ sleep 5s &
> >   $ perf record -p `pidof sleep`

> > Reported-by: Jiri Olsa <jolsa@...hat.com>
<SNIP>
> > +++ b/tools/perf/builtin-record.c
> > @@ -467,6 +467,9 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
> >  			if (err > 0 || (err < 0 && errno == EINTR))
> >  				err = 0;
> >  			waking++;
> > +			if (perf_evlist__filter_pollfd(rec->evlist, POLLERR | POLLHUP) == 0)
> 
> If the poll fds only include the ones mmapped, then mightn't it filter out
> ones still in use? e.g. what if you record two processes and one exits?

Humm, without looking at the code, we would be needlessly looking at the
mmaps for the threads that exited.

I.e. we need to associate at the fdarray level the mmaps for the
descriptors, so that we can, at fdarray__filter() time, munmap the
POLLHUPed descriptors, right?

I'll take a look at the code to see what is needed.

But then this makes the patch set good for a class of problems while
maintaining existing behaviour for the case you outlined, i.e. this is a
partial fix and should go now, with further patches on top of it?

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