Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755088AbbBTP27 (ORCPT ); Fri, 20 Feb 2015 10:28:59 -0500 Received: from foss-mx-na.arm.com ([217.140.108.86]:44286 "EHLO foss-mx-na.foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754046AbbBTP26 (ORCPT ); Fri, 20 Feb 2015 10:28:58 -0500 Message-ID: <1424446129.6259.5.camel@arm.com> Subject: Re: [RFC][PATCH 2/2] perf: Add per event clockid support From: Pawel Moll To: Peter Zijlstra Cc: "adrian.hunter@intel.com" , "john.stultz@linaro.org" , "mingo@kernel.org" , "eranian@google.com" , "linux-kernel@vger.kernel.org" , "acme@kernel.org" , "dsahern@gmail.com" , "fweisbec@gmail.com" , "jolsa@redhat.com" , "namhyung@gmail.com" , "paulus@samba.org" , "tglx@linutronix.de" , "rostedt@goodmis.org" , "sonnyrao@chromium.org" , "ak@linux.intel.com" , "vincent.weaver@maine.edu" Date: Fri, 20 Feb 2015 15:28:49 +0000 In-Reply-To: <20150220143754.852733868@infradead.org> References: <20150220142930.013968488@infradead.org> <20150220143754.852733868@infradead.org> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.12.7-0ubuntu1 Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1470 Lines: 47 On Fri, 2015-02-20 at 14:29 +0000, Peter Zijlstra wrote: > The below patch makes the distinction between these two cases by > adding perf_event_clock() which is used for the second case. It > further makes this configurable on a per-event basis, but adds a few > sanity checks such that we cannot combine events with different clocks > in confusing ways. The idea works for me (obviously :-) > And since we then have per-event configurability we might as well > retain the 'legacy' behaviour as a default. Don't mind that at all. > @@ -334,8 +335,7 @@ struct perf_event_attr { > */ > __u32 sample_stack_user; > > - /* Align to u64. */ > - __u32 __reserved_2; > + __u32 clockid; I thought about it, but was sort-of-afraid to propose it :-) Now, one thing I'm not 100% sure about it is it being unsigned, as clockid_t is signed for a reason (negative values have meaning - eg. dynamic clocks, which could be useful in some circumstances). Of course casting could be an answer, but is there any reason not to make it __s32? > + default: > + /* XXX add: clock_id_valid() && clock_gettime_ns() ? */ > + err = -EINVAL; > + goto err_alloc; > + } If you asked me, I'd say -EINVAL, no default. Cheers! Pawel -- 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/