From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755234AbbBBJyT (ORCPT ); Mon, 2 Feb 2015 04:54:19 -0500 Received: from mga14.intel.com ([192.55.52.115]:33535 "EHLO mga14.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753478AbbBBJyS (ORCPT ); Mon, 2 Feb 2015 04:54:18 -0500 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.09,505,1418112000"; d="scan'208";a="646067602" Message-ID: <54CF48DA.1050805@intel.com> Date: Mon, 02 Feb 2015 11:52:26 +0200 From: Adrian Hunter Organization: Intel Finland Oy, Registered Address: PL 281, 00181 Helsinki, Business Identity Code: 0357606 - 4, Domiciled in Helsinki User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.4.0 MIME-Version: 1.0 To: Jiri Olsa CC: Namhyung Kim , Arnaldo Carvalho de Melo , Ingo Molnar , Peter Zijlstra , LKML , David Ahern , Andi Kleen , Stephane Eranian , Frederic Weisbecker Subject: Re: [PATCH 14/42] perf record: Add --index option for building index table References: <1422518843-25818-1-git-send-email-namhyung@kernel.org> <1422518843-25818-15-git-send-email-namhyung@kernel.org> <20150201180635.GA6317@krava.brq.redhat.com> <54CF36AA.50508@intel.com> <20150202091554.GA1404@krava.brq.redhat.com> In-Reply-To: <20150202091554.GA1404@krava.brq.redhat.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 02/02/15 11:15, Jiri Olsa wrote: > On Mon, Feb 02, 2015 at 10:34:50AM +0200, Adrian Hunter wrote: > > SNIP > >>> but how about bump up the header version for this feature? ;-) >>> >>> currently it's: >>> >>> struct perf_file_header { >>> u64 magic; >>> u64 size; >>> u64 attr_size; >>> struct perf_file_section attrs; >>> struct perf_file_section data; >>> /* event_types is ignored */ >>> struct perf_file_section event_types; >>> DECLARE_BITMAP(adds_features, HEADER_FEAT_BITS); >>> }; >>> >>> >>> - we already store attrs as a FEATURE so we could omit that >>> - your patch stores only synthesized data into 'data' section (-1 idx) >>> this could be stored into separate file and get merged with the rest >>> - new header version would have 'features' section, so the features >>> position wouldnt depend on the 'data' end as of now and we could >>> easily store after all data is merged: >>> >>> struct perf_file_header { >>> u64 magic; >>> u64 size; >>> u64 attr_size; >>> struct perf_file_section features; >>> DECLARE_BITMAP(adds_features, HEADER_FEAT_BITS); >>> }; >>> >>> >>> thoughts? >> >> How come the features are being written before the sample data anyway? >> I would have expected: >> - write the data (update the index in memory) >> - write the features (including index) >> > > I think the problem is that the only way how to get features offset > right now is via perf_file_header::data.offset + perf_file_headerdata.size, > and we still use this section to carry 'sythesized' data, so it needs > to have correct size. Why not make it the same as all the other data. i.e. find the start and size via the index? And then just lump all the data together? > I guess we could workaround that by storing the 'perf_file_header::data' > as the last data section. That would require to treat it the same way as > all other data sections, but we could keep current header layout. Would it need to be last? Logically it should precede the data that depends on it.