Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757158AbcC2OEy (ORCPT ); Tue, 29 Mar 2016 10:04:54 -0400 Received: from bombadil.infradead.org ([198.137.202.9]:33189 "EHLO bombadil.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756814AbcC2OEw (ORCPT ); Tue, 29 Mar 2016 10:04:52 -0400 Date: Tue, 29 Mar 2016 16:04:39 +0200 From: Peter Zijlstra To: Wang Nan Cc: Alexei Starovoitov , Arnaldo Carvalho de Melo , linux-kernel@vger.kernel.org, Brendan Gregg , He Kuang , Jiri Olsa , Masami Hiramatsu , Namhyung Kim , pi3orama@163.com, Zefan Li Subject: Re: [PATCH 4/4] perf core: Add backward attribute to perf event Message-ID: <20160329140439.GK3408@twins.programming.kicks-ass.net> References: <1459147292-239310-1-git-send-email-wangnan0@huawei.com> <1459147292-239310-5-git-send-email-wangnan0@huawei.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1459147292-239310-5-git-send-email-wangnan0@huawei.com> User-Agent: Mutt/1.5.21 (2012-12-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 942 Lines: 35 On Mon, Mar 28, 2016 at 06:41:32AM +0000, Wang Nan wrote: Could you maybe write a perf/tests thingy for this so that _some_ userspace exists that exercises this new code? > int perf_output_begin(struct perf_output_handle *handle, > struct perf_event *event, unsigned int size) > { > + if (unlikely(is_write_backward(event))) > + return __perf_output_begin(handle, event, size, true); > return __perf_output_begin(handle, event, size, false); > } Would something like: int perf_output_begin(...) { if (unlikely(is_write_backward(event)) return perf_output_begin_backward(...); return perf_output_begin_forward(...); } make sense; I'm not sure how much is still using this, but it seems somewhat excessive to inline two copies of that thing into a single function. Alternatively; something like: int perf_output_begin(...) { return __perf_output_begin(..., unlikely(event->attr.backwards)); } might make sense too.