2016-10-13 12:42:52

by Heiko Carstens

[permalink] [raw]
Subject: [PATCH 0/3] THREAD_INFO_IN_TASK_STRUCT vs generic preemption code

Commit c65eacbe290b ("sched/core: Allow putting thread_info into
task_struct") made struct thread_info a generic struct with only a
single flags member if THREAD_INFO_IN_TASK_STRUCT is selected.

This change however seems to be quite x86 centric, since at least the
generic preemption code (asm-generic/preempt.h) assumes that struct
thread_info also has a preempt_count member, which apparently was not
true for x86.

We could add a bit more ifdefs to solve this problem too, but it seems
to be much simpler to make struct thread_info arch specific
again. This also makes the conversion to THREAD_INFO_IN_TASK_STRUCT a
bit easier for architectures that have a couple of arch specific stuff
in their thread_info definition.

The arch specific stuff _could_ be moved to thread_struct. However
keeping them in thread_info makes it easier: accessing thread_info
members is simple, since it is at the beginning of the task_struct,
while the thread_struct is at the end. At least on s390 the offsets
needed to access members of the thread_struct (with task_struct as
base) are too large for various asm instructions. This is not a
problem when keeping these members within thread_info.

The above is actually the same as the description of the first
patch. The second patch is a simple compile fix and the third one the
s390 conversion to THREAD_INFO_IN_TASK_STRUCT.

Heiko Carstens (3):
sched/core,x86: make struct thread_info arch specific again
sched/preempt: include asm/current.h
s390: move thread_info into task_struct

arch/s390/Kconfig | 1 +
arch/s390/include/asm/lowcore.h | 2 +-
arch/s390/include/asm/thread_info.h | 11 --------
arch/s390/kernel/asm-offsets.c | 17 +++++--------
arch/s390/kernel/entry.S | 51 ++++++++++++++++++-------------------
arch/s390/kernel/head64.S | 5 ++--
arch/s390/kernel/setup.c | 3 +--
arch/s390/kernel/smp.c | 1 -
arch/x86/include/asm/thread_info.h | 9 +++++++
include/asm-generic/preempt.h | 1 +
include/linux/thread_info.h | 11 --------
11 files changed, 47 insertions(+), 65 deletions(-)

--
2.8.4


2016-10-13 13:02:48

by Heiko Carstens

[permalink] [raw]
Subject: [PATCH 1/3] sched/core,x86: make struct thread_info arch specific again

commit c65eacbe290b ("sched/core: Allow putting thread_info into
task_struct") made struct thread_info a generic struct with only a
single flags member if THREAD_INFO_IN_TASK_STRUCT is selected.

This change however seems to be quite x86 centric, since at least the
generic preemption code (asm-generic/preempt.h) assumes that struct
thread_info also has a preempt_count member, which apparently was not
true for x86.

We could add a bit more ifdefs to solve this problem too, but it seems
to be much simpler to make struct thread_info arch specific
again. This also makes the conversion to THREAD_INFO_IN_TASK_STRUCT a
bit easier for architectures that have a couple of arch specific stuff
in their thread_info definition.

The arch specific stuff _could_ be moved to thread_struct. However
keeping them in thread_info makes it easier: accessing thread_info
members is simple, since it is at the beginning of the task_struct,
while the thread_struct is at the end. At least on s390 the offsets
needed to access members of the thread_struct (with task_struct as
base) are too large for various asm instructions. This is not a
problem when keeping these members within thread_info.

Signed-off-by: Heiko Carstens <[email protected]>
---
arch/x86/include/asm/thread_info.h | 9 +++++++++
include/linux/thread_info.h | 11 -----------
2 files changed, 9 insertions(+), 11 deletions(-)

diff --git a/arch/x86/include/asm/thread_info.h b/arch/x86/include/asm/thread_info.h
index 2aaca53c0974..ad6f5eb07a95 100644
--- a/arch/x86/include/asm/thread_info.h
+++ b/arch/x86/include/asm/thread_info.h
@@ -52,6 +52,15 @@ struct task_struct;
#include <asm/cpufeature.h>
#include <linux/atomic.h>

+struct thread_info {
+ unsigned long flags; /* low level flags */
+};
+
+#define INIT_THREAD_INFO(tsk) \
+{ \
+ .flags = 0, \
+}
+
#define init_stack (init_thread_union.stack)

#else /* !__ASSEMBLY__ */
diff --git a/include/linux/thread_info.h b/include/linux/thread_info.h
index 45f004e9cc59..2873baf5372a 100644
--- a/include/linux/thread_info.h
+++ b/include/linux/thread_info.h
@@ -14,17 +14,6 @@ struct timespec;
struct compat_timespec;

#ifdef CONFIG_THREAD_INFO_IN_TASK
-struct thread_info {
- unsigned long flags; /* low level flags */
-};
-
-#define INIT_THREAD_INFO(tsk) \
-{ \
- .flags = 0, \
-}
-#endif
-
-#ifdef CONFIG_THREAD_INFO_IN_TASK
#define current_thread_info() ((struct thread_info *)current)
#endif

--
2.8.4

2016-10-13 13:06:30

by Heiko Carstens

[permalink] [raw]
Subject: [PATCH 2/3] sched/preempt: include asm/current.h

The generic preempt code needs to include <asm/current.h>. Otherwise
compilation fails if THREAD_INFO_IN_TASK is selected and the generic
preempt code is used:

./include/linux/thread_info.h:17:54: error: 'current' undeclared (first use in this function)
#define current_thread_info() ((struct thread_info *)current)

Signed-off-by: Heiko Carstens <[email protected]>
---
include/asm-generic/preempt.h | 1 +
1 file changed, 1 insertion(+)

diff --git a/include/asm-generic/preempt.h b/include/asm-generic/preempt.h
index c1cde3577551..66fcd6cd7fc6 100644
--- a/include/asm-generic/preempt.h
+++ b/include/asm-generic/preempt.h
@@ -2,6 +2,7 @@
#define __ASM_PREEMPT_H

#include <linux/thread_info.h>
+#include <asm/current.h>

#define PREEMPT_ENABLED (0)

--
2.8.4

2016-10-13 20:01:41

by Heiko Carstens

[permalink] [raw]
Subject: [PATCH 3/3] s390: move thread_info into task_struct

This is the s390 variant of commit 15f4eae70d36 ("x86: Move
thread_info into task_struct").

Signed-off-by: Heiko Carstens <[email protected]>
---
arch/s390/Kconfig | 1 +
arch/s390/include/asm/lowcore.h | 2 +-
arch/s390/include/asm/thread_info.h | 11 --------
arch/s390/kernel/asm-offsets.c | 17 +++++--------
arch/s390/kernel/entry.S | 51 ++++++++++++++++++-------------------
arch/s390/kernel/head64.S | 5 ++--
arch/s390/kernel/setup.c | 3 +--
arch/s390/kernel/smp.c | 1 -
8 files changed, 37 insertions(+), 54 deletions(-)

diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
index 426481d4cc86..a159dfc27b76 100644
--- a/arch/s390/Kconfig
+++ b/arch/s390/Kconfig
@@ -169,6 +169,7 @@ config S390
select OLD_SIGSUSPEND3
select SPARSE_IRQ
select SYSCTL_EXCEPTION_TRACE
+ select THREAD_INFO_IN_TASK
select TTY
select VIRT_CPU_ACCOUNTING
select VIRT_TO_BUS
diff --git a/arch/s390/include/asm/lowcore.h b/arch/s390/include/asm/lowcore.h
index 7b93b78f423c..27402de81047 100644
--- a/arch/s390/include/asm/lowcore.h
+++ b/arch/s390/include/asm/lowcore.h
@@ -95,7 +95,7 @@ struct lowcore {

/* Current process. */
__u64 current_task; /* 0x0310 */
- __u64 thread_info; /* 0x0318 */
+ __u8 pad_0x318[0x320-0x318]; /* 0x0318 */
__u64 kernel_stack; /* 0x0320 */

/* Interrupt, panic and restart stack. */
diff --git a/arch/s390/include/asm/thread_info.h b/arch/s390/include/asm/thread_info.h
index f15c0398c363..f5fc4efa9bec 100644
--- a/arch/s390/include/asm/thread_info.h
+++ b/arch/s390/include/asm/thread_info.h
@@ -30,10 +30,8 @@
* - if the contents of this structure are changed, the assembly constants must also be changed
*/
struct thread_info {
- struct task_struct *task; /* main task structure */
unsigned long flags; /* low level flags */
unsigned long sys_call_table; /* System call table address */
- unsigned int cpu; /* current CPU */
int preempt_count; /* 0 => preemptable, <0 => BUG */
unsigned int system_call;
__u64 user_timer;
@@ -46,21 +44,12 @@ struct thread_info {
*/
#define INIT_THREAD_INFO(tsk) \
{ \
- .task = &tsk, \
.flags = 0, \
- .cpu = 0, \
.preempt_count = INIT_PREEMPT_COUNT, \
}

-#define init_thread_info (init_thread_union.thread_info)
#define init_stack (init_thread_union.stack)

-/* how to get the thread information struct from C */
-static inline struct thread_info *current_thread_info(void)
-{
- return (struct thread_info *) S390_lowcore.thread_info;
-}
-
void arch_release_task_struct(struct task_struct *tsk);
int arch_dup_task_struct(struct task_struct *dst, struct task_struct *src);

diff --git a/arch/s390/kernel/asm-offsets.c b/arch/s390/kernel/asm-offsets.c
index f3df9e0a5dec..e6969ebd0d44 100644
--- a/arch/s390/kernel/asm-offsets.c
+++ b/arch/s390/kernel/asm-offsets.c
@@ -25,7 +25,7 @@
int main(void)
{
/* task struct offsets */
- OFFSET(__TASK_thread_info, task_struct, stack);
+ OFFSET(__TASK_stack, task_struct, stack);
OFFSET(__TASK_thread, task_struct, thread);
OFFSET(__TASK_pid, task_struct, pid);
BLANK();
@@ -39,14 +39,12 @@ int main(void)
OFFSET(__THREAD_trap_tdb, thread_struct, trap_tdb);
BLANK();
/* thread info offsets */
- OFFSET(__TI_task, thread_info, task);
- OFFSET(__TI_flags, thread_info, flags);
- OFFSET(__TI_sysc_table, thread_info, sys_call_table);
- OFFSET(__TI_cpu, thread_info, cpu);
- OFFSET(__TI_precount, thread_info, preempt_count);
- OFFSET(__TI_user_timer, thread_info, user_timer);
- OFFSET(__TI_system_timer, thread_info, system_timer);
- OFFSET(__TI_last_break, thread_info, last_break);
+ OFFSET(__TASK_TI_flags, task_struct, thread_info.flags);
+ OFFSET(__TASK_TI_sysc_table, task_struct, thread_info.sys_call_table);
+ OFFSET(__TASK_TI_precount, task_struct, thread_info.preempt_count);
+ OFFSET(__TASK_TI_user_timer, task_struct, thread_info.user_timer);
+ OFFSET(__TASK_TI_system_timer, task_struct, thread_info.system_timer);
+ OFFSET(__TASK_TI_last_break, task_struct, thread_info.last_break);
BLANK();
/* pt_regs offsets */
OFFSET(__PT_ARGS, pt_regs, args);
@@ -159,7 +157,6 @@ int main(void)
OFFSET(__LC_INT_CLOCK, lowcore, int_clock);
OFFSET(__LC_MCCK_CLOCK, lowcore, mcck_clock);
OFFSET(__LC_CURRENT, lowcore, current_task);
- OFFSET(__LC_THREAD_INFO, lowcore, thread_info);
OFFSET(__LC_KERNEL_STACK, lowcore, kernel_stack);
OFFSET(__LC_ASYNC_STACK, lowcore, async_stack);
OFFSET(__LC_PANIC_STACK, lowcore, panic_stack);
diff --git a/arch/s390/kernel/entry.S b/arch/s390/kernel/entry.S
index c51650a1ed16..1becf48d914f 100644
--- a/arch/s390/kernel/entry.S
+++ b/arch/s390/kernel/entry.S
@@ -123,7 +123,7 @@ _PIF_WORK = (_PIF_PER_TRAP)
.macro LAST_BREAK scratch
srag \scratch,%r10,23
jz .+10
- stg %r10,__TI_last_break(%r12)
+ stg %r10,__TASK_TI_last_break(%r12)
.endm

.macro REENABLE_IRQS
@@ -185,14 +185,13 @@ ENTRY(__switch_to)
stmg %r6,%r15,__SF_GPRS(%r15) # store gprs of prev task
lgr %r1,%r2
aghi %r1,__TASK_thread # thread_struct of prev task
- lg %r5,__TASK_thread_info(%r3) # get thread_info of next
+ lg %r5,__TASK_stack(%r3) # start of kernel stack of next
stg %r15,__THREAD_ksp(%r1) # store kernel stack of prev
lgr %r1,%r3
aghi %r1,__TASK_thread # thread_struct of next task
lgr %r15,%r5
aghi %r15,STACK_INIT # end of kernel stack of next
stg %r3,__LC_CURRENT # store task struct of next
- stg %r5,__LC_THREAD_INFO # store thread info of next
stg %r15,__LC_KERNEL_STACK # store end of kernel stack
lg %r15,__THREAD_ksp(%r1) # load kernel stack of next
/* c4 is used in guest detection: arch/s390/kernel/perf_cpum_sf.c */
@@ -271,7 +270,7 @@ ENTRY(system_call)
.Lsysc_stmg:
stmg %r8,%r15,__LC_SAVE_AREA_SYNC
lg %r10,__LC_LAST_BREAK
- lg %r12,__LC_THREAD_INFO
+ lg %r12,__LC_CURRENT
lghi %r14,_PIF_SYSCALL
.Lsysc_per:
lg %r15,__LC_KERNEL_STACK
@@ -285,7 +284,7 @@ ENTRY(system_call)
mvc __PT_INT_CODE(4,%r11),__LC_SVC_ILC
stg %r14,__PT_FLAGS(%r11)
.Lsysc_do_svc:
- lg %r10,__TI_sysc_table(%r12) # address of system call table
+ lg %r10,__TASK_TI_sysc_table(%r12) # address of system call table
llgh %r8,__PT_INT_CODE+2(%r11)
slag %r8,%r8,2 # shift and test for svc 0
jnz .Lsysc_nr_ok
@@ -300,7 +299,7 @@ ENTRY(system_call)
stg %r2,__PT_ORIG_GPR2(%r11)
stg %r7,STACK_FRAME_OVERHEAD(%r15)
lgf %r9,0(%r8,%r10) # get system call add.
- TSTMSK __TI_flags(%r12),_TIF_TRACE
+ TSTMSK __TASK_TI_flags(%r12),_TIF_TRACE
jnz .Lsysc_tracesys
basr %r14,%r9 # call sys_xxxx
stg %r2,__PT_R2(%r11) # store return value
@@ -310,7 +309,7 @@ ENTRY(system_call)
.Lsysc_tif:
TSTMSK __PT_FLAGS(%r11),_PIF_WORK
jnz .Lsysc_work
- TSTMSK __TI_flags(%r12),_TIF_WORK
+ TSTMSK __TASK_TI_flags(%r12),_TIF_WORK
jnz .Lsysc_work # check for work
TSTMSK __LC_CPU_FLAGS,_CIF_WORK
jnz .Lsysc_work
@@ -330,17 +329,17 @@ ENTRY(system_call)
.Lsysc_work:
TSTMSK __LC_CPU_FLAGS,_CIF_MCCK_PENDING
jo .Lsysc_mcck_pending
- TSTMSK __TI_flags(%r12),_TIF_NEED_RESCHED
+ TSTMSK __TASK_TI_flags(%r12),_TIF_NEED_RESCHED
jo .Lsysc_reschedule
#ifdef CONFIG_UPROBES
- TSTMSK __TI_flags(%r12),_TIF_UPROBE
+ TSTMSK __TASK_TI_flags(%r12),_TIF_UPROBE
jo .Lsysc_uprobe_notify
#endif
TSTMSK __PT_FLAGS(%r11),_PIF_PER_TRAP
jo .Lsysc_singlestep
- TSTMSK __TI_flags(%r12),_TIF_SIGPENDING
+ TSTMSK __TASK_TI_flags(%r12),_TIF_SIGPENDING
jo .Lsysc_sigpending
- TSTMSK __TI_flags(%r12),_TIF_NOTIFY_RESUME
+ TSTMSK __TASK_TI_flags(%r12),_TIF_NOTIFY_RESUME
jo .Lsysc_notify_resume
TSTMSK __LC_CPU_FLAGS,_CIF_FPU
jo .Lsysc_vxrs
@@ -386,7 +385,7 @@ ENTRY(system_call)
TSTMSK __PT_FLAGS(%r11),_PIF_SYSCALL
jno .Lsysc_return
lmg %r2,%r7,__PT_R2(%r11) # load svc arguments
- lg %r10,__TI_sysc_table(%r12) # address of system call table
+ lg %r10,__TASK_TI_sysc_table(%r12) # address of system call table
lghi %r8,0 # svc 0 returns -ENOSYS
llgh %r1,__PT_INT_CODE+2(%r11) # load new svc number
cghi %r1,NR_syscalls
@@ -443,7 +442,7 @@ ENTRY(system_call)
basr %r14,%r9 # call sys_xxx
stg %r2,__PT_R2(%r11) # store return value
.Lsysc_tracenogo:
- TSTMSK __TI_flags(%r12),_TIF_TRACE
+ TSTMSK __TASK_TI_flags(%r12),_TIF_TRACE
jz .Lsysc_return
lgr %r2,%r11 # pass pointer to pt_regs
larl %r14,.Lsysc_return
@@ -454,7 +453,7 @@ ENTRY(system_call)
#
ENTRY(ret_from_fork)
la %r11,STACK_FRAME_OVERHEAD(%r15)
- lg %r12,__LC_THREAD_INFO
+ lg %r12,__LC_CURRENT
brasl %r14,schedule_tail
TRACE_IRQS_ON
ssm __LC_SVC_NEW_PSW # reenable interrupts
@@ -475,7 +474,7 @@ ENTRY(pgm_check_handler)
stpt __LC_SYNC_ENTER_TIMER
stmg %r8,%r15,__LC_SAVE_AREA_SYNC
lg %r10,__LC_LAST_BREAK
- lg %r12,__LC_THREAD_INFO
+ lg %r12,__LC_CURRENT
larl %r13,cleanup_critical
lmg %r8,%r9,__LC_PGM_OLD_PSW
tmhh %r8,0x0001 # test problem state bit
@@ -498,7 +497,7 @@ ENTRY(pgm_check_handler)
2: LAST_BREAK %r14
UPDATE_VTIME %r14,%r15,__LC_SYNC_ENTER_TIMER
lg %r15,__LC_KERNEL_STACK
- lg %r14,__TI_task(%r12)
+ lgr %r14,%r12
aghi %r14,__TASK_thread # pointer to thread_struct
lghi %r13,__LC_PGM_TDB
tm __LC_PGM_ILC+2,0x02 # check for transaction abort
@@ -564,7 +563,7 @@ ENTRY(io_int_handler)
stpt __LC_ASYNC_ENTER_TIMER
stmg %r8,%r15,__LC_SAVE_AREA_ASYNC
lg %r10,__LC_LAST_BREAK
- lg %r12,__LC_THREAD_INFO
+ lg %r12,__LC_CURRENT
larl %r13,cleanup_critical
lmg %r8,%r9,__LC_IO_OLD_PSW
SWITCH_ASYNC __LC_SAVE_AREA_ASYNC,__LC_ASYNC_ENTER_TIMER
@@ -595,7 +594,7 @@ ENTRY(io_int_handler)
LOCKDEP_SYS_EXIT
TRACE_IRQS_ON
.Lio_tif:
- TSTMSK __TI_flags(%r12),_TIF_WORK
+ TSTMSK __TASK_TI_flags(%r12),_TIF_WORK
jnz .Lio_work # there is work to do (signals etc.)
TSTMSK __LC_CPU_FLAGS,_CIF_WORK
jnz .Lio_work
@@ -623,9 +622,9 @@ ENTRY(io_int_handler)
jo .Lio_work_user # yes -> do resched & signal
#ifdef CONFIG_PREEMPT
# check for preemptive scheduling
- icm %r0,15,__TI_precount(%r12)
+ icm %r0,15,__TASK_TI_precount(%r12)
jnz .Lio_restore # preemption is disabled
- TSTMSK __TI_flags(%r12),_TIF_NEED_RESCHED
+ TSTMSK __TASK_TI_flags(%r12),_TIF_NEED_RESCHED
jno .Lio_restore
# switch to kernel stack
lg %r1,__PT_R15(%r11)
@@ -659,11 +658,11 @@ ENTRY(io_int_handler)
.Lio_work_tif:
TSTMSK __LC_CPU_FLAGS,_CIF_MCCK_PENDING
jo .Lio_mcck_pending
- TSTMSK __TI_flags(%r12),_TIF_NEED_RESCHED
+ TSTMSK __TASK_TI_flags(%r12),_TIF_NEED_RESCHED
jo .Lio_reschedule
- TSTMSK __TI_flags(%r12),_TIF_SIGPENDING
+ TSTMSK __TASK_TI_flags(%r12),_TIF_SIGPENDING
jo .Lio_sigpending
- TSTMSK __TI_flags(%r12),_TIF_NOTIFY_RESUME
+ TSTMSK __TASK_TI_flags(%r12),_TIF_NOTIFY_RESUME
jo .Lio_notify_resume
TSTMSK __LC_CPU_FLAGS,_CIF_FPU
jo .Lio_vxrs
@@ -738,7 +737,7 @@ ENTRY(ext_int_handler)
stpt __LC_ASYNC_ENTER_TIMER
stmg %r8,%r15,__LC_SAVE_AREA_ASYNC
lg %r10,__LC_LAST_BREAK
- lg %r12,__LC_THREAD_INFO
+ lg %r12,__LC_CURRENT
larl %r13,cleanup_critical
lmg %r8,%r9,__LC_EXT_OLD_PSW
SWITCH_ASYNC __LC_SAVE_AREA_ASYNC,__LC_ASYNC_ENTER_TIMER
@@ -883,7 +882,7 @@ ENTRY(mcck_int_handler)
spt __LC_CPU_TIMER_SAVE_AREA-4095(%r1) # revalidate cpu timer
lmg %r0,%r15,__LC_GPREGS_SAVE_AREA-4095(%r1)# revalidate gprs
lg %r10,__LC_LAST_BREAK
- lg %r12,__LC_THREAD_INFO
+ lg %r12,__LC_CURRENT
larl %r13,cleanup_critical
lmg %r8,%r9,__LC_MCK_OLD_PSW
TSTMSK __LC_MCCK_CODE,MCCK_CODE_SYSTEM_DAMAGE
@@ -1100,7 +1099,7 @@ cleanup_critical:
lg %r9,16(%r11)
srag %r9,%r9,23
jz 0f
- mvc __TI_last_break(8,%r12),16(%r11)
+ mvc __TASK_TI_last_break(8,%r12),16(%r11)
0: # set up saved register r11
lg %r15,__LC_KERNEL_STACK
la %r9,STACK_FRAME_OVERHEAD(%r15)
diff --git a/arch/s390/kernel/head64.S b/arch/s390/kernel/head64.S
index 03c2b469c472..a46201d2ed07 100644
--- a/arch/s390/kernel/head64.S
+++ b/arch/s390/kernel/head64.S
@@ -32,10 +32,9 @@ ENTRY(startup_continue)
#
# Setup stack
#
- larl %r15,init_thread_union
- stg %r15,__LC_THREAD_INFO # cache thread info in lowcore
- lg %r14,__TI_task(%r15) # cache current in lowcore
+ larl %r14,init_task
stg %r14,__LC_CURRENT
+ larl %r15,init_thread_union
aghi %r15,1<<(PAGE_SHIFT+THREAD_ORDER) # init_task_union + THREAD_SIZE
stg %r15,__LC_KERNEL_STACK # set end of kernel stack
aghi %r15,-160
diff --git a/arch/s390/kernel/setup.c b/arch/s390/kernel/setup.c
index 7f7ba5f23f13..6bb266003495 100644
--- a/arch/s390/kernel/setup.c
+++ b/arch/s390/kernel/setup.c
@@ -329,8 +329,7 @@ static void __init setup_lowcore(void)
lc->panic_stack = (unsigned long)
__alloc_bootmem(PAGE_SIZE, PAGE_SIZE, 0)
+ PAGE_SIZE - STACK_FRAME_OVERHEAD - sizeof(struct pt_regs);
- lc->current_task = (unsigned long) init_thread_union.thread_info.task;
- lc->thread_info = (unsigned long) &init_thread_union;
+ lc->current_task = (unsigned long)&init_task;
lc->lpp = LPP_MAGIC;
lc->machine_flags = S390_lowcore.machine_flags;
lc->stfl_fac_list = S390_lowcore.stfl_fac_list;
diff --git a/arch/s390/kernel/smp.c b/arch/s390/kernel/smp.c
index 35531fe1c5ea..8c7ab7bd3e10 100644
--- a/arch/s390/kernel/smp.c
+++ b/arch/s390/kernel/smp.c
@@ -263,7 +263,6 @@ static void pcpu_attach_task(struct pcpu *pcpu, struct task_struct *tsk)

lc->kernel_stack = (unsigned long) task_stack_page(tsk)
+ THREAD_SIZE - STACK_FRAME_OVERHEAD - sizeof(struct pt_regs);
- lc->thread_info = (unsigned long) task_thread_info(tsk);
lc->current_task = (unsigned long) tsk;
lc->lpp = LPP_MAGIC;
lc->current_pid = tsk->pid;
--
2.8.4

2016-10-13 22:00:21

by Andy Lutomirski

[permalink] [raw]
Subject: Re: [PATCH 0/3] THREAD_INFO_IN_TASK_STRUCT vs generic preemption code

On Thu, Oct 13, 2016 at 4:57 AM, Heiko Carstens
<[email protected]> wrote:
> Commit c65eacbe290b ("sched/core: Allow putting thread_info into
> task_struct") made struct thread_info a generic struct with only a
> single flags member if THREAD_INFO_IN_TASK_STRUCT is selected.
>
> This change however seems to be quite x86 centric, since at least the
> generic preemption code (asm-generic/preempt.h) assumes that struct
> thread_info also has a preempt_count member, which apparently was not
> true for x86.
>
> We could add a bit more ifdefs to solve this problem too, but it seems
> to be much simpler to make struct thread_info arch specific
> again. This also makes the conversion to THREAD_INFO_IN_TASK_STRUCT a
> bit easier for architectures that have a couple of arch specific stuff
> in their thread_info definition.

OK, I give in.

But can you coordinate with Mark, because I think I convinced him to
do it a little differently? I might be changing my mind a bit for an
evil reason. Specifically, on x86_64, we could do the following evil,
horrible thing:

union {
u64 flags;
struct {
u32 atomic_flags;
u32 nonatomic_flags;
}
};

Then we could read and test the full set of flags (currently split
between "flags" and "status") with a single instruction, and we could
set them maximally efficiently. I don't actually want to do this
right away, but making thread_info fully arch-controlled would allow
this.

--Andy

2016-10-13 23:26:11

by Mark Rutland

[permalink] [raw]
Subject: Re: [PATCH 2/3] sched/preempt: include asm/current.h

Hi,

On Thu, Oct 13, 2016 at 01:57:11PM +0200, Heiko Carstens wrote:
> The generic preempt code needs to include <asm/current.h>. Otherwise
> compilation fails if THREAD_INFO_IN_TASK is selected and the generic
> preempt code is used:
>
> ./include/linux/thread_info.h:17:54: error: 'current' undeclared (first use in this function)
> #define current_thread_info() ((struct thread_info *)current)

I don't think this is the right fix. Users of current_thread_info() should only
have to include <linux/thread_info.h>, as <asm-generic/preempt.h> already does.

I have a patch [1] which has <linux/thread_info.h> include <asm/current.h> the
THREAD_INFO_IN_TASK case (while avoiding circular includes over <asm/current.h>
and <asm/thread_info.h> in the !THREAD_INFO_IN_TASK case).

I was planning on posting an updated series with that come -rc1.

Thanks,
Mark.

[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2016-September/457243.html

>
> Signed-off-by: Heiko Carstens <[email protected]>
> ---
> include/asm-generic/preempt.h | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/include/asm-generic/preempt.h b/include/asm-generic/preempt.h
> index c1cde3577551..66fcd6cd7fc6 100644
> --- a/include/asm-generic/preempt.h
> +++ b/include/asm-generic/preempt.h
> @@ -2,6 +2,7 @@
> #define __ASM_PREEMPT_H
>
> #include <linux/thread_info.h>
> +#include <asm/current.h>
>
> #define PREEMPT_ENABLED (0)
>
> --
> 2.8.4
>

2016-10-13 23:42:02

by Mark Rutland

[permalink] [raw]
Subject: Re: [PATCH 1/3] sched/core,x86: make struct thread_info arch specific again

Hi,

On Thu, Oct 13, 2016 at 01:57:10PM +0200, Heiko Carstens wrote:
> commit c65eacbe290b ("sched/core: Allow putting thread_info into
> task_struct") made struct thread_info a generic struct with only a
> single flags member if THREAD_INFO_IN_TASK_STRUCT is selected.
>
> This change however seems to be quite x86 centric, since at least the
> generic preemption code (asm-generic/preempt.h) assumes that struct
> thread_info also has a preempt_count member, which apparently was not
> true for x86.
>
> We could add a bit more ifdefs to solve this problem too, but it seems
> to be much simpler to make struct thread_info arch specific
> again. This also makes the conversion to THREAD_INFO_IN_TASK_STRUCT a
> bit easier for architectures that have a couple of arch specific stuff
> in their thread_info definition.
>
> The arch specific stuff _could_ be moved to thread_struct. However
> keeping them in thread_info makes it easier: accessing thread_info
> members is simple, since it is at the beginning of the task_struct,
> while the thread_struct is at the end. At least on s390 the offsets
> needed to access members of the thread_struct (with task_struct as
> base) are too large for various asm instructions. This is not a
> problem when keeping these members within thread_info.

The exact same applies for arm64 on all counts. This is also simpler than both
attempts I had at this, so FWIW:

Acked-by: Mark Rutland <[email protected]>

To make merging less painful, I guess we'll need a stable branch with (just)
this and whatever patch we end up with for fixing current_thread_info(), so we
can independently merge the arch-specific parts.

I guess it'd make sense for the tip tree to host that?

Thanks,
Mark.

> Signed-off-by: Heiko Carstens <[email protected]>
> ---
> arch/x86/include/asm/thread_info.h | 9 +++++++++
> include/linux/thread_info.h | 11 -----------
> 2 files changed, 9 insertions(+), 11 deletions(-)
>
> diff --git a/arch/x86/include/asm/thread_info.h b/arch/x86/include/asm/thread_info.h
> index 2aaca53c0974..ad6f5eb07a95 100644
> --- a/arch/x86/include/asm/thread_info.h
> +++ b/arch/x86/include/asm/thread_info.h
> @@ -52,6 +52,15 @@ struct task_struct;
> #include <asm/cpufeature.h>
> #include <linux/atomic.h>
>
> +struct thread_info {
> + unsigned long flags; /* low level flags */
> +};
> +
> +#define INIT_THREAD_INFO(tsk) \
> +{ \
> + .flags = 0, \
> +}
> +
> #define init_stack (init_thread_union.stack)
>
> #else /* !__ASSEMBLY__ */
> diff --git a/include/linux/thread_info.h b/include/linux/thread_info.h
> index 45f004e9cc59..2873baf5372a 100644
> --- a/include/linux/thread_info.h
> +++ b/include/linux/thread_info.h
> @@ -14,17 +14,6 @@ struct timespec;
> struct compat_timespec;
>
> #ifdef CONFIG_THREAD_INFO_IN_TASK
> -struct thread_info {
> - unsigned long flags; /* low level flags */
> -};
> -
> -#define INIT_THREAD_INFO(tsk) \
> -{ \
> - .flags = 0, \
> -}
> -#endif
> -
> -#ifdef CONFIG_THREAD_INFO_IN_TASK
> #define current_thread_info() ((struct thread_info *)current)
> #endif
>
> --
> 2.8.4
>

2016-10-13 23:51:45

by Andy Lutomirski

[permalink] [raw]
Subject: Re: [PATCH 1/3] sched/core,x86: make struct thread_info arch specific again

On Thu, Oct 13, 2016 at 4:41 PM, Mark Rutland <[email protected]> wrote:
> Hi,
>
> On Thu, Oct 13, 2016 at 01:57:10PM +0200, Heiko Carstens wrote:
>> commit c65eacbe290b ("sched/core: Allow putting thread_info into
>> task_struct") made struct thread_info a generic struct with only a
>> single flags member if THREAD_INFO_IN_TASK_STRUCT is selected.
>>
>> This change however seems to be quite x86 centric, since at least the
>> generic preemption code (asm-generic/preempt.h) assumes that struct
>> thread_info also has a preempt_count member, which apparently was not
>> true for x86.
>>
>> We could add a bit more ifdefs to solve this problem too, but it seems
>> to be much simpler to make struct thread_info arch specific
>> again. This also makes the conversion to THREAD_INFO_IN_TASK_STRUCT a
>> bit easier for architectures that have a couple of arch specific stuff
>> in their thread_info definition.
>>
>> The arch specific stuff _could_ be moved to thread_struct. However
>> keeping them in thread_info makes it easier: accessing thread_info
>> members is simple, since it is at the beginning of the task_struct,
>> while the thread_struct is at the end. At least on s390 the offsets
>> needed to access members of the thread_struct (with task_struct as
>> base) are too large for various asm instructions. This is not a
>> problem when keeping these members within thread_info.
>
> The exact same applies for arm64 on all counts. This is also simpler than both
> attempts I had at this, so FWIW:
>
> Acked-by: Mark Rutland <[email protected]>
>
> To make merging less painful, I guess we'll need a stable branch with (just)
> this and whatever patch we end up with for fixing current_thread_info(), so we
> can independently merge the arch-specific parts.
>
> I guess it'd make sense for the tip tree to host that?
>

I wonder if this could even make 4.9. It's pretty clearly a no-op. Ingo?

2016-10-14 08:17:11

by Heiko Carstens

[permalink] [raw]
Subject: Re: [PATCH 2/3] sched/preempt: include asm/current.h

On Fri, Oct 14, 2016 at 12:25:56AM +0100, Mark Rutland wrote:
> Hi,
>
> On Thu, Oct 13, 2016 at 01:57:11PM +0200, Heiko Carstens wrote:
> > The generic preempt code needs to include <asm/current.h>. Otherwise
> > compilation fails if THREAD_INFO_IN_TASK is selected and the generic
> > preempt code is used:
> >
> > ./include/linux/thread_info.h:17:54: error: 'current' undeclared (first use in this function)
> > #define current_thread_info() ((struct thread_info *)current)
>
> I don't think this is the right fix. Users of current_thread_info() should only
> have to include <linux/thread_info.h>, as <asm-generic/preempt.h> already does.
>
> I have a patch [1] which has <linux/thread_info.h> include <asm/current.h> the
> THREAD_INFO_IN_TASK case (while avoiding circular includes over <asm/current.h>
> and <asm/thread_info.h> in the !THREAD_INFO_IN_TASK case).

I added that include initially to <linux/thread_info.h> too, but was afraid
of include dependency hell. So I tried the minimal version, and it worked.

However with the ifdef within your patch you make sure that nothing breaks
by accident; so I think your version is better.

> I was planning on posting an updated series with that come -rc1.

That could/should also go into 4.9, so architectures could independently
convert to THREAD_INFO_IN_TASK for the next merge window.

2016-10-14 10:43:04

by Mark Rutland

[permalink] [raw]
Subject: [PATCH] thread_info: include <current.h> for THREAD_INFO_IN_TASK (WAS: [PATCH 2/3] sched/preempt: include asm/current.h)

When CONFIG_THREAD_INFO_IN_TASK is selected, the current_thread_info()
macro relies on current having been defined prior to its use. However,
not all users of current_thread_info() include <asm/current.h>, and thus
current is not guaranteed to be defined.

When CONFIG_THREAD_INFO_IN_TASK is not selected, it's possible that
get_current() / current are based upon current_thread_info(), and
<asm/current.h> includes <asm/thread_info.h>. Thus always including
<asm/current.h> would result in circular dependences on some platforms.

To ensure both cases work, this patch includes <asm/current.h>, but only
when CONFIG_THREAD_INFO_IN_TASK is selected.

Signed-off-by: Mark Rutland <[email protected]>
---
include/linux/thread_info.h | 1 +
1 file changed, 1 insertion(+)

As discussed, it would be great if this could go in along with the patch to
make thread_info arch-specific again, to make merging the arch-specific parts
easier (for arm64 and s390).

Mark.

diff --git a/include/linux/thread_info.h b/include/linux/thread_info.h
index 45f004e..521f84b 100644
--- a/include/linux/thread_info.h
+++ b/include/linux/thread_info.h
@@ -25,6 +25,7 @@ struct thread_info {
#endif

#ifdef CONFIG_THREAD_INFO_IN_TASK
+#include <asm/current.h>
#define current_thread_info() ((struct thread_info *)current)
#endif

--
1.9.1

2016-10-17 14:48:53

by Mark Rutland

[permalink] [raw]
Subject: Re: [PATCH] thread_info: include <current.h> for THREAD_INFO_IN_TASK (WAS: [PATCH 2/3] sched/preempt: include asm/current.h)

On Fri, Oct 14, 2016 at 11:42:25AM +0100, Mark Rutland wrote:
> When CONFIG_THREAD_INFO_IN_TASK is selected, the current_thread_info()
> macro relies on current having been defined prior to its use. However,
> not all users of current_thread_info() include <asm/current.h>, and thus
> current is not guaranteed to be defined.
>
> When CONFIG_THREAD_INFO_IN_TASK is not selected, it's possible that
> get_current() / current are based upon current_thread_info(), and
> <asm/current.h> includes <asm/thread_info.h>. Thus always including
> <asm/current.h> would result in circular dependences on some platforms.
>
> To ensure both cases work, this patch includes <asm/current.h>, but only
> when CONFIG_THREAD_INFO_IN_TASK is selected.
>
> Signed-off-by: Mark Rutland <[email protected]>
> ---
> include/linux/thread_info.h | 1 +
> 1 file changed, 1 insertion(+)
>
> As discussed, it would be great if this could go in along with the patch to
> make thread_info arch-specific again, to make merging the arch-specific parts
> easier (for arm64 and s390).

Urrgh; I've just recalled that this patch alone is insufficient. The
h8300 arch code has an unfixed bug [1], and relies on some implicit
definition ordering that will be broken by this patch.

I have a workaround/cleanup for core code that I'll send with an updated
version of my arm64 series shortly.

Thanks,
Mark.

[1] http://lkml.kernel.org/r/<[email protected]

> Mark.
>
> diff --git a/include/linux/thread_info.h b/include/linux/thread_info.h
> index 45f004e..521f84b 100644
> --- a/include/linux/thread_info.h
> +++ b/include/linux/thread_info.h
> @@ -25,6 +25,7 @@ struct thread_info {
> #endif
>
> #ifdef CONFIG_THREAD_INFO_IN_TASK
> +#include <asm/current.h>
> #define current_thread_info() ((struct thread_info *)current)
> #endif
>
> --
> 1.9.1
>

2016-10-17 17:33:53

by Mark Rutland

[permalink] [raw]
Subject: Re: [PATCH] thread_info: include <current.h> for THREAD_INFO_IN_TASK (WAS: [PATCH 2/3] sched/preempt: include asm/current.h)

On Mon, Oct 17, 2016 at 03:48:13PM +0100, Mark Rutland wrote:
> On Fri, Oct 14, 2016 at 11:42:25AM +0100, Mark Rutland wrote:
> > When CONFIG_THREAD_INFO_IN_TASK is selected, the current_thread_info()
> > macro relies on current having been defined prior to its use. However,
> > not all users of current_thread_info() include <asm/current.h>, and thus
> > current is not guaranteed to be defined.
> >
> > When CONFIG_THREAD_INFO_IN_TASK is not selected, it's possible that
> > get_current() / current are based upon current_thread_info(), and
> > <asm/current.h> includes <asm/thread_info.h>. Thus always including
> > <asm/current.h> would result in circular dependences on some platforms.
> >
> > To ensure both cases work, this patch includes <asm/current.h>, but only
> > when CONFIG_THREAD_INFO_IN_TASK is selected.
> >
> > Signed-off-by: Mark Rutland <[email protected]>
> > ---
> > include/linux/thread_info.h | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > As discussed, it would be great if this could go in along with the patch to
> > make thread_info arch-specific again, to make merging the arch-specific parts
> > easier (for arm64 and s390).
>
> Urrgh; I've just recalled that this patch alone is insufficient. The
> h8300 arch code has an unfixed bug [1], and relies on some implicit
> definition ordering that will be broken by this patch.
>
> I have a workaround/cleanup for core code that I'll send with an updated
> version of my arm64 series shortly.

Looks like I spoke too soon. I have another circular include issue with
raw_smp_processor_id() and task_struct::cpu to solve -- it looks like
s390 doesn't suffer from that per my reading of your headers.

In the mean time, I've pushed out a branch [2] with the common patches,
atop of v4.9-rc1.

Thanks,
Mark.

[1] http://lkml.kernel.org/r/<[email protected]
[2] https://git.kernel.org/cgit/linux/kernel/git/mark/linux.git/log/?h=core/ti-stack-split

2016-10-18 10:34:58

by Heiko Carstens

[permalink] [raw]
Subject: Re: [PATCH] thread_info: include <current.h> for THREAD_INFO_IN_TASK (WAS: [PATCH 2/3] sched/preempt: include asm/current.h)

On Mon, Oct 17, 2016 at 06:33:15PM +0100, Mark Rutland wrote:
> On Mon, Oct 17, 2016 at 03:48:13PM +0100, Mark Rutland wrote:
> > On Fri, Oct 14, 2016 at 11:42:25AM +0100, Mark Rutland wrote:
> > > When CONFIG_THREAD_INFO_IN_TASK is selected, the current_thread_info()
> > > macro relies on current having been defined prior to its use. However,
> > > not all users of current_thread_info() include <asm/current.h>, and thus
> > > current is not guaranteed to be defined.
> > >
> > > When CONFIG_THREAD_INFO_IN_TASK is not selected, it's possible that
> > > get_current() / current are based upon current_thread_info(), and
> > > <asm/current.h> includes <asm/thread_info.h>. Thus always including
> > > <asm/current.h> would result in circular dependences on some platforms.
> > >
> > > To ensure both cases work, this patch includes <asm/current.h>, but only
> > > when CONFIG_THREAD_INFO_IN_TASK is selected.
> > >
> > > Signed-off-by: Mark Rutland <[email protected]>
> > > ---
> > > include/linux/thread_info.h | 1 +
> > > 1 file changed, 1 insertion(+)
> > >
> > > As discussed, it would be great if this could go in along with the patch to
> > > make thread_info arch-specific again, to make merging the arch-specific parts
> > > easier (for arm64 and s390).
> >
> > Urrgh; I've just recalled that this patch alone is insufficient. The
> > h8300 arch code has an unfixed bug [1], and relies on some implicit
> > definition ordering that will be broken by this patch.
> >
> > I have a workaround/cleanup for core code that I'll send with an updated
> > version of my arm64 series shortly.
>
> Looks like I spoke too soon. I have another circular include issue with
> raw_smp_processor_id() and task_struct::cpu to solve -- it looks like
> s390 doesn't suffer from that per my reading of your headers.

That's correct.

> In the mean time, I've pushed out a branch [2] with the common patches,
> atop of v4.9-rc1.

I just verified that your branch works fine for s390 (with and without the
THREAD_INFO_IN_TASK conversion).

Thanks,
Heiko