Re: [PATCH] iommu/arm-smmu-v3: add tracepoints for cmdq_issue_cmdlist

[Jean-Philippe Brucker <jean-philippe@linaro.org>]

Hi,

On Thu, Aug 27, 2020 at 09:33:51PM +1200, Barry Song wrote:
> cmdq_issue_cmdlist() is the hotspot that uses a lot of time. This patch
> adds tracepoints for it to help debug.
> 
> Signed-off-by: Barry Song <song.bao.hua@hisilicon.com>
> ---
>  * can furthermore develop an eBPF program to benchmark using this trace

Have you tried using kprobe and kretprobe instead of tracepoints?
Any noticeable performance drop?

Thanks,
Jean

> 
>   cmdlistlat.c:
> #include <uapi/linux/ptrace.h>
> 
> BPF_HASH(start, u32);
> BPF_HISTOGRAM(dist);
> 
> TRACEPOINT_PROBE(arm_smmu_v3, issue_cmdlist_entry)
> {
>         u32 pid;
>         u64 ts, *val;
> 
>         pid = bpf_get_current_pid_tgid();
>         ts = bpf_ktime_get_ns();
>         start.update(&pid, &ts);
>         return 0;
> }
> 
> TRACEPOINT_PROBE(arm_smmu_v3, issue_cmdlist_exit)
> {
>         u32 pid;
>         u64 *tsp, delta;
> 
>         pid = bpf_get_current_pid_tgid();
>         tsp = start.lookup(&pid);
> 
>         if (tsp != 0) {
>                 delta = bpf_ktime_get_ns() - *tsp;
>                 dist.increment(bpf_log2l(delta));
>                 start.delete(&pid);
>         }
> 
>         return 0;
> }
> 
>  cmdlistlat.py:
> #!/usr/bin/python3
> #
> from __future__ import print_function
> from bcc import BPF
> from ctypes import c_ushort, c_int, c_ulonglong
> from time import sleep
> from sys import argv
> 
> def usage():
>         print("USAGE: %s [interval [count]]" % argv[0])
>         exit()
> 
> # arguments
> interval = 5
> count = -1
> if len(argv) > 1:
>         try:
>                 interval = int(argv[1])
>                 if interval == 0:
>                         raise
>                 if len(argv) > 2:
>                         count = int(argv[2])
>         except: # also catches -h, --help
>                 usage()
> 
> # load BPF program
> b = BPF(src_file = "cmdlistlat.c")
> 
> # header
> print("Tracing... Hit Ctrl-C to end.")
> 
> # output
> loop = 0
> do_exit = 0
> while (1):
>         if count > 0:
>                 loop += 1
>                 if loop > count:
>                         exit()
>         try:
>                 sleep(interval)
>         except KeyboardInterrupt:
>                 pass; do_exit = 1
> 
>         print()
>         b["dist"].print_log2_hist("nsecs")
>         b["dist"].clear()
>         if do_exit:
>                 exit()
> 
> 
>  drivers/iommu/arm/arm-smmu-v3/Makefile        |  1 +
>  .../iommu/arm/arm-smmu-v3/arm-smmu-v3-trace.h | 48 +++++++++++++++++++
>  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c   |  8 ++++
>  3 files changed, 57 insertions(+)
>  create mode 100644 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-trace.h
> 
> diff --git a/drivers/iommu/arm/arm-smmu-v3/Makefile b/drivers/iommu/arm/arm-smmu-v3/Makefile
> index 569e24e9f162..dba1087f91f3 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/Makefile
> +++ b/drivers/iommu/arm/arm-smmu-v3/Makefile
> @@ -1,2 +1,3 @@
>  # SPDX-License-Identifier: GPL-2.0
> +ccflags-y += -I$(src)                   # needed for trace events
>  obj-$(CONFIG_ARM_SMMU_V3) += arm-smmu-v3.o
> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-trace.h b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-trace.h
> new file mode 100644
> index 000000000000..29ab96706124
> --- /dev/null
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-trace.h
> @@ -0,0 +1,48 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copyright (C) 2020 Hisilicon Limited.
> + */
> +
> +#undef TRACE_SYSTEM
> +#define TRACE_SYSTEM arm_smmu_v3
> +
> +#if !defined(_ARM_SMMU_V3_TRACE_H) || defined(TRACE_HEADER_MULTI_READ)
> +#define _ARM_SMMU_V3_TRACE_H
> +
> +#include <linux/tracepoint.h>
> +
> +struct device;
> +
> +DECLARE_EVENT_CLASS(issue_cmdlist_class,
> +	TP_PROTO(struct device *dev, int n, bool sync),
> +	TP_ARGS(dev, n, sync),
> +
> +	TP_STRUCT__entry(
> +		__string(device, dev_name(dev))
> +		__field(int, n)
> +		__field(bool, sync)
> +	),
> +	TP_fast_assign(
> +		__assign_str(device, dev_name(dev));
> +		__entry->n = n;
> +		__entry->sync = sync;
> +	),
> +	TP_printk("%s cmd number=%d sync=%d",
> +			__get_str(device), __entry->n, __entry->sync)
> +);
> +
> +#define DEFINE_ISSUE_CMDLIST_EVENT(name)       \
> +DEFINE_EVENT(issue_cmdlist_class, name,        \
> +	TP_PROTO(struct device *dev, int n, bool sync), \
> +	TP_ARGS(dev, n, sync))
> +
> +DEFINE_ISSUE_CMDLIST_EVENT(issue_cmdlist_entry);
> +DEFINE_ISSUE_CMDLIST_EVENT(issue_cmdlist_exit);
> +
> +#endif /* _ARM_SMMU_V3_TRACE_H */
> +
> +#undef TRACE_INCLUDE_PATH
> +#undef TRACE_INCLUDE_FILE
> +#define TRACE_INCLUDE_PATH .
> +#define TRACE_INCLUDE_FILE arm-smmu-v3-trace
> +#include <trace/define_trace.h>
> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> index 7332251dd8cd..e2d7d5f1d234 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> @@ -33,6 +33,8 @@
>  
>  #include <linux/amba/bus.h>
>  
> +#include "arm-smmu-v3-trace.h"
> +
>  /* MMIO registers */
>  #define ARM_SMMU_IDR0			0x0
>  #define IDR0_ST_LVL			GENMASK(28, 27)
> @@ -1389,6 +1391,8 @@ static int arm_smmu_cmdq_issue_cmdlist(struct arm_smmu_device *smmu,
>  	}, head = llq;
>  	int ret = 0;
>  
> +	trace_issue_cmdlist_entry(smmu->dev, n, sync);
> +
>  	/* 1. Allocate some space in the queue */
>  	local_irq_save(flags);
>  	llq.val = READ_ONCE(cmdq->q.llq.val);
> @@ -1493,6 +1497,7 @@ static int arm_smmu_cmdq_issue_cmdlist(struct arm_smmu_device *smmu,
>  	}
>  
>  	local_irq_restore(flags);
> +	trace_issue_cmdlist_exit(smmu->dev, n, sync);
>  	return ret;
>  }
>  
> @@ -4166,6 +4171,9 @@ static struct platform_driver arm_smmu_driver = {
>  };
>  module_platform_driver(arm_smmu_driver);
>  
> +#define CREATE_TRACE_POINTS
> +#include "arm-smmu-v3-trace.h"
> +
>  MODULE_DESCRIPTION("IOMMU API for ARM architected SMMUv3 implementations");
>  MODULE_AUTHOR("Will Deacon <will@kernel.org>");
>  MODULE_ALIAS("platform:arm-smmu-v3");
> -- 
> 2.27.0
> 
> 
> _______________________________________________
> iommu mailing list
> iommu@lists.linux-foundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/iommu

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel