Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755760AbaAFRQT (ORCPT ); Mon, 6 Jan 2014 12:16:19 -0500 Received: from smtp3-g21.free.fr ([212.27.42.3]:37985 "EHLO smtp3-g21.free.fr" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755690AbaAFRQR (ORCPT ); Mon, 6 Jan 2014 12:16:17 -0500 Message-ID: <1389028547.13828.20.camel@localhost.localdomain> Subject: Re: [PATCH] perf tools: enable close-on-exec flag on perf file descriptor From: Yann Droneaud To: Andi Kleen Cc: Peter Zijlstra , Paul Mackerras , Ingo Molnar , Arnaldo Carvalho de Melo , Jiri Olsa , Namhyung Kim , David Ahern , Frederic Weisbecker , Mike Galbraith , Stephane Eranian , Adrian Hunter , Benjamin Herrenschmidt , Michael Ellerman , linux-kernel@vger.kernel.org, Yann Droneaud Date: Mon, 06 Jan 2014 18:15:47 +0100 In-Reply-To: <20140106165231.GF27909@tassilo.jf.intel.com> References: <8c03f54e1598b1727c19706f3af03f98685d9fe6.1388952061.git.ydroneaud@opteya.com> <20140106092929.GA31570@twins.programming.kicks-ass.net> <1389005485-12778-1-git-send-email-ydroneaud@opteya.com> <20140106162754.GE27909@tassilo.jf.intel.com> <20140106163950.GP31570@twins.programming.kicks-ass.net> <20140106165231.GF27909@tassilo.jf.intel.com> Organization: OPTEYA Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.10.3 (3.10.3-1.fc20) Mime-Version: 1.0 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi, Le lundi 06 janvier 2014 à 08:52 -0800, Andi Kleen a écrit : > > AFAICT its got nothing to do with threaded or not, but only with exec() > > and we do in fact call exec() quite a lot in perf. > > > > It ensures we do not leak open perf FDs into our child processes. Now > > I'm not entirely sure how we do the exec these days but I think we were > > good about not not leaking them anyway, but more paranoia never really > > hurts. > > Then you can just set it with fcntl, which works everywhere, > instead of doing extra feature tests. > > The atomic setup is only needed for threaded programs to avoid > races with other threads doing exec. > True. The purpose of this patch was more about exercising the new flag proposed for perf_event_open() as a way to demonstrate its usage. Having it used in the perf code base will likely help to spread it to other tools based on perf event API. Trying to add a close-on-exec enable flag late emphasis it should have been done earlier and perhaps made the default. As I wrote in "[PATCH v5 0/7] Getting rid of get_unused_fd() / enable close-on-exec" [1], setting close-on-exec at open time is easier to write for the programmer and less error prone than trying to add call to fcntl() and forget to add it in some code path. (forgetting to add the proper flag to open() is probably more error prone ... so one might want to make close-on-exec the *default* for any new API). [1] http://lkml.kernel.org/r/cover.1388952061.git.ydroneaud@opteya.com Regards. -- Yann Droneaud OPTEYA -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/