On context switch we are locking the vtime seqcount of the scheduling-out task twice: * On vtime_task_switch_common(), when we flush the pending vtime through vtime_account_system() * On arch_vtime_task_switch() to reset the vtime state. This is pointless as these actions can be performed without the need to unlock/lock in the middle. The reason these steps are separated is to consolidate a very small amount of common code between CONFIG_VIRT_CPU_ACCOUNTING_GEN and CONFIG_VIRT_CPU_ACCOUNTING_NATIVE. Performance in this fast path is definitely a priority over artificial code factorization so split the task switch code between GEN and NATIVE and mutualize the parts than can run under a single seqcount locked block. As a side effect, vtime_account_idle() becomes included in the seqcount protection. This happens to be a welcome preparation in order to properly support kcpustat under vtime in the future and fetch CPUTIME_IDLE without race.
Unified · Split
| @@ -14,8 +14,12 @@ struct task_struct; · 13 unmodified lines | |||
| 14 | * vtime_accounting_cpu_enabled() definitions/declarations | 14 | * vtime_accounting_cpu_enabled() definitions/declarations |
| 15 | */ | 15 | */ |
| 16 | #if defined(CONFIG_VIRT_CPU_ACCOUNTING_NATIVE) | 16 | #if defined(CONFIG_VIRT_CPU_ACCOUNTING_NATIVE) |
| 17 | |||
| 17 | static inline bool vtime_accounting_cpu_enabled(void) { return true; } | 18 | static inline bool vtime_accounting_cpu_enabled(void) { return true; } |
| 19 | extern void vtime_task_switch(struct task_struct *prev); | ||
| 20 | |||
| 18 | #elif defined(CONFIG_VIRT_CPU_ACCOUNTING_GEN) | 21 | #elif defined(CONFIG_VIRT_CPU_ACCOUNTING_GEN) |
| 22 | |||
| 19 | /* | 23 | /* |
| 20 | * Checks if vtime is enabled on some CPU. Cputime readers want to be careful | 24 | * Checks if vtime is enabled on some CPU. Cputime readers want to be careful |
| 21 | * in that case and compute the tickless cputime. | 25 | * in that case and compute the tickless cputime. |
| @@ -36,33 +40,29 @@ static inline bool vtime_accounting_cpu_enabled(void) · 14 unmodified lines | |||
| 36 | 40 | ||
| 37 | return false; | 41 | return false; |
| 38 | } | 42 | } |
| 43 | |||
| 44 | extern void vtime_task_switch_generic(struct task_struct *prev); | ||
| 45 | |||
| 46 | static inline void vtime_task_switch(struct task_struct *prev) | ||
| 47 | { | ||
| 48 | if (vtime_accounting_cpu_enabled()) | ||
| 49 | vtime_task_switch_generic(prev); | ||
| 50 | } | ||
| 51 | |||
| 39 | #else /* !CONFIG_VIRT_CPU_ACCOUNTING */ | 52 | #else /* !CONFIG_VIRT_CPU_ACCOUNTING */ |
| 53 | |||
| 40 | static inline bool vtime_accounting_cpu_enabled(void) { return false; } | 54 | static inline bool vtime_accounting_cpu_enabled(void) { return false; } |
| 41 | #endif | 55 | static inline void vtime_task_switch(struct task_struct *prev) { } |
| 42 | 56 | ||
| 57 | #endif | ||
| 43 | 58 | ||
| 44 | /* | 59 | /* |
| 45 | * Common vtime APIs | 60 | * Common vtime APIs |
| 46 | */ | 61 | */ |
| 47 | #ifdef CONFIG_VIRT_CPU_ACCOUNTING | 62 | #ifdef CONFIG_VIRT_CPU_ACCOUNTING |
| 48 | |||
| 49 | #ifdef __ARCH_HAS_VTIME_TASK_SWITCH | ||
| 50 | extern void vtime_task_switch(struct task_struct *prev); | ||
| 51 | #else | ||
| 52 | extern void vtime_common_task_switch(struct task_struct *prev); | ||
| 53 | static inline void vtime_task_switch(struct task_struct *prev) | ||
| 54 | { | ||
| 55 | if (vtime_accounting_cpu_enabled()) | ||
| 56 | vtime_common_task_switch(prev); | ||
| 57 | } | ||
| 58 | #endif /* __ARCH_HAS_VTIME_TASK_SWITCH */ | ||
| 59 | |||
| 60 | extern void vtime_account_kernel(struct task_struct *tsk); | 63 | extern void vtime_account_kernel(struct task_struct *tsk); |
| 61 | extern void vtime_account_idle(struct task_struct *tsk); | 64 | extern void vtime_account_idle(struct task_struct *tsk); |
| 62 | |||
| 63 | #else /* !CONFIG_VIRT_CPU_ACCOUNTING */ | 65 | #else /* !CONFIG_VIRT_CPU_ACCOUNTING */ |
| 64 | |||
| 65 | static inline void vtime_task_switch(struct task_struct *prev) { } | ||
| 66 | static inline void vtime_account_kernel(struct task_struct *tsk) { } | 66 | static inline void vtime_account_kernel(struct task_struct *tsk) { } |
| 67 | #endif /* !CONFIG_VIRT_CPU_ACCOUNTING */ | 67 | #endif /* !CONFIG_VIRT_CPU_ACCOUNTING */ |
| 68 | 68 | ||
| @@ -405,9 +405,10 @@ static inline void irqtime_account_process_tick(struct task_struct *p, int user_ · 404 unmodified lines | |||
| 405 | /* | 405 | /* |
| 406 | * Use precise platform statistics if available: | 406 | * Use precise platform statistics if available: |
| 407 | */ | 407 | */ |
| 408 | #ifdef CONFIG_VIRT_CPU_ACCOUNTING | 408 | #ifdef CONFIG_VIRT_CPU_ACCOUNTING_NATIVE |
| 409 | |||
| 409 | # ifndef __ARCH_HAS_VTIME_TASK_SWITCH | 410 | # ifndef __ARCH_HAS_VTIME_TASK_SWITCH |
| 410 | void vtime_common_task_switch(struct task_struct *prev) | 411 | void vtime_task_switch(struct task_struct *prev) |
| 411 | { | 412 | { |
| 412 | if (is_idle_task(prev)) | 413 | if (is_idle_task(prev)) |
| 413 | vtime_account_idle(prev); | 414 | vtime_account_idle(prev); |
| @@ -418,10 +419,7 @@ void vtime_common_task_switch(struct task_struct *prev) · 4 unmodified lines | |||
| 418 | arch_vtime_task_switch(prev); | 419 | arch_vtime_task_switch(prev); |
| 419 | } | 420 | } |
| 420 | # endif | 421 | # endif |
| 421 | #endif /* CONFIG_VIRT_CPU_ACCOUNTING */ | ||
| 422 | |||
| 423 | 422 | ||
| 424 | #ifdef CONFIG_VIRT_CPU_ACCOUNTING_NATIVE | ||
| 425 | /* | 423 | /* |
| 426 | * Archs that account the whole time spent in the idle task | 424 | * Archs that account the whole time spent in the idle task |
| 427 | * (outside irq) as idle time can rely on this and just implement | 425 | * (outside irq) as idle time can rely on this and just implement |
| @@ -731,6 +729,16 @@ static void vtime_account_guest(struct task_struct *tsk, · 303 unmodified lines | |||
| 731 | } | 729 | } |
| 732 | } | 730 | } |
| 733 | 731 | ||
| 732 | static void __vtime_account_kernel(struct task_struct *tsk, | ||
| 733 | struct vtime *vtime) | ||
| 734 | { | ||
| 735 | /* We might have scheduled out from guest path */ | ||
| 736 | if (tsk->flags & PF_VCPU) | ||
| 737 | vtime_account_guest(tsk, vtime); | ||
| 738 | else | ||
| 739 | vtime_account_system(tsk, vtime); | ||
| 740 | } | ||
| 741 | |||
| 734 | void vtime_account_kernel(struct task_struct *tsk) | 742 | void vtime_account_kernel(struct task_struct *tsk) |
| 735 | { | 743 | { |
| 736 | struct vtime *vtime = &tsk->vtime; | 744 | struct vtime *vtime = &tsk->vtime; |
| @@ -739,11 +747,7 @@ void vtime_account_kernel(struct task_struct *tsk) · 2 unmodified lines | |||
| 739 | return; | 747 | return; |
| 740 | 748 | ||
| 741 | write_seqcount_begin(&vtime->seqcount); | 749 | write_seqcount_begin(&vtime->seqcount); |
| 742 | /* We might have scheduled out from guest path */ | 750 | __vtime_account_kernel(tsk, vtime); |
| 743 | if (tsk->flags & PF_VCPU) | ||
| 744 | vtime_account_guest(tsk, vtime); | ||
| 745 | else | ||
| 746 | vtime_account_system(tsk, vtime); | ||
| 747 | write_seqcount_end(&vtime->seqcount); | 751 | write_seqcount_end(&vtime->seqcount); |
| 748 | } | 752 | } |
| 749 | 753 | ||
| @@ -804,11 +808,15 @@ void vtime_account_idle(struct task_struct *tsk) · 54 unmodified lines | |||
| 804 | account_idle_time(get_vtime_delta(&tsk->vtime)); | 808 | account_idle_time(get_vtime_delta(&tsk->vtime)); |
| 805 | } | 809 | } |
| 806 | 810 | ||
| 807 | void arch_vtime_task_switch(struct task_struct *prev) | 811 | void vtime_task_switch_generic(struct task_struct *prev) |
| 808 | { | 812 | { |
| 809 | struct vtime *vtime = &prev->vtime; | 813 | struct vtime *vtime = &prev->vtime; |
| 810 | 814 | ||
| 811 | write_seqcount_begin(&vtime->seqcount); | 815 | write_seqcount_begin(&vtime->seqcount); |
| 816 | if (is_idle_task(prev)) | ||
| 817 | vtime_account_idle(prev); | ||
| 818 | else | ||
| 819 | __vtime_account_kernel(prev, vtime); | ||
| 812 | vtime->state = VTIME_INACTIVE; | 820 | vtime->state = VTIME_INACTIVE; |
| 813 | write_seqcount_end(&vtime->seqcount); | 821 | write_seqcount_end(&vtime->seqcount); |
| 814 | 822 | ||