Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1662421 > unrolled thread
| Started by | Vitaly Kuznetsov <vkuznets@redhat.com> |
|---|---|
| First post | 2017-06-09 15:30 +0200 |
| Last post | 2017-06-12 05:00 +0200 |
| Articles | 8 — 4 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
[PATCH v8 10/10] tracing/hyper-v: trace hyperv_mmu_flush_tlb_others() Vitaly Kuznetsov <vkuznets@redhat.com> - 2017-06-09 15:30 +0200
Re: [PATCH v8 10/10] tracing/hyper-v: trace hyperv_mmu_flush_tlb_others() Steven Rostedt <rostedt@goodmis.org> - 2017-06-09 20:10 +0200
Re: [PATCH v8 10/10] tracing/hyper-v: trace hyperv_mmu_flush_tlb_others() Andy Shevchenko <andy.shevchenko@gmail.com> - 2017-06-09 20:30 +0200
Re: [PATCH v8 10/10] tracing/hyper-v: trace hyperv_mmu_flush_tlb_others() Steven Rostedt <rostedt@goodmis.org> - 2017-06-09 20:40 +0200
Re: [PATCH v8 10/10] tracing/hyper-v: trace hyperv_mmu_flush_tlb_others() Andy Shevchenko <andy.shevchenko@gmail.com> - 2017-06-09 20:50 +0200
Re: [PATCH v8 10/10] tracing/hyper-v: trace hyperv_mmu_flush_tlb_others() Paul Bolle <pebolle@tiscali.nl> - 2017-06-09 21:00 +0200
Re: [PATCH v8 10/10] tracing/hyper-v: trace hyperv_mmu_flush_tlb_others() Steven Rostedt <rostedt@goodmis.org> - 2017-06-09 21:10 +0200
Re: [PATCH v8 10/10] tracing/hyper-v: trace hyperv_mmu_flush_tlb_others() Vitaly Kuznetsov <vkuznets@redhat.com> - 2017-06-12 05:00 +0200
| From | Vitaly Kuznetsov <vkuznets@redhat.com> |
|---|---|
| Date | 2017-06-09 15:30 +0200 |
| Subject | [PATCH v8 10/10] tracing/hyper-v: trace hyperv_mmu_flush_tlb_others() |
| Message-ID | <tQs9d-22p-31@gated-at.bofh.it> |
Add Hyper-V tracing subsystem and trace hyperv_mmu_flush_tlb_others().
Tracing is done the same way we do xen_mmu_flush_tlb_others().
Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
MAINTAINERS | 1 +
arch/x86/hyperv/mmu.c | 7 +++++++
arch/x86/include/asm/trace/hyperv.h | 38 +++++++++++++++++++++++++++++++++++++
3 files changed, 46 insertions(+)
create mode 100644 arch/x86/include/asm/trace/hyperv.h
diff --git a/MAINTAINERS b/MAINTAINERS
index e3f44fd..001614b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6168,6 +6168,7 @@ M: Stephen Hemminger <sthemmin@microsoft.com>
L: devel@linuxdriverproject.org
S: Maintained
F: arch/x86/include/asm/mshyperv.h
+F: arch/x86/include/asm/trace/hyperv.h
F: arch/x86/include/uapi/asm/hyperv.h
F: arch/x86/kernel/cpu/mshyperv.c
F: arch/x86/hyperv
diff --git a/arch/x86/hyperv/mmu.c b/arch/x86/hyperv/mmu.c
index 0712111..cb35453 100644
--- a/arch/x86/hyperv/mmu.c
+++ b/arch/x86/hyperv/mmu.c
@@ -8,6 +8,9 @@
#include <asm/msr.h>
#include <asm/tlbflush.h>
+#define CREATE_TRACE_POINTS
+#include <asm/trace/hyperv.h>
+
/* HvFlushVirtualAddressSpace, HvFlushVirtualAddressList hypercalls */
struct hv_flush_pcpu {
u64 address_space;
@@ -103,6 +106,8 @@ static void hyperv_flush_tlb_others(const struct cpumask *cpus,
u64 status = U64_MAX;
unsigned long flags;
+ trace_hyperv_mmu_flush_tlb_others(cpus, mm, start, end);
+
if (!pcpu_flush || !hv_hypercall_pg)
goto do_native;
@@ -172,6 +177,8 @@ static void hyperv_flush_tlb_others_ex(const struct cpumask *cpus,
u64 status = U64_MAX;
unsigned long flags;
+ trace_hyperv_mmu_flush_tlb_others(cpus, mm, start, end);
+
if (!pcpu_flush_ex || !hv_hypercall_pg)
goto do_native;
diff --git a/arch/x86/include/asm/trace/hyperv.h b/arch/x86/include/asm/trace/hyperv.h
new file mode 100644
index 0000000..e44ece1
--- /dev/null
+++ b/arch/x86/include/asm/trace/hyperv.h
@@ -0,0 +1,38 @@
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM hyperv
+
+#if !defined(_TRACE_HYPERV_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _TRACE_HYPERV_H
+
+#include <linux/tracepoint.h>
+
+#if IS_ENABLED(CONFIG_HYPERV)
+
+TRACE_EVENT(hyperv_mmu_flush_tlb_others,
+ TP_PROTO(const struct cpumask *cpus, struct mm_struct *mm,
+ unsigned long addr, unsigned long end),
+ TP_ARGS(cpus, mm, addr, end),
+ TP_STRUCT__entry(
+ __field(unsigned int, ncpus)
+ __field(struct mm_struct *, mm)
+ __field(unsigned long, addr)
+ __field(unsigned long, end)
+ ),
+ TP_fast_assign(__entry->ncpus = cpumask_weight(cpus);
+ __entry->mm = mm;
+ __entry->addr = addr,
+ __entry->end = end),
+ TP_printk("ncpus %d mm %p addr %lx, end %lx",
+ __entry->ncpus, __entry->mm, __entry->addr, __entry->end)
+ );
+
+#endif /* CONFIG_HYPERV */
+
+#undef TRACE_INCLUDE_PATH
+#define TRACE_INCLUDE_PATH asm/trace/
+#undef TRACE_INCLUDE_FILE
+#define TRACE_INCLUDE_FILE hyperv
+#endif /* _TRACE_HYPERV_H */
+
+/* This part must be outside protection */
+#include <trace/define_trace.h>
--
2.9.4
[toc] | [next] | [standalone]
| From | Steven Rostedt <rostedt@goodmis.org> |
|---|---|
| Date | 2017-06-09 20:10 +0200 |
| Subject | Re: [PATCH v8 10/10] tracing/hyper-v: trace hyperv_mmu_flush_tlb_others() |
| Message-ID | <tQww9-4Rr-3@gated-at.bofh.it> |
| In reply to | #1662421 |
On Fri, 9 Jun 2017 15:27:36 +0200
Vitaly Kuznetsov <vkuznets@redhat.com> wrote:
> Add Hyper-V tracing subsystem and trace hyperv_mmu_flush_tlb_others().
> Tracing is done the same way we do xen_mmu_flush_tlb_others().
>
> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
> ---
> MAINTAINERS | 1 +
> arch/x86/hyperv/mmu.c | 7 +++++++
> arch/x86/include/asm/trace/hyperv.h | 38 +++++++++++++++++++++++++++++++++++++
> 3 files changed, 46 insertions(+)
> create mode 100644 arch/x86/include/asm/trace/hyperv.h
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index e3f44fd..001614b 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -6168,6 +6168,7 @@ M: Stephen Hemminger <sthemmin@microsoft.com>
> L: devel@linuxdriverproject.org
> S: Maintained
> F: arch/x86/include/asm/mshyperv.h
> +F: arch/x86/include/asm/trace/hyperv.h
> F: arch/x86/include/uapi/asm/hyperv.h
> F: arch/x86/kernel/cpu/mshyperv.c
> F: arch/x86/hyperv
> diff --git a/arch/x86/hyperv/mmu.c b/arch/x86/hyperv/mmu.c
> index 0712111..cb35453 100644
> --- a/arch/x86/hyperv/mmu.c
> +++ b/arch/x86/hyperv/mmu.c
> @@ -8,6 +8,9 @@
> #include <asm/msr.h>
> #include <asm/tlbflush.h>
>
> +#define CREATE_TRACE_POINTS
> +#include <asm/trace/hyperv.h>
> +
> /* HvFlushVirtualAddressSpace, HvFlushVirtualAddressList hypercalls */
> struct hv_flush_pcpu {
> u64 address_space;
> @@ -103,6 +106,8 @@ static void hyperv_flush_tlb_others(const struct cpumask *cpus,
> u64 status = U64_MAX;
> unsigned long flags;
>
> + trace_hyperv_mmu_flush_tlb_others(cpus, mm, start, end);
> +
> if (!pcpu_flush || !hv_hypercall_pg)
> goto do_native;
>
> @@ -172,6 +177,8 @@ static void hyperv_flush_tlb_others_ex(const struct cpumask *cpus,
> u64 status = U64_MAX;
> unsigned long flags;
>
> + trace_hyperv_mmu_flush_tlb_others(cpus, mm, start, end);
> +
> if (!pcpu_flush_ex || !hv_hypercall_pg)
> goto do_native;
>
> diff --git a/arch/x86/include/asm/trace/hyperv.h b/arch/x86/include/asm/trace/hyperv.h
> new file mode 100644
> index 0000000..e44ece1
> --- /dev/null
> +++ b/arch/x86/include/asm/trace/hyperv.h
> @@ -0,0 +1,38 @@
> +#undef TRACE_SYSTEM
> +#define TRACE_SYSTEM hyperv
> +
> +#if !defined(_TRACE_HYPERV_H) || defined(TRACE_HEADER_MULTI_READ)
> +#define _TRACE_HYPERV_H
> +
> +#include <linux/tracepoint.h>
> +
> +#if IS_ENABLED(CONFIG_HYPERV)
Hmm, this is new to me. I know you can use IS_ENABLED() inside C code,
but I've never seen it used for preprocessor directives. This usually
is just:
#ifdef CONFIG_HYPERV
Other than that, this looks fine to me.
-- Steve
> +
> +TRACE_EVENT(hyperv_mmu_flush_tlb_others,
> + TP_PROTO(const struct cpumask *cpus, struct mm_struct *mm,
> + unsigned long addr, unsigned long end),
> + TP_ARGS(cpus, mm, addr, end),
> + TP_STRUCT__entry(
> + __field(unsigned int, ncpus)
> + __field(struct mm_struct *, mm)
> + __field(unsigned long, addr)
> + __field(unsigned long, end)
> + ),
> + TP_fast_assign(__entry->ncpus = cpumask_weight(cpus);
> + __entry->mm = mm;
> + __entry->addr = addr,
> + __entry->end = end),
> + TP_printk("ncpus %d mm %p addr %lx, end %lx",
> + __entry->ncpus, __entry->mm, __entry->addr, __entry->end)
> + );
> +
> +#endif /* CONFIG_HYPERV */
> +
> +#undef TRACE_INCLUDE_PATH
> +#define TRACE_INCLUDE_PATH asm/trace/
> +#undef TRACE_INCLUDE_FILE
> +#define TRACE_INCLUDE_FILE hyperv
> +#endif /* _TRACE_HYPERV_H */
> +
> +/* This part must be outside protection */
> +#include <trace/define_trace.h>
[toc] | [prev] | [next] | [standalone]
| From | Andy Shevchenko <andy.shevchenko@gmail.com> |
|---|---|
| Date | 2017-06-09 20:30 +0200 |
| Message-ID | <tQwPv-4Zo-7@gated-at.bofh.it> |
| In reply to | #1662654 |
On Fri, Jun 9, 2017 at 9:04 PM, Steven Rostedt <rostedt@goodmis.org> wrote: > On Fri, 9 Jun 2017 15:27:36 +0200 > Vitaly Kuznetsov <vkuznets@redhat.com> wrote: >> +#if IS_ENABLED(CONFIG_HYPERV) > > Hmm, this is new to me. I know you can use IS_ENABLED() inside C code, > but I've never seen it used for preprocessor directives. This usually > is just: > > #ifdef CONFIG_HYPERV > > Other than that, this looks fine to me. That is the magic of IS_ENABLED(), IS_BUILTIN() macros. So, the code above is fine as is. -- With Best Regards, Andy Shevchenko
[toc] | [prev] | [next] | [standalone]
| From | Steven Rostedt <rostedt@goodmis.org> |
|---|---|
| Date | 2017-06-09 20:40 +0200 |
| Subject | Re: [PATCH v8 10/10] tracing/hyper-v: trace hyperv_mmu_flush_tlb_others() |
| Message-ID | <tQwZc-52A-5@gated-at.bofh.it> |
| In reply to | #1662661 |
On Fri, 9 Jun 2017 21:23:52 +0300 Andy Shevchenko <andy.shevchenko@gmail.com> wrote: > On Fri, Jun 9, 2017 at 9:04 PM, Steven Rostedt <rostedt@goodmis.org> wrote: > > On Fri, 9 Jun 2017 15:27:36 +0200 > > Vitaly Kuznetsov <vkuznets@redhat.com> wrote: > > > >> +#if IS_ENABLED(CONFIG_HYPERV) > > > > Hmm, this is new to me. I know you can use IS_ENABLED() inside C code, > > but I've never seen it used for preprocessor directives. This usually > > is just: > > > > #ifdef CONFIG_HYPERV > > > > Other than that, this looks fine to me. > > That is the magic of IS_ENABLED(), IS_BUILTIN() macros. > So, the code above is fine as is. > I'm sure it works, but it just adds one more way of doing the same thing. I thought that was what perl was always criticized for, and why people usually prefer python. Don't get me wrong, I prefer oysters over snakes. But I just wanted to point out the lack of consistency here. -- Steve
[toc] | [prev] | [next] | [standalone]
| From | Andy Shevchenko <andy.shevchenko@gmail.com> |
|---|---|
| Date | 2017-06-09 20:50 +0200 |
| Message-ID | <tQx8S-55Y-19@gated-at.bofh.it> |
| In reply to | #1662666 |
On Fri, Jun 9, 2017 at 9:32 PM, Steven Rostedt <rostedt@goodmis.org> wrote: > On Fri, 9 Jun 2017 21:23:52 +0300 > Andy Shevchenko <andy.shevchenko@gmail.com> wrote: > I'm sure it works, but it just adds one more way of doing the same > thing. I thought that was what perl was always criticized for, and why > people usually prefer python. Don't get me wrong, I prefer oysters over > snakes. But I just wanted to point out the lack of consistency here. Fair enough. -- With Best Regards, Andy Shevchenko
[toc] | [prev] | [next] | [standalone]
| From | Paul Bolle <pebolle@tiscali.nl> |
|---|---|
| Date | 2017-06-09 21:00 +0200 |
| Subject | Re: [PATCH v8 10/10] tracing/hyper-v: trace hyperv_mmu_flush_tlb_others() |
| Message-ID | <tQxiy-59d-25@gated-at.bofh.it> |
| In reply to | #1662666 |
On Fri, 2017-06-09 at 14:32 -0400, Steven Rostedt wrote:
> I'm sure it works, but it just adds one more way of doing the same
> thing. I thought that was what perl was always criticized for, and why
> people usually prefer python. Don't get me wrong, I prefer oysters over
> snakes. But I just wanted to point out the lack of consistency here.
A major benefit is that
#if IS_ENABLED(CONFIG_HYPERV)
is shorter than
#if defined(CONFIG_HYPERV) || defined(CONFIG_HYPERV_MODULE)
and less prone to typos.
Paul Bolle
[toc] | [prev] | [next] | [standalone]
| From | Steven Rostedt <rostedt@goodmis.org> |
|---|---|
| Date | 2017-06-09 21:10 +0200 |
| Subject | Re: [PATCH v8 10/10] tracing/hyper-v: trace hyperv_mmu_flush_tlb_others() |
| Message-ID | <tQxse-5ru-11@gated-at.bofh.it> |
| In reply to | #1662694 |
On Fri, 09 Jun 2017 20:53:53 +0200 Paul Bolle <pebolle@tiscali.nl> wrote: > On Fri, 2017-06-09 at 14:32 -0400, Steven Rostedt wrote: > > I'm sure it works, but it just adds one more way of doing the same > > thing. I thought that was what perl was always criticized for, and why > > people usually prefer python. Don't get me wrong, I prefer oysters over > > snakes. But I just wanted to point out the lack of consistency here. > > A major benefit is that > #if IS_ENABLED(CONFIG_HYPERV) > > is shorter than > #if defined(CONFIG_HYPERV) || defined(CONFIG_HYPERV_MODULE) > > and less prone to typos. > I don't believe the module version is needed here. Otherwise I would question the #if altogether. Which now I'm looking at it, why is it needed? What includes this header file that wouldn't have that set anyway? The only place it is included is in: arch/x86/hyperv/mmu.c Is that compiled without CONFIG_HYPERV? -- Steve
[toc] | [prev] | [next] | [standalone]
| From | Vitaly Kuznetsov <vkuznets@redhat.com> |
|---|---|
| Date | 2017-06-12 05:00 +0200 |
| Message-ID | <tRnK9-4Ot-1@gated-at.bofh.it> |
| In reply to | #1662697 |
Steven Rostedt <rostedt@goodmis.org> writes: > On Fri, 09 Jun 2017 20:53:53 +0200 > Paul Bolle <pebolle@tiscali.nl> wrote: > >> On Fri, 2017-06-09 at 14:32 -0400, Steven Rostedt wrote: >> > I'm sure it works, but it just adds one more way of doing the same >> > thing. I thought that was what perl was always criticized for, and why >> > people usually prefer python. Don't get me wrong, I prefer oysters over >> > snakes. But I just wanted to point out the lack of consistency here. >> >> A major benefit is that >> #if IS_ENABLED(CONFIG_HYPERV) >> >> is shorter than >> #if defined(CONFIG_HYPERV) || defined(CONFIG_HYPERV_MODULE) >> >> and less prone to typos. >> > > I don't believe the module version is needed here. Otherwise I would > question the #if altogether. Which now I'm looking at it, why is it > needed? > > What includes this header file that wouldn't have that set anyway? The > only place it is included is in: > > arch/x86/hyperv/mmu.c > > Is that compiled without CONFIG_HYPERV? No, it is not but as was already mentioned it is valid and common to have CONFIG_HYPERV=m (we should've probably done things differently in the past; CONFIG_HYPERV=y/n should've been used for indicating Hyper-V support and something like CONFIG_HYPERV_VMBUS=y/m/n to say if we want to have vmbus as a module but...). arch/x86/hyperv/mmu.c is compiled in vmlinux when CONFIG_HYPERV is enabled in any way, this is updated in PATCH8 of this series: diff --git a/arch/x86/Kbuild b/arch/x86/Kbuild index 586b786..3e6f640 100644 --- a/arch/x86/Kbuild +++ b/arch/x86/Kbuild @@ -8,7 +8,7 @@ obj-$(CONFIG_KVM) += kvm/ obj-$(CONFIG_XEN) += xen/ # Hyper-V paravirtualization support -obj-$(CONFIG_HYPERVISOR_GUEST) += hyperv/ +obj-$(subst m,y,$(CONFIG_HYPERV)) += hyperv/ # lguest paravirtualization support obj-$(CONFIG_LGUEST_GUEST) += lguest/ (it was Andy who suggested we use 'subst', not me :-) So we can't just change IS_ENABLED -> ifdef in this patch. We can, of course, write " #if defined(CONFIG_HYPERV) || defined(CONFIG_HYPERV_MODULE)" but we were replacing it with IF_ENABLED in the past, not sure we should do that. Dropping the #if altogether is possible, but why having it when CONFIG_HYPERV=n? -- Vitaly
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web