Re: [PATCH 1/2] perf/core: Share an event with multiple cgroups

[Arnaldo Carvalho de Melo <acme@kernel.org>]

Em Thu, Mar 25, 2021 at 12:55:50AM +0000, Song Liu escreveu:
> > On Mar 23, 2021, at 9:21 AM, Namhyung Kim <namhyung@kernel.org> wrote:
> > #ifdef CONFIG_SECURITY
> > @@ -780,6 +792,14 @@ struct perf_event {
> > #endif /* CONFIG_PERF_EVENTS */
> > };

> > +struct perf_cgroup_node {
> > +	struct hlist_node		node;
> > +	u64				id;
> > +	u64				count;
> > +	u64				time_enabled;
> > +	u64				time_running;
> > +	u64				padding[2];
> 
> Do we really need the padding? For cache line alignment? 

I guess so, to get it to 64 bytes, then having it as:

struct perf_cgroup_node {
	struct hlist_node		node;
	u64				id;
	u64				count;
	u64				time_enabled;
	u64				time_running;
} ____cacheline_aligned;

Seems better :-)

Testing:

[acme@five c]$ cat cacheline_aligned.c
#ifndef ____cacheline_aligned
#define ____cacheline_aligned __attribute__((__aligned__(SMP_CACHE_BYTES)))
#endif

// from ../build/v5.12.0-rc4+/include/generated/autoconf.h
#define CONFIG_X86_L1_CACHE_SHIFT 6

#define L1_CACHE_SHIFT  (CONFIG_X86_L1_CACHE_SHIFT)
#define L1_CACHE_BYTES  (1 << L1_CACHE_SHIFT)

#ifndef SMP_CACHE_BYTES
#define SMP_CACHE_BYTES L1_CACHE_BYTES
#endif

typedef long long unsigned int u64;

struct hlist_node {
	struct hlist_node *        next;                 /*     0     8 */
	struct hlist_node * *      pprev;                /*     8     8 */

	/* size: 16, cachelines: 1, members: 2 */
	/* last cacheline: 16 bytes */
};

struct perf_cgroup_node {
        struct hlist_node               node;
        u64                             id;
        u64                             count;
        u64                             time_enabled;
        u64                             time_running;
} ____cacheline_aligned foo;

[acme@five c]$ cc  -g  -c -o cacheline_aligned.o cacheline_aligned.c
[acme@five c]$ pahole cacheline_aligned.o
struct hlist_node {
	struct hlist_node *        next;                 /*     0     8 */
	struct hlist_node * *      pprev;                /*     8     8 */

	/* size: 16, cachelines: 1, members: 2 */
	/* last cacheline: 16 bytes */
};
struct perf_cgroup_node {
	struct hlist_node          node;                 /*     0    16 */
	u64                        id;                   /*    16     8 */
	u64                        count;                /*    24     8 */
	u64                        time_enabled;         /*    32     8 */
	u64                        time_running;         /*    40     8 */

	/* size: 64, cachelines: 1, members: 5 */
	/* padding: 16 */
} __attribute__((__aligned__(64)));
[acme@five c]$

- Arnaldo