All of lore.kernel.org
 help / color / mirror / Atom feed
From: Yann Droneaud <ydroneaud@opteya.com>
To: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
Cc: Peter Zijlstra <peterz@infradead.org>,
	Paul Mackerras <paulus@samba.org>, Ingo Molnar <mingo@redhat.com>,
	Jiri Olsa <jolsa@redhat.com>, Namhyung Kim <namhyung@kernel.org>,
	Andi Kleen <ak@linux.intel.com>, David Ahern <dsahern@gmail.com>,
	Frederic Weisbecker <fweisbec@gmail.com>,
	Mike Galbraith <efault@gmx.de>,
	Stephane Eranian <eranian@google.com>,
	Adrian Hunter <adrian.hunter@intel.com>,
	Benjamin Herrenschmidt <benh@kernel.crashing.org>,
	Michael Ellerman <michael@ellerman.id.au>,
	linux-kernel@vger.kernel.org,
	Yann Droneaud <ydroneaud@opteya.com>
Subject: Re: [PATCH] perf tools: enable close-on-exec flag on perf file descriptor
Date: Mon, 06 Jan 2014 22:01:35 +0100	[thread overview]
Message-ID: <1389042095.13828.30.camel@localhost.localdomain> (raw)
In-Reply-To: <20140106144347.GA13500@ghostprotocols.net>

Hi,

Le lundi 06 janvier 2014 à 11:43 -0300, Arnaldo Carvalho de Melo a
écrit :
> Em Mon, Jan 06, 2014 at 12:24:36PM +0100, Peter Zijlstra escreveu:

> > acme, ACK? I was thinking I'd keep these two patches together so the
> > entire things lands in tip in one go?
> 
> Nope, it should notice the EINVAL, drop the flag that doesn't work on
> older kernels, retry, so that new tools continue to work on older
> kernels, with yet another fallback.
> 

But it may be difficult to distinguish the 'origin' of EINVAL: is it
from FD_CLOEXEC flag or from an unsupported parameter in the attributes.

> Please take a look at __perf_evsel__open(), probably it will be best to
> add a flag to perf_missing_features, like the one we have for the
> perf_event_attr.mmap2 flag, so that we fail just once, etc.
> 

Unfortunately perf_event_open() is called in multiple places, not only
in __perf_evsel__open(). So a more generic solution should be designed.

Is something like the function proposed in message
<1389022310.13828.9.camel@localhost.localdomain> [1]
ok to be added to its own module:

static int cloexec = PERF_FLAG_FD_CLOEXEC;

int perf_flag_fd_cloexec(void)
{
    static int probed;

    if (!probed) {
        struct perf_event_attr attr = { 0 };
        int fd = perf_event_open(&attr, 0, -1, -1,
PERF_FLAG_FD_CLOEXEC);
        probed = 1;
        if (fd >= 0)
            close(fd);
        else
            cloexec = 0;
     }

     return cloexec;
}

This function should be used to build the flag passed to
perf_event_open().

Regards.

[1] 
http://lkml.kernel.org/r/1389022310.13828.9.camel@localhost.localdomain

-- 
Yann Droneaud
OPTEYA



  reply	other threads:[~2014-01-06 21:02 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-01-05 20:36 [PATCH v5 0/7] Getting rid of get_unused_fd() / enable close-on-exec Yann Droneaud
2014-01-05 20:36 ` Yann Droneaud
2014-01-05 20:36 ` Yann Droneaud
2014-01-05 20:36 ` [PATCHv5 1/7] ia64: use get_unused_fd_flags(0) instead of get_unused_fd() Yann Droneaud
2014-01-05 20:36   ` Yann Droneaud
2014-01-05 20:36 ` [PATCHv5 2/7] ppc/cell: " Yann Droneaud
2014-01-05 20:36   ` Yann Droneaud
2014-01-05 20:36 ` [PATCHv5 3/7] binfmt_misc: " Yann Droneaud
2014-01-05 20:36 ` [PATCHv5 4/7] file: " Yann Droneaud
2014-01-05 20:36 ` [PATCHv5 5/7] fanotify: enable close-on-exec on events' fd when requested in fanotify_init() Yann Droneaud
2014-01-20 17:15   ` Yann Droneaud
2014-01-05 20:36 ` [PATCHv5 6/7] perf: introduce a flag to enable close-on-exec in perf_event_open() Yann Droneaud
2014-01-06  9:29   ` Peter Zijlstra
2014-01-06 10:51     ` [PATCH] perf tools: enable close-on-exec flag on perf file descriptor Yann Droneaud
2014-01-06 11:24       ` Peter Zijlstra
2014-01-06 14:43         ` Arnaldo Carvalho de Melo
2014-01-06 21:01           ` Yann Droneaud [this message]
2014-01-06 21:14             ` Arnaldo Carvalho de Melo
2014-01-06 14:22       ` Jiri Olsa
2014-01-06 15:31         ` Yann Droneaud
2014-01-06 16:27       ` Andi Kleen
2014-01-06 16:39         ` Peter Zijlstra
2014-01-06 16:52           ` Andi Kleen
2014-01-06 17:15             ` Yann Droneaud
2014-01-11 18:07       ` [PATCHv1] " Yann Droneaud
2014-01-13 10:09         ` [PATCHv2] " Yann Droneaud
2014-01-15 18:50           ` Arnaldo Carvalho de Melo
2014-01-26 21:20             ` [PATCHv3] " Yann Droneaud
2014-03-11  8:39               ` [PATCHv4] " Yann Droneaud
2014-06-02 10:56                 ` [PATCHv5] " Yann Droneaud
2014-06-02 19:23                   ` Jiri Olsa
2014-06-03  8:57                     ` Yann Droneaud
2014-06-03  9:23                       ` Adrian Hunter
2014-06-03 11:51                       ` Jiri Olsa
2014-06-30 20:28                     ` [PATCHv6] " Yann Droneaud
2014-07-12 23:28                       ` Jiri Olsa
2014-01-12 18:43   ` [tip:perf/core] perf: Introduce a flag to enable close-on-exec in perf_event_open() tip-bot for Yann Droneaud
2014-01-05 20:36 ` [PATCHv5 7/7] file: remove macro get_unused_fd() Yann Droneaud

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=1389042095.13828.30.camel@localhost.localdomain \
    --to=ydroneaud@opteya.com \
    --cc=acme@ghostprotocols.net \
    --cc=adrian.hunter@intel.com \
    --cc=ak@linux.intel.com \
    --cc=benh@kernel.crashing.org \
    --cc=dsahern@gmail.com \
    --cc=efault@gmx.de \
    --cc=eranian@google.com \
    --cc=fweisbec@gmail.com \
    --cc=jolsa@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=michael@ellerman.id.au \
    --cc=mingo@redhat.com \
    --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 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.