| Message ID | 20191223164329.3113378-3-george.dunlap@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | x86: Remove force-invalidate loop from relinqusish_memory | expand |
On 23/12/2019 16:43, George Dunlap 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 This is pandoc, not markdown, and underscores like these are one of the differences. I specifically took these underscores out so the HTML anchor links work correctly. > , and > remove some trailing whitespace. > > Signed-off-by: George Dunlap <george.dunlap@citrix.com> > --- > CC: Andrew Cooper <andrew.cooper3@citrix.com> > CC: Jan Beulich <jbeulich@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. Please, not like this. I learnt the hard way with hvm_fep that it is important to have functionality like this in release builds as well. Give it a Kconfig option, and default it to DEBUG. ~Andrew
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@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.dunlap@citrix.com> > --- > CC: Andrew Cooper <andrew.cooper3@citrix.com> > CC: Jan Beulich <jbeulich@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
On 23.12.2019 17:43, George Dunlap wrote: > --- 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. If this is meant to be x86 only, then text here should state this. Otherwise I think it would help if the example described in the last sentence would mention its x86 relationship. > --- 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; static unsigned int __read_mostly? > +integer_param("debug-synthetic-preemption", debug_synthetic_preemption); Perhaps allow changing at runtime? > +bool synthetic_preemption_check(void) { Brace placement. > + if ( debug_synthetic_preemption == 0 ) > + return false; > + return !(rdtsc() % debug_synthetic_preemption); Please consistently use either ! or "== 0". Jan
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 +#else +#define synthetic_preempiton_check() false +#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() \ )) /*
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@citrix.com> --- CC: Andrew Cooper <andrew.cooper3@citrix.com> CC: Jan Beulich <jbeulich@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(-)