From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932099AbbBBMJW (ORCPT ); Mon, 2 Feb 2015 07:09:22 -0500 Received: from mga01.intel.com ([192.55.52.88]:15353 "EHLO mga01.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753569AbbBBMJT (ORCPT ); Mon, 2 Feb 2015 07:09:19 -0500 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="4.97,862,1389772800"; d="scan'208";a="448665358" Message-ID: <54CF687F.9090703@intel.com> Date: Mon, 02 Feb 2015 14:07:27 +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> <54CF48DA.1050805@intel.com> <20150202100512.GA2241@krava.brq.redhat.com> In-Reply-To: <20150202100512.GA2241@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 12:05, Jiri Olsa wrote: > On Mon, Feb 02, 2015 at 11:52:26AM +0200, Adrian Hunter wrote: >> 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? > > thats what I suggested No, I meant really lump it all together. i.e. perf_file_header.data.size = total data size > >> >>> 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. > > i suggested this as a workaround for having features at the end of the file > while keeping the current perf data header Which wouldn't be necessary if you lump it all together?