From: Arnaldo Carvalho de Melo <acme@kernel.org>
To: Feng Tang <feng.tang@intel.com>
Cc: Peter Zijlstra <peterz@infradead.org>,
Ingo Molnar <mingo@redhat.com>,
Mark Rutland <mark.rutland@arm.com>,
Alexander Shishkin <alexander.shishkin@linux.intel.com>,
Jiri Olsa <jolsa@kernel.org>, Namhyung Kim <namhyung@kernel.org>,
Joe Mario <jmario@redhat.com>, Leo Yan <leo.yan@linaro.org>,
linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org,
Andi Kleen <ak@linux.intel.com>,
Kan Liang <kan.liang@linux.intel.com>,
Xing Zhengjun <zhengjun.xing@linux.intel.com>,
tim.c.chen@intel.com
Subject: Re: [PATCH v2] perf c2c: Add report option to show false sharing in adjacent cachelines
Date: Tue, 14 Feb 2023 16:30:56 -0300 [thread overview]
Message-ID: <Y+vhcAYQrX6Lv7cL@kernel.org> (raw)
In-Reply-To: <20230214075823.246414-1-feng.tang@intel.com>
Em Tue, Feb 14, 2023 at 03:58:23PM +0800, Feng Tang escreveu:
> Many platforms have feature of adjacent cachelines prefetch, when it
> is enabled, for data in RAM of 2 cachelines (2N and 2N+1) granularity,
> if one is fetched to cache, the other one could likely be fetched too,
> which sort of extends the cacheline size to double, thus the false
> sharing could happens in adjacent cachelines.
>
> 0Day has captured performance changed related with this [1], and some
> commercial software explicitly makes its hot global variables 128 bytes
> aligned (2 cache lines) to avoid this kind of extended false sharing.
>
> So add an option "-a" or "--double-cl" for c2c report to show false
> sharing in double cache line granularity, which acts just like the
> cacheline size is doubled. There is no change to c2c record. The
> hardware events of shared cacheline are still per cacheline, and
> this option just changes the granularity of how events are grouped
> and displayed.
>
> In the c2c report below (will-it-scale's 'pagefault2' case on old kernel):
>
> ----------------------------------------------------------------------
> 26 31 2 0 0 0 0xffff888103ec6000
> ----------------------------------------------------------------------
> 35.48% 50.00% 0.00% 0.00% 0.00% 0x10 0 1 0xffffffff8133148b 1153 66 971 3748 74 [k] get_mem_cgroup_from_mm
> 6.45% 0.00% 0.00% 0.00% 0.00% 0x10 0 1 0xffffffff813396e4 570 0 1531 879 75 [k] mem_cgroup_charge
> 25.81% 50.00% 0.00% 0.00% 0.00% 0x54 0 1 0xffffffff81331472 949 70 593 3359 74 [k] get_mem_cgroup_from_mm
> 19.35% 0.00% 0.00% 0.00% 0.00% 0x54 0 1 0xffffffff81339686 1352 0 1073 1022 74 [k] mem_cgroup_charge
> 9.68% 0.00% 0.00% 0.00% 0.00% 0x54 0 1 0xffffffff813396d6 1401 0 863 768 74 [k] mem_cgroup_charge
> 3.23% 0.00% 0.00% 0.00% 0.00% 0x54 0 1 0xffffffff81333106 618 0 804 11 9 [k] uncharge_batch
>
> The offset 0x10 and 0x54 used to displayed in 2 groups, and now they
> are listed together to give users a hint of extended false sharing.
>
> [1]. https://lore.kernel.org/lkml/20201102091543.GM31092@shao2-debian/
>
> Signed-off-by: Feng Tang <feng.tang@intel.com>
> Reviewed-by: Andi Kleen <ak@linux.intel.com>
> Reviewed-by: Leo Yan <leo.yan@linaro.org>
> Tested-by: Leo Yan <leo.yan@linaro.org>
> ---
> Changelog:
>
> v1 -> v2:
> * Refine comments and fix typos (Leo Yan)
> * Add reviewd-by and tested-by(for Arm64) tag from Leo Yan
> * Refine cmd description and commit log to avoid using
> architecture specific name (Joe Mario)
>
> tools/perf/Documentation/perf-c2c.txt | 7 +++++++
> tools/perf/builtin-c2c.c | 22 +++++++++++++---------
> tools/perf/util/cacheline.h | 25 ++++++++++++++++++++-----
> tools/perf/util/sort.c | 13 ++++++++++---
> tools/perf/util/sort.h | 1 +
> 5 files changed, 51 insertions(+), 17 deletions(-)
>
> diff --git a/tools/perf/Documentation/perf-c2c.txt b/tools/perf/Documentation/perf-c2c.txt
> index 4e8c263e1721..cc18d61ec5d5 100644
> --- a/tools/perf/Documentation/perf-c2c.txt
> +++ b/tools/perf/Documentation/perf-c2c.txt
> @@ -130,6 +130,13 @@ REPORT OPTIONS
> The known limitations include exception handing such as
> setjmp/longjmp will have calls/returns not match.
>
> +-a::
> +--double-cl::
> + Group the detection of shared cacheline events into double cacheline
> + granularity. Some architectures have an Adjacent Cacheline Prefetch
> + feature, which causes cacheline sharing to behave like the cacheline
> + size is doubled.
> +
Humm, this is something not that usual, so I think we should have it
just as --double-cl, ok?
I can do the adjustment here if you agree.
Thanks,
- Arnaldo
> C2C RECORD
> ----------
> The perf c2c record command setup options related to HITM cacheline analysis
> diff --git a/tools/perf/builtin-c2c.c b/tools/perf/builtin-c2c.c
> index 52d94c7dd836..1a98c6e97a4b 100644
> --- a/tools/perf/builtin-c2c.c
> +++ b/tools/perf/builtin-c2c.c
> @@ -524,7 +524,7 @@ static int dcacheline_entry(struct perf_hpp_fmt *fmt, struct perf_hpp *hpp,
> char buf[20];
>
> if (he->mem_info)
> - addr = cl_address(he->mem_info->daddr.addr);
> + addr = cl_address(he->mem_info->daddr.addr, chk_double_cl);
>
> return scnprintf(hpp->buf, hpp->size, "%*s", width, HEX_STR(buf, addr));
> }
> @@ -562,7 +562,7 @@ static int offset_entry(struct perf_hpp_fmt *fmt, struct perf_hpp *hpp,
> char buf[20];
>
> if (he->mem_info)
> - addr = cl_offset(he->mem_info->daddr.al_addr);
> + addr = cl_offset(he->mem_info->daddr.al_addr, chk_double_cl);
>
> return scnprintf(hpp->buf, hpp->size, "%*s", width, HEX_STR(buf, addr));
> }
> @@ -574,9 +574,10 @@ offset_cmp(struct perf_hpp_fmt *fmt __maybe_unused,
> uint64_t l = 0, r = 0;
>
> if (left->mem_info)
> - l = cl_offset(left->mem_info->daddr.addr);
> + l = cl_offset(left->mem_info->daddr.addr, chk_double_cl);
> +
> if (right->mem_info)
> - r = cl_offset(right->mem_info->daddr.addr);
> + r = cl_offset(right->mem_info->daddr.addr, chk_double_cl);
>
> return (int64_t)(r - l);
> }
> @@ -2590,7 +2591,7 @@ perf_c2c_cacheline_browser__title(struct hist_browser *browser,
> he = cl_browser->he;
>
> if (he->mem_info)
> - addr = cl_address(he->mem_info->daddr.addr);
> + addr = cl_address(he->mem_info->daddr.addr, chk_double_cl);
>
> scnprintf(bf, size, "Cacheline 0x%lx", addr);
> return 0;
> @@ -2788,15 +2789,16 @@ static int ui_quirks(void)
> if (!c2c.use_stdio) {
> dim_offset.width = 5;
> dim_offset.header = header_offset_tui;
> - nodestr = "CL";
> + nodestr = chk_double_cl ? "Double-CL" : "CL";
> }
>
> dim_percent_costly_snoop.header = percent_costly_snoop_header[c2c.display];
>
> /* Fix the zero line for dcacheline column. */
> - buf = fill_line("Cacheline", dim_dcacheline.width +
> - dim_dcacheline_node.width +
> - dim_dcacheline_count.width + 4);
> + buf = fill_line(chk_double_cl ? "Double-Cacheline" : "Cacheline",
> + dim_dcacheline.width +
> + dim_dcacheline_node.width +
> + dim_dcacheline_count.width + 4);
> if (!buf)
> return -ENOMEM;
>
> @@ -3037,6 +3039,8 @@ static int perf_c2c__report(int argc, const char **argv)
> OPT_BOOLEAN('f', "force", &symbol_conf.force, "don't complain, do it"),
> OPT_BOOLEAN(0, "stitch-lbr", &c2c.stitch_lbr,
> "Enable LBR callgraph stitching approach"),
> + OPT_BOOLEAN('a', "double-cl", &chk_double_cl,
> + "Detect adjacent cacheline false sharing"),
> OPT_PARENT(c2c_options),
> OPT_END()
> };
> diff --git a/tools/perf/util/cacheline.h b/tools/perf/util/cacheline.h
> index dec8c0fb1f4a..fe6d5b60a031 100644
> --- a/tools/perf/util/cacheline.h
> +++ b/tools/perf/util/cacheline.h
> @@ -6,16 +6,31 @@
>
> int __pure cacheline_size(void);
>
> -static inline u64 cl_address(u64 address)
> +
> +/*
> + * Some architectures have 'Adjacent Cacheline Prefetch' feature,
> + * which performs like the cacheline size being doubled.
> + */
> +static inline u64 cl_address(u64 address, bool double_cl)
> {
> + u64 size = cacheline_size();
> +
> + if (double_cl)
> + size *= 2;
> +
> /* return the cacheline of the address */
> - return (address & ~(cacheline_size() - 1));
> + return (address & ~(size - 1));
> }
>
> -static inline u64 cl_offset(u64 address)
> +static inline u64 cl_offset(u64 address, bool double_cl)
> {
> - /* return the cacheline of the address */
> - return (address & (cacheline_size() - 1));
> + u64 size = cacheline_size();
> +
> + if (double_cl)
> + size *= 2;
> +
> + /* return the offset inside cacheline */
> + return (address & (size - 1));
> }
>
> #endif // PERF_CACHELINE_H
> diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
> index 4a648231fe72..093a0c8b2e3d 100644
> --- a/tools/perf/util/sort.c
> +++ b/tools/perf/util/sort.c
> @@ -53,6 +53,13 @@ enum sort_mode sort__mode = SORT_MODE__NORMAL;
> static const char *const dynamic_headers[] = {"local_ins_lat", "ins_lat", "local_p_stage_cyc", "p_stage_cyc"};
> static const char *const arch_specific_sort_keys[] = {"local_p_stage_cyc", "p_stage_cyc"};
>
> +/*
> + * Some architectures have Adjacent Cacheline Prefetch feature, which
> + * behaves like the cacheline size is doubled. Enable this flag to
> + * check things in double cacheline granularity.
> + */
> +bool chk_double_cl;
> +
> /*
> * Replaces all occurrences of a char used with the:
> *
> @@ -1500,8 +1507,8 @@ sort__dcacheline_cmp(struct hist_entry *left, struct hist_entry *right)
>
> addr:
> /* al_addr does all the right addr - start + offset calculations */
> - l = cl_address(left->mem_info->daddr.al_addr);
> - r = cl_address(right->mem_info->daddr.al_addr);
> + l = cl_address(left->mem_info->daddr.al_addr, chk_double_cl);
> + r = cl_address(right->mem_info->daddr.al_addr, chk_double_cl);
>
> if (l > r) return -1;
> if (l < r) return 1;
> @@ -1520,7 +1527,7 @@ static int hist_entry__dcacheline_snprintf(struct hist_entry *he, char *bf,
> if (he->mem_info) {
> struct map *map = he->mem_info->daddr.ms.map;
>
> - addr = cl_address(he->mem_info->daddr.al_addr);
> + addr = cl_address(he->mem_info->daddr.al_addr, chk_double_cl);
> ms = &he->mem_info->daddr.ms;
>
> /* print [s] for shared data mmaps */
> diff --git a/tools/perf/util/sort.h b/tools/perf/util/sort.h
> index 9a91d0df2833..d79a100e5999 100644
> --- a/tools/perf/util/sort.h
> +++ b/tools/perf/util/sort.h
> @@ -35,6 +35,7 @@ extern struct sort_entry sort_sym_from;
> extern struct sort_entry sort_sym_to;
> extern struct sort_entry sort_srcline;
> extern const char default_mem_sort_order[];
> +extern bool chk_double_cl;
>
> struct res_sample {
> u64 time;
> --
> 2.34.1
>
--
- Arnaldo
next prev parent reply other threads:[~2023-02-14 19:31 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-02-14 7:58 [PATCH v2] perf c2c: Add report option to show false sharing in adjacent cachelines Feng Tang
2023-02-14 19:30 ` Arnaldo Carvalho de Melo [this message]
2023-02-15 1:03 ` Feng Tang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Y+vhcAYQrX6Lv7cL@kernel.org \
--to=acme@kernel.org \
--cc=ak@linux.intel.com \
--cc=alexander.shishkin@linux.intel.com \
--cc=feng.tang@intel.com \
--cc=jmario@redhat.com \
--cc=jolsa@kernel.org \
--cc=kan.liang@linux.intel.com \
--cc=leo.yan@linaro.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-perf-users@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=mingo@redhat.com \
--cc=namhyung@kernel.org \
--cc=peterz@infradead.org \
--cc=tim.c.chen@intel.com \
--cc=zhengjun.xing@linux.intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.