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.dun...@citrix.com> 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.dun...@citrix.com>
> ---
> CC: Andrew Cooper <andrew.coop...@citrix.com>
> CC: Jan Beulich <jbeul...@suse.com>
> ---
> 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@lists.xenproject.org
> https://lists.xenproject.org/mailman/listinfo/xen-devel
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
