Re: [PATCH v3] xen: add steal_clock support on x86

[Juergen Gross]

On 19/05/16 19:15, Stefano Stabellini wrote:
> On Thu, 19 May 2016, Juergen Gross wrote:
>> The pv_time_ops structure contains a function pointer for the
>> "steal_clock" functionality used only by KVM and Xen on ARM. Xen on x86
>> uses its own mechanism to account for the "stolen" time a thread wasn't
>> able to run due to hypervisor scheduling.
>>
>> Add support in Xen arch independent time handling for this feature by
>> moving it out of the arm arch into drivers/xen and remove the x86 Xen
>> hack.
>>
>> Signed-off-by: Juergen Gross <jgross@xxxxxxxx>
>> ---
>> V3: add #include <asm/paravirt.h> to avoid build error on arm
>> V2: remove the x86 do_stolen_accounting() hack
>> ---
>>  arch/arm/xen/enlighten.c    | 17 ++---------------
>>  arch/x86/xen/time.c         | 44 ++------------------------------------------
>>  drivers/xen/time.c          | 20 ++++++++++++++++++++
>>  include/linux/kernel_stat.h |  1 -
>>  include/xen/xen-ops.h       |  1 +
>>  kernel/sched/cputime.c      | 10 ----------
>>  6 files changed, 25 insertions(+), 68 deletions(-)
>>
>> diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
>> index 75cd734..9163b94 100644
>> --- a/arch/arm/xen/enlighten.c
>> +++ b/arch/arm/xen/enlighten.c
>> @@ -84,19 +84,6 @@ int xen_unmap_domain_gfn_range(struct vm_area_struct *vma,
>>  }
>>  EXPORT_SYMBOL_GPL(xen_unmap_domain_gfn_range);
>>  
>> -static unsigned long long xen_stolen_accounting(int cpu)
>> -{
>> -	struct vcpu_runstate_info state;
>> -
>> -	BUG_ON(cpu != smp_processor_id());
>> -
>> -	xen_get_runstate_snapshot(&state);
>> -
>> -	WARN_ON(state.state != RUNSTATE_running);
>> -
>> -	return state.time[RUNSTATE_runnable] + state.time[RUNSTATE_offline];
>> -}
>> -
>>  static void xen_read_wallclock(struct timespec64 *ts)
>>  {
>>  	u32 version;
>> @@ -355,8 +342,8 @@ static int __init xen_guest_init(void)
>>  
>>  	register_cpu_notifier(&xen_cpu_notifier);
>>  
>> -	pv_time_ops.steal_clock = xen_stolen_accounting;
>> -	static_key_slow_inc(&paravirt_steal_enabled);
>> +	xen_time_setup_guest();
> 
> You can remove
> 
> #include <asm/paravirt.h>
> 
> from headers now I believe

Correct. Done.

>> diff --git a/include/linux/kernel_stat.h b/include/linux/kernel_stat.h
>> index 25a822f..44fda64 100644
>> --- a/include/linux/kernel_stat.h
>> +++ b/include/linux/kernel_stat.h
>> @@ -92,7 +92,6 @@ static inline void account_process_tick(struct task_struct *tsk, int user)
>>  extern void account_process_tick(struct task_struct *, int user);
>>  #endif
>>  
>> -extern void account_steal_ticks(unsigned long ticks);
>>  extern void account_idle_ticks(unsigned long ticks);
>>  
>>  #endif /* _LINUX_KERNEL_STAT_H */
>> diff --git a/include/xen/xen-ops.h b/include/xen/xen-ops.h
>> index 86abe07..5ce51c2 100644
>> --- a/include/xen/xen-ops.h
>> +++ b/include/xen/xen-ops.h
>> @@ -21,6 +21,7 @@ void xen_resume_notifier_unregister(struct notifier_block *nb);
>>  
>>  bool xen_vcpu_stolen(int vcpu);
>>  void xen_setup_runstate_info(int cpu);
>> +void __init xen_time_setup_guest(void);
> 
> please avoid __init in headers

Okay.


Juergen