[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH 2/4] xen: Add 'synthetic' preemption check parameter



Hi George,

I was expecting a bigger list of CC here. What did you use to compute it?

On Mon, 23 Dec 2019, 17:45 George Dunlap, <george.dunlap@xxxxxxxxxx> wrote:
In order to better test hypervisor preemption paths, add an option to
artificially increase the number of preemptions.

While modifying xen-command-line.pandoc, escape some underscores, and
remove some trailing whitespace.

Signed-off-by: George Dunlap <george.dunlap@xxxxxxxxxx>
---
CC: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
CC: Jan Beulich <jbeulich@xxxxxxxx>
---
 docs/misc/xen-command-line.pandoc | 20 ++++++++++++++++++--
 xen/arch/x86/time.c               | 11 +++++++++++
 xen/include/xen/sched.h           | 10 +++++++++-
 3 files changed, 38 insertions(+), 3 deletions(-)

diff --git a/docs/misc/xen-command-line.pandoc b/docs/misc/xen-command-line.pandoc
index 981a5e2381..1a9fda8627 100644
--- a/docs/misc/xen-command-line.pandoc
+++ b/docs/misc/xen-command-line.pandoc
@@ -636,13 +636,29 @@ Available alternatives, with their meaning, are:
 Specify the USB controller to use, either by instance number (when going
 over the PCI busses sequentially) or by PCI device (must be on segment 0).

-### debug_stack_lines
+### debug\_stack\_lines
 > `= <integer>`

 > Default: `20`

 Limits the number lines printed in Xen stack traces.

+### debug-synthetic-preemption
+> `= <integer>`
+
+> Default: `0`
+
+Artificially increases rate at which `hypercall_preempt_check()`
+returns `true`, for debugging purposes, to a rate of one in `N`. (The
+default, `0`, disables the feature.)
+
+When promoting pagetables, for instance, `hypercall_preempt_check()`
+is called before processing each PTE.  Since there are 512 PTEs per
+page, a value of `1024` should result in pagetable promotion being
+interrupted every other page on average.
+
+Only available in DEBUG builds.
+
 ### debugtrace
 > `= [cpu:]<size>`

@@ -1690,7 +1706,7 @@ The following resources are available:
     CDP, one COS will corespond two CBMs other than one with CAT, due to the
     sum of CBMs is fixed, that means actual `cos_max` in use will automatically
     reduce to half when CDP is enabled.
-       
+
 ### pv-linear-pt (x86)
 > `= <boolean>`

diff --git a/xen/arch/x86/time.c b/xen/arch/x86/time.c
index 64e471a39b..34302f81e7 100644
--- a/xen/arch/x86/time.c
+++ b/xen/arch/x86/time.c
@@ -43,6 +43,17 @@
 static char __initdata opt_clocksource[10];
 string_param("clocksource", opt_clocksource);

+#ifndef NDEBUG
+int debug_synthetic_preemption = 0;
+integer_param("debug-synthetic-preemption", debug_synthetic_preemption);
+
+bool synthetic_preemption_check(void) {
+    if ( debug_synthetic_preemption == 0 )
+        return false;
+    return !(rdtsc() % debug_synthetic_preemption);
+}
+#endif
+
 unsigned long __read_mostly cpu_khz;  /* CPU clock frequency in kHz. */
 DEFINE_SPINLOCK(rtc_lock);
 unsigned long pit0_ticks;
diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index 9f7bc69293..c0071eee04 100644
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -748,6 +748,13 @@ static inline void hypercall_cancel_continuation(struct vcpu *v)
     v->hcall_preempted = false;
 }

+#ifndef NDEBUG
+bool synthetic_preemption_check(void);
+#define synthetic_preemption_check synthetic_preemption_check

Why do you need this define?

+#else
+#define synthetic_preempiton_check() false

Typo in the name. Also, it seems like this wasn't tested on Arm and, AFAICT break because the function would not be definr in debug build.

But, I am not sure why the implementation needs to be arch specific when get_cycles() could do the job.

+#endif
+
 /*
  * For long-running operations that must be in hypercall context, check
  * if there is background work to be done that should interrupt this
@@ -755,7 +762,8 @@ static inline void hypercall_cancel_continuation(struct vcpu *v)
  */
 #define hypercall_preempt_check() (unlikely(    \
         softirq_pending(smp_processor_id()) |   \
-        local_events_need_delivery()            \
+        local_events_need_delivery() |          \
+        synthetic_preemption_check()            \

The function you return bool, so shouldn't it be ||?

Cheers,

     ))

 /*
--
2.24.0


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.