Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Tue, 19 Nov 2019 12:06:41 +0100
From:   Jiri Olsa <jolsa@redhat.com>
To:     Arnaldo Carvalho de Melo <acme@kernel.org>
Cc:     Jiri Olsa <jolsa@kernel.org>, lkml <linux-kernel@vger.kernel.org>,
        Ingo Molnar <mingo@kernel.org>,
        Namhyung Kim <namhyung@kernel.org>,
        Alexander Shishkin <alexander.shishkin@linux.intel.com>,
        Peter Zijlstra <a.p.zijlstra@chello.nl>,
        Michael Petlan <mpetlan@redhat.com>,
        Andi Kleen <ak@linux.intel.com>,
        Stephane Eranian <eranian@google.com>,
        Jin Yao <yao.jin@linux.intel.com>,
        Alexey Budankov <alexey.budankov@linux.intel.com>
Subject: Re: [PATCHv2 0/2] perf tools: Share struct map after clone
Message-ID: <20191119110641.GC21165@krava>
References: <20191016082226.10325-1-jolsa@kernel.org>
 <20191023075517.GA22919@krava>
 <20191029205855.GA20826@krava>
 <20191118121400.GA14046@krava>
 <20191118214851.GA17315@kernel.org>
MIME-Version: 1.0
In-Reply-To: <20191118214851.GA17315@kernel.org>
User-Agent: Mutt/1.12.1 (2019-06-15)
Content-Type: text/plain; charset=WINDOWS-1252
Content-Transfer-Encoding: quoted-printable
Content-Disposition: inline
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Mon, Nov 18, 2019 at 06:48:51PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Mon, Nov 18, 2019 at 01:14:00PM +0100, Jiri Olsa escreveu:
> > On Tue, Oct 29, 2019 at 09:58:55PM +0100, Jiri Olsa wrote:
> > > > >=20
> > > > > Also available in here:
> > > > >   git://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git
> > > > >   perf/map_shared
>=20
> > > > I rebased to latest perf/core and pushed the branch out
>=20
> > > rebased and pushed out
> =20
> > heya,
> > I lost track of this.. what's the status, are you going with your
> > version, or is this one still in? I don't see any of them in latest
> > code..
>=20
> So, I'm still working on and off on this, current status is at:
>=20
> https://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git/log/?h=3Dp=
erf/map_share
>=20
> Its just one patch more than perf/core, the one that does the sharing.
>=20
> The thing is, as I'm going over all the fields in 'struct map', it seems
> that we'll end up with just one cacheline per instance, as there are
> things there that are not strictly related to a map, but to a map_group
> (unmap_ip/map_ip), or to a dso (maj, min, ino, ino_generation), and some
> need less than what is allocated to them.
>=20
> Current status is:
>=20
> [root@quaco ~]# pahole -C map ~acme/bin/perf
> struct map {
> =09union {
> =09=09struct rb_node rb_node __attribute__((__aligned__(8))); /*     0   =
 24 */
> =09=09struct list_head node;                   /*     0    16 */
> =09} __attribute__((__aligned__(8)));                                    =
           /*     0    24 */
> =09u64                        start;                /*    24     8 */
> =09u64                        end;                  /*    32     8 */
> =09_Bool                      erange_warned:1;      /*    40: 0  1 */
> =09_Bool                      priv:1;               /*    40: 1  1 */
>=20
> =09/* XXX 6 bits hole, try to pack */
> =09/* XXX 3 bytes hole, try to pack */
>=20
> =09u32                        prot;                 /*    44     4 */
> =09u64                        pgoff;                /*    48     8 */
> =09u64                        reloc;                /*    56     8 */
> =09/* --- cacheline 1 boundary (64 bytes) --- */
> =09u64                        (*map_ip)(struct map *, u64); /*    64     =
8 */
> =09u64                        (*unmap_ip)(struct map *, u64); /*    72   =
  8 */
> =09struct dso *               dso;                  /*    80     8 */
> =09refcount_t                 refcnt;               /*    88     4 */
> =09u32                        flags;                /*    92     4 */
>=20
> =09/* size: 96, cachelines: 2, members: 13 */
> =09/* sum members: 92, holes: 1, sum holes: 3 */
> =09/* sum bitfield members: 2 bits, bit holes: 1, sum bit holes: 6 bits *=
/
> =09/* forced alignments: 1 */
> =09/* last cacheline: 32 bytes */
> } __attribute__((__aligned__(8)));
> [root@quaco ~]#
>=20
> This is with the tentative move of maj/min/ino/ino_generation to 'struct
> dso', but that needs more work to match the sort order that touches it
> "dcacheline", i.e. a map that comes with the same backing DSO but
> different values for those fields is not the same DSO, right?
>=20
> Right now with moving the maj/min/etc to dso, in the map_share patch we
> get the structure used to keep shared entries in the rb_tree at 40
> bytes, under one cacheline, while the full 'struct map' is 32 bytes more
> than one cacheline, so still good for sharing:
>=20
> [acme@quaco perf]$ pahole -C map_node ~/bin/perf
> struct map_node {
> =09union {
> =09=09struct rb_node rb_node __attribute__((__aligned__(8))); /*     0   =
 24 */
> =09=09struct list_head node;                   /*     0    16 */
> =09} __attribute__((__aligned__(8)));               /*     0    24 */
> =09refcount_t                 refcnt;               /*    24     4 */
> =09_Bool                      is_node:1;            /*    28: 0  1 */
>=20
> =09/* XXX 7 bits hole, try to pack */
> =09/* XXX 3 bytes hole, try to pack */
>=20
> =09struct map *               map;                  /*    32     8 */
>=20
> =09/* size: 40, cachelines: 1, members: 4 */
> =09/* sum members: 36, holes: 1, sum holes: 3 */
> =09/* sum bitfield members: 1 bits, bit holes: 1, sum bit holes: 7 bits *=
/
> =09/* forced alignments: 1 */
> =09/* last cacheline: 40 bytes */
> } __attribute__((__aligned__(8)));
> [acme@quaco perf]$ pahole -C map ~/bin/perf
> struct map {
> =09union {
> =09=09struct rb_node rb_node __attribute__((__aligned__(8))); /*     0   =
 24 */
> =09=09struct list_head node;                   /*     0    16 */
> =09} __attribute__((__aligned__(8)));               /*     0    24 */
> =09refcount_t                 refcnt;               /*    24     4 */
> =09_Bool                      is_node:1;            /*    28: 0  1 */
> =09_Bool                      erange_warned:1;      /*    28: 1  1 */
> =09_Bool                      priv:1;               /*    28: 2  1 */
>=20
> =09/* XXX 5 bits hole, try to pack */
> =09/* XXX 3 bytes hole, try to pack */
>=20
> =09u64                        start;                /*    32     8 */
> =09u64                        end;                  /*    40     8 */
> =09u64                        pgoff;                /*    48     8 */
> =09u64                        reloc;                /*    56     8 */
> =09/* --- cacheline 1 boundary (64 bytes) --- */
> =09u64                        (*map_ip)(struct map *, u64); /*    64     =
8 */
> =09u64                        (*unmap_ip)(struct map *, u64); /*    72   =
  8 */
> =09struct dso *               dso;                  /*    80     8 */
> =09u32                        flags;                /*    88     4 */
> =09u32                        prot;                 /*    92     4 */
>=20
> =09/* size: 96, cachelines: 2, members: 14 */
> =09/* sum members: 92, holes: 1, sum holes: 3 */
> =09/* sum bitfield members: 3 bits, bit holes: 1, sum bit holes: 5 bits *=
/
> =09/* forced alignments: 1 */
> =09/* last cacheline: 32 bytes */
> } __attribute__((__aligned__(8)));
> [acme@quaco perf]$
>=20
> So give me some more time, please :-)

sure ;-) I just did not want to loose track of this

thanks,
jirka