From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
	id S1757125AbcAOJex (ORCPT <rfc822;w@1wt.eu>);
	Fri, 15 Jan 2016 04:34:53 -0500
Received: from casper.infradead.org ([85.118.1.10]:56135 "EHLO
	casper.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1757080AbcAOJeq (ORCPT
	<rfc822;linux-kernel@vger.kernel.org>);
	Fri, 15 Jan 2016 04:34:46 -0500
Date: Fri, 15 Jan 2016 10:34:36 +0100
From: Peter Zijlstra <peterz@infradead.org>
To: Stephane Eranian <eranian@google.com>
Cc: Ingo Molnar <mingo@kernel.org>, Arnaldo Carvalho de Melo <acme@kernel.org>,
        Namhyung Kim <namhyung@gmail.com>, LKML <linux-kernel@vger.kernel.org>,
        Jiri Olsa <jolsa@kernel.org>, Namhyung Kim <namhyung@kernel.org>,
        Adrian Hunter <adrian.hunter@intel.com>,
        "ak@linux.intel.com" <ak@linux.intel.com>
Subject: Re: [RFC] perf record: missing buildid for callstack modules
Message-ID: <20160115093436.GG3421@worktop>
References: <20160112153440.GJ6357@twins.programming.kicks-ass.net>
 <20160112154812.GH18367@kernel.org>
 <20160112161027.GN6357@twins.programming.kicks-ass.net>
 <20160112162719.GX6373@twins.programming.kicks-ass.net>
 <20160112171517.GI18367@kernel.org>
 <20160113102107.GA9644@gmail.com>
 <20160113124039.GA3421@worktop>
 <20160114112733.GB17869@gmail.com>
 <20160114113617.GX6357@twins.programming.kicks-ass.net>
 <CABPqkBRcrUYscmAAbvucTYDNsY1+XWXrP+V89FHFwiYaET9FBw@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CABPqkBRcrUYscmAAbvucTYDNsY1+XWXrP+V89FHFwiYaET9FBw@mail.gmail.com>
User-Agent: Mutt/1.5.22.1 (2013-10-16)
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Thu, Jan 14, 2016 at 05:59:48PM -0800, Stephane Eranian wrote:
> Peter,
> 
> On Thu, Jan 14, 2016 at 3:36 AM, Peter Zijlstra <peterz@infradead.org> wrote:
> > On Thu, Jan 14, 2016 at 12:27:34PM +0100, Ingo Molnar wrote:
> >> > +    *      u32                             filename_len;
> >> > +    *      char                            filename[2+];
> >
> >> Acked-by: Ingo Molnar <mingo@kernel.org>
> >
> > except of course that sizeof(u32) == 4 :/

> There is no padding here. Are you concerned about running out of bits
> in filename_len?

No, I just made a mess of it :-)

filename_len should have been u16 and filename should then be 6+8n in
size.

> Any extension possible because header.size - sizeof(mmap3) -
> filename_len sizing what's after filename, right?

Right, current MMAP records use the remaining size as the filename
length, but by explicitly specifying that we can add optional fields.

These fields must be after filename_len, otherwise you'd not be able to
find filename_len and you could not compute the extra size. And given
alignment constraints it makes sense to do it after filename[].

So suppose we wanted to also add atime and ctime, we could do.

  PERF_RECORD_MMAP3 {
    ...
    u16 filename_len;
    char filename[6+8n];

    if (extra_size >= 16) {
      u64 stime;
      u64 ctime;
    };
  }

or something like that.