[RFC][PATCH 0/7] sched: Remove finish_arch_switch()

[RFC][PATCH 0/7] sched: Remove finish_arch_switch()

[Peter Zijlstra]

Hi,

There were only a few architectures utilizing finish_arch_switch() and it looks
like they can all do without.

This means we can remove finish_arch_switch(); partly to make up for
introducing finish_arch_post_lock_switch(), partly to reduce the number of arch
hooks to consider.

Much thanks to Ralf, Will and Chris for helping out with this!

[RFC][PATCH 1/7] arm: Remove finish_arch_switch

[Peter Zijlstra]

[-- Attachment #1: will_deacon-finish_arch_switch.patch --]
[-- Type: text/plain, Size: 825 bytes --]

From: Will Deacon <will.deacon@arm.com>

Fold finish_arch_switch() into switch_to().

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 arch/arm/include/asm/switch_to.h |    5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

--- a/arch/arm/include/asm/switch_to.h
+++ b/arch/arm/include/asm/switch_to.h
@@ -10,7 +10,9 @@
  * CPU.
  */
 #if defined(CONFIG_PREEMPT) && defined(CONFIG_SMP) && defined(CONFIG_CPU_V7)
-#define finish_arch_switch(prev)	dsb(ish)
+#define __complete_pending_tlbi()	dsb(ish)
+#else
+#define __complete_pending_tlbi()
 #endif
 
 /*
@@ -22,6 +24,7 @@ extern struct task_struct *__switch_to(s
 
 #define switch_to(prev,next,last)					\
 do {									\
+	__complete_pending_tlbi();					\
 	last = __switch_to(prev,task_thread_info(prev), task_thread_info(next));	\
 } while (0)
 

Re: [RFC][PATCH 1/7] arm: Remove finish_arch_switch

[Ingo Molnar]

* Peter Zijlstra <peterz@infradead.org> wrote:

> From: Will Deacon <will.deacon@arm.com>
> 
> Fold finish_arch_switch() into switch_to().
> 
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>

Will, can I add your:

 Signed-off-by: Will Deacon <will.deacon@arm.com>

?

Thanks,

	Ingo

Re: [RFC][PATCH 1/7] arm: Remove finish_arch_switch

[Russell King - ARM Linux]

On Tue, Aug 04, 2015 at 09:39:43AM +0200, Ingo Molnar wrote:
> 
> * Peter Zijlstra <peterz@infradead.org> wrote:
> 
> > From: Will Deacon <will.deacon@arm.com>
> > 
> > Fold finish_arch_switch() into switch_to().
> > 
> > Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> 
> Will, can I add your:
> 
>  Signed-off-by: Will Deacon <will.deacon@arm.com>

I've already merged this patch as Will had submitted it to me for merging.
Peter's S-o-b should be an acked-by though in that case...

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.

Re: [RFC][PATCH 1/7] arm: Remove finish_arch_switch

[Will Deacon]

On Tue, Aug 04, 2015 at 09:26:22AM +0100, Russell King - ARM Linux wrote:
> On Tue, Aug 04, 2015 at 09:39:43AM +0200, Ingo Molnar wrote:
> > 
> > * Peter Zijlstra <peterz@infradead.org> wrote:
> > 
> > > From: Will Deacon <will.deacon@arm.com>
> > > 
> > > Fold finish_arch_switch() into switch_to().
> > > 
> > > Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> > 
> > Will, can I add your:
> > 
> >  Signed-off-by: Will Deacon <will.deacon@arm.com>
> 
> I've already merged this patch as Will had submitted it to me for merging.

Yup. Peter said he would pick up what's left after the merge window, so
arch maintainers can pick up the patches now for 4.3 (I think Ralf had
picked up the mips part already).

> Peter's S-o-b should be an acked-by though in that case...

I guess, although he turned my diff into a patch (i.e. wrote the commit
message) and posted it, so I just left it there. Not fussed either way.

Will

[RFC][PATCH 2/7] MIPS: Get rid of finish_arch_switch().

[Peter Zijlstra]

[-- Attachment #1: ralf-mips-finish_arch_switch.patch --]
[-- Type: text/plain, Size: 3140 bytes --]

From: Ralf Baechle <ralf@linux-mips.org>

MIPS was using finish_arch_switch() as a hook to restore and initialize
CPU context for all threads, even newly created kernel and user threads.
This is however entirely solvable within switch_to() so get rid of
finish_arch_switch() which is in the way of scheduler cleanups.

Signed-off-by: Ralf Baechle <ralf@linux-mips.org>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 arch/mips/include/asm/switch_to.h | 48 +++++++++++++++++++--------------------
 1 file changed, 23 insertions(+), 25 deletions(-)

diff --git a/arch/mips/include/asm/switch_to.h b/arch/mips/include/asm/switch_to.h
index 7163cd7..9733cd0 100644
--- a/arch/mips/include/asm/switch_to.h
+++ b/arch/mips/include/asm/switch_to.h
@@ -83,45 +83,43 @@ do {	if (cpu_has_rw_llb) {						\
 	}								\
 } while (0)
 
+/*
+ * For newly created kernel threads switch_to() will return to
+ * ret_from_kernel_thread, newly created user threads to ret_from_fork.
+ * That is, everything following resume() will be skipped for new threads.
+ * So everything that matters to new threads should be placed before resume().
+ */
 #define switch_to(prev, next, last)					\
 do {									\
-	u32 __c0_stat;							\
 	s32 __fpsave = FP_SAVE_NONE;					\
 	__mips_mt_fpaff_switch_to(prev);				\
-	if (cpu_has_dsp)						\
+	if (cpu_has_dsp) {						\
 		__save_dsp(prev);					\
-	if (cop2_present && (KSTK_STATUS(prev) & ST0_CU2)) {		\
-		if (cop2_lazy_restore)					\
-			KSTK_STATUS(prev) &= ~ST0_CU2;			\
-		__c0_stat = read_c0_status();				\
-		write_c0_status(__c0_stat | ST0_CU2);			\
-		cop2_save(prev);					\
-		write_c0_status(__c0_stat & ~ST0_CU2);			\
+		__restore_dsp(next);					\
+	}								\
+	if (cop2_present) {						\
+		set_c0_status(ST0_CU2);					\
+		if ((KSTK_STATUS(prev) & ST0_CU2)) {			\
+			if (cop2_lazy_restore)				\
+				KSTK_STATUS(prev) &= ~ST0_CU2;		\
+			cop2_save(prev);				\
+		}							\
+		if (KSTK_STATUS(next) & ST0_CU2 &&			\
+		    !cop2_lazy_restore) {				\
+			cop2_restore(next);				\
+		}							\
+		clear_c0_status(ST0_CU2);				\
 	}								\
 	__clear_software_ll_bit();					\
 	if (test_and_clear_tsk_thread_flag(prev, TIF_USEDFPU))		\
 		__fpsave = FP_SAVE_SCALAR;				\
 	if (test_and_clear_tsk_thread_flag(prev, TIF_USEDMSA))		\
 		__fpsave = FP_SAVE_VECTOR;				\
-	(last) = resume(prev, next, task_thread_info(next), __fpsave);	\
-} while (0)
-
-#define finish_arch_switch(prev)					\
-do {									\
-	u32 __c0_stat;							\
-	if (cop2_present && !cop2_lazy_restore &&			\
-			(KSTK_STATUS(current) & ST0_CU2)) {		\
-		__c0_stat = read_c0_status();				\
-		write_c0_status(__c0_stat | ST0_CU2);			\
-		cop2_restore(current);					\
-		write_c0_status(__c0_stat & ~ST0_CU2);			\
-	}								\
-	if (cpu_has_dsp)						\
-		__restore_dsp(current);					\
 	if (cpu_has_userlocal)						\
-		write_c0_userlocal(current_thread_info()->tp_value);	\
+		write_c0_userlocal(task_thread_info(next)->tp_value);	\
 	__restore_watch();						\
 	disable_msa();							\
+	(last) = resume(prev, next, task_thread_info(next), __fpsave);	\
 } while (0)
 
 #endif /* _ASM_SWITCH_TO_H */
-- 
cgit v0.11.2

[RFC][PATCH 3/7] avr32: Remove finish_arch_switch()

[Peter Zijlstra]

[-- Attachment #1: peterz-avr32-finish_arch_switch.patch --]
[-- Type: text/plain, Size: 1016 bytes --]

Fold the tracing hook into switch_to() in order to remove
finish_arch_switch().

Cc: Hans-Christian Egtvedt <egtvedt@samfundet.no>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 arch/avr32/include/asm/switch_to.h |    7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

--- a/arch/avr32/include/asm/switch_to.h
+++ b/arch/avr32/include/asm/switch_to.h
@@ -15,11 +15,13 @@
  */
 #ifdef CONFIG_OWNERSHIP_TRACE
 #include <asm/ocd.h>
-#define finish_arch_switch(prev)			\
+#define ocd_switch(prev, next)				\
 	do {						\
 		ocd_write(PID, prev->pid);		\
-		ocd_write(PID, current->pid);		\
+		ocd_write(PID, next->pid);		\
 	} while(0)
+#else
+#define ocd_switch(prev, next)
 #endif
 
 /*
@@ -38,6 +40,7 @@ extern struct task_struct *__switch_to(s
 				       struct cpu_context *);
 #define switch_to(prev, next, last)					\
 	do {								\
+		ocd_switch(prev, next);					\
 		last = __switch_to(prev, &prev->thread.cpu_context + 1,	\
 				   &next->thread.cpu_context);		\
 	} while (0)

Re: [RFC][PATCH 3/7] avr32: Remove finish_arch_switch()

[Hans-Christian Egtvedt]

Around Wed 29 Jul 2015 21:08:51 +0200 or thereabout, Peter Zijlstra wrote:
> Fold the tracing hook into switch_to() in order to remove
> finish_arch_switch().

AFAICT this should work, I do not have a nexus debugger to verify behavior.

> Cc: Hans-Christian Egtvedt <egtvedt@samfundet.no>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>

Acked-by: Hans-Christian Egtvedt <egtvedt@samfundet.no>

> ---
>  arch/avr32/include/asm/switch_to.h |    7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> --- a/arch/avr32/include/asm/switch_to.h
> +++ b/arch/avr32/include/asm/switch_to.h
> @@ -15,11 +15,13 @@
>   */
>  #ifdef CONFIG_OWNERSHIP_TRACE
>  #include <asm/ocd.h>
> -#define finish_arch_switch(prev)			\
> +#define ocd_switch(prev, next)				\
>  	do {						\
>  		ocd_write(PID, prev->pid);		\
> -		ocd_write(PID, current->pid);		\
> +		ocd_write(PID, next->pid);		\
>  	} while(0)
> +#else
> +#define ocd_switch(prev, next)
>  #endif
>  
>  /*
> @@ -38,6 +40,7 @@ extern struct task_struct *__switch_to(s
>  				       struct cpu_context *);
>  #define switch_to(prev, next, last)					\
>  	do {								\
> +		ocd_switch(prev, next);					\
>  		last = __switch_to(prev, &prev->thread.cpu_context + 1,	\
>  				   &next->thread.cpu_context);		\
>  	} while (0)
-- 
mvh
Hans-Christian Egtvedt

[RFC][PATCH 4/7] score: Remove finish_arch_switch()

[Peter Zijlstra]

[-- Attachment #1: peterz-score-finish_arch_switch.patch --]
[-- Type: text/plain, Size: 462 bytes --]

An empty implementation, make it go away.

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 arch/score/include/asm/switch_to.h |    2 --
 1 file changed, 2 deletions(-)

--- a/arch/score/include/asm/switch_to.h
+++ b/arch/score/include/asm/switch_to.h
@@ -8,6 +8,4 @@ do {								\
 	(last) = resume(prev, next, task_thread_info(next));	\
 } while (0)
 
-#define finish_arch_switch(prev)	do {} while (0)
-
 #endif /* _ASM_SCORE_SWITCH_TO_H */

[RFC][PATCH 5/7] sh: Fold finish_arch_switch() into switch_to()

[Peter Zijlstra]

[-- Attachment #1: peterz-sh-finish_arch_switch.patch --]
[-- Type: text/plain, Size: 988 bytes --]

The code looks buggy; why would we be restoring the previous task's
DSP state after we've switched to the next task?

Fix that and put the restore in switch_to(), removing the need for
finish_arch_switch().

Cc: michael@amarulasolutions.com
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 arch/sh/include/asm/switch_to_32.h |    8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

--- a/arch/sh/include/asm/switch_to_32.h
+++ b/arch/sh/include/asm/switch_to_32.h
@@ -78,6 +78,8 @@ do {								\
 								\
 	if (is_dsp_enabled(prev))				\
 		__save_dsp(prev);				\
+	if (is_dsp_enabled(next))				\
+		__restore_dsp(next);				\
 								\
 	__ts1 = (u32 *)&prev->thread.sp;			\
 	__ts2 = (u32 *)&prev->thread.pc;			\
@@ -125,10 +127,4 @@ do {								\
 	last = __last;						\
 } while (0)
 
-#define finish_arch_switch(prev)				\
-do {								\
-	if (is_dsp_enabled(prev))				\
-		__restore_dsp(prev);				\
-} while (0)
-
 #endif /* __ASM_SH_SWITCH_TO_32_H */

[RFC][PATCH 6/7] Remove finish_arch_switch

[Peter Zijlstra]

[-- Attachment #1: chris_metcalf-finish_arch_switch.patch --]
[-- Type: text/plain, Size: 2589 bytes --]

From: Chris Metcalf <cmetcalf@ezchip.com>

Move the simulator bits into switch_to() and use
finish_arch_post_lock_switch() for the homecache migration bits.

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 arch/tile/include/asm/switch_to.h |   10 +++-------
 arch/tile/kernel/process.c        |   17 +++++++++++++++--
 2 files changed, 18 insertions(+), 9 deletions(-)

--- a/arch/tile/include/asm/switch_to.h
+++ b/arch/tile/include/asm/switch_to.h
@@ -53,15 +53,11 @@ extern unsigned long get_switch_to_pc(vo
  * Kernel threads can check to see if they need to migrate their
  * stack whenever they return from a context switch; for user
  * threads, we defer until they are returning to user-space.
+ * We defer homecache migration until the runqueue lock is released.
  */
-#define finish_arch_switch(prev) do {                                     \
-	if (unlikely((prev)->state == TASK_DEAD))                         \
-		__insn_mtspr(SPR_SIM_CONTROL, SIM_CONTROL_OS_EXIT |       \
-			((prev)->pid << _SIM_CONTROL_OPERATOR_BITS));     \
-	__insn_mtspr(SPR_SIM_CONTROL, SIM_CONTROL_OS_SWITCH |             \
-		(current->pid << _SIM_CONTROL_OPERATOR_BITS));            \
+#define finish_arch_post_lock_switch() do {                               \
 	if (current->mm == NULL && !kstack_hash &&                        \
-	    current_thread_info()->homecache_cpu != smp_processor_id())   \
+	    current_thread_info()->homecache_cpu != raw_smp_processor_id()) \
 		homecache_migrate_kthread();                              \
 } while (0)
 
--- a/arch/tile/kernel/process.c
+++ b/arch/tile/kernel/process.c
@@ -448,11 +448,24 @@ struct task_struct *__sched _switch_to(s
 
 	/*
 	 * Switch kernel SP, PC, and callee-saved registers.
+	 * Pass the value to use for SYSTEM_SAVE_K_0 when we reset our sp.
+	 * Once we return from this function we will have changed stacks
+	 * and be running with current == next.
+	 */
+	__switch_to(prev, next, next_current_ksp0(next));
+
+	/* Notify the simulator of task switch and task exit. */
+	if (unlikely(prev->state == TASK_DEAD))
+		__insn_mtspr(SPR_SIM_CONTROL, SIM_CONTROL_OS_EXIT |
+			     (prev->pid << _SIM_CONTROL_OPERATOR_BITS));
+	__insn_mtspr(SPR_SIM_CONTROL, SIM_CONTROL_OS_SWITCH |
+		(next->pid << _SIM_CONTROL_OPERATOR_BITS));
+
+	/*
 	 * In the context of the new task, return the old task pointer
 	 * (i.e. the task that actually called __switch_to).
-	 * Pass the value to use for SYSTEM_SAVE_K_0 when we reset our sp.
 	 */
-	return __switch_to(prev, next, next_current_ksp0(next));
+	return prev;
 }
 
 /*

[RFC][PATCH 7/7] sched: Remove finish_arch_switch()

[Peter Zijlstra]

[-- Attachment #1: peterz-sched-remove-finish_arch_switch.patch --]
[-- Type: text/plain, Size: 899 bytes --]

One less arch hook..

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 kernel/sched/core.c  |    1 -
 kernel/sched/sched.h |    3 ---
 2 files changed, 4 deletions(-)

--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -2468,7 +2468,6 @@ static struct rq *finish_task_switch(str
 	 */
 	prev_state = prev->state;
 	vtime_task_switch(prev);
-	finish_arch_switch(prev);
 	perf_event_task_sched_in(prev, current);
 	finish_lock_switch(rq, prev);
 	finish_arch_post_lock_switch();
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -1055,9 +1055,6 @@ static inline int task_on_rq_migrating(s
 #ifndef prepare_arch_switch
 # define prepare_arch_switch(next)	do { } while (0)
 #endif
-#ifndef finish_arch_switch
-# define finish_arch_switch(prev)	do { } while (0)
-#endif
 #ifndef finish_arch_post_lock_switch
 # define finish_arch_post_lock_switch()	do { } while (0)
 #endif

Re: [RFC][PATCH 0/7] sched: Remove finish_arch_switch()

[Peter Zijlstra]

On Wed, Jul 29, 2015 at 09:08:48PM +0200, Peter Zijlstra wrote:
> Hi,
> 
> There were only a few architectures utilizing finish_arch_switch() and it looks
> like they can all do without.
> 
> This means we can remove finish_arch_switch(); partly to make up for
> introducing finish_arch_post_lock_switch(), partly to reduce the number of arch
> hooks to consider.
> 
> Much thanks to Ralf, Will and Chris for helping out with this!

So the mips patch is already in Ralf's mips-next tree, which means I
have to wait for that to his Linus' tree before I can commit the last
patch in this series.

It also means that all the other patches can go through their respective
arch tree.

So anybody who feels so inclined, go ahead, I'll check after the next
merge window and commit everything that's left -- unless there's
objections of course :-)

Re: [RFC][PATCH 0/7] sched: Remove finish_arch_switch()

[Ingo Molnar]

* Peter Zijlstra <peterz@infradead.org> wrote:

> On Wed, Jul 29, 2015 at 09:08:48PM +0200, Peter Zijlstra wrote:
> > Hi,
> > 
> > There were only a few architectures utilizing finish_arch_switch() and it looks
> > like they can all do without.
> > 
> > This means we can remove finish_arch_switch(); partly to make up for
> > introducing finish_arch_post_lock_switch(), partly to reduce the number of arch
> > hooks to consider.
> > 
> > Much thanks to Ralf, Will and Chris for helping out with this!
> 
> So the mips patch is already in Ralf's mips-next tree, which means I
> have to wait for that to his Linus' tree before I can commit the last
> patch in this series.
> 
> It also means that all the other patches can go through their respective
> arch tree.
> 
> So anybody who feels so inclined, go ahead, I'll check after the next
> merge window and commit everything that's left -- unless there's
> objections of course :-)

Sounds sensible to me!

Thanks,

	Ingo