[PATCH 0/3] THREAD_INFO_IN_TASK_STRUCT vs generic preemption code

[PATCH 0/3] THREAD_INFO_IN_TASK_STRUCT vs generic preemption code

[Heiko Carstens]

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

[PATCH 0/3] THREAD_INFO_IN_TASK_STRUCT vs generic preemption code

[Heiko Carstens]

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

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

[Heiko Carstens]

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 <heiko.carstens@de.ibm.com>
---
 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

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

[Heiko Carstens]

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 <heiko.carstens@de.ibm.com>
---
 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

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

[Mark Rutland]

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 <mark.rutland@arm.com>

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 <heiko.carstens@de.ibm.com>
> ---
>  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
> 

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

[Andy Lutomirski]

On Thu, Oct 13, 2016 at 4:41 PM, Mark Rutland <mark.rutland@arm.com> 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 <mark.rutland@arm.com>
>
> 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?

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

[Heiko Carstens]

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 <heiko.carstens@de.ibm.com>
---
 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

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

[Mark Rutland]

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 <heiko.carstens@de.ibm.com>
> ---
>  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
> 

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

[Heiko Carstens]

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.

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

[Mark Rutland]

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 <mark.rutland@arm.com>
---
 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

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

[Mark Rutland]

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 <mark.rutland@arm.com>
> ---
>  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/<1476714934-11635-1-git-send-email-mark.rutland@arm.com

> 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
> 

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

[Mark Rutland]

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 <mark.rutland@arm.com>
> > ---
> >  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/<1476714934-11635-1-git-send-email-mark.rutland@arm.com
[2] https://git.kernel.org/cgit/linux/kernel/git/mark/linux.git/log/?h=core/ti-stack-split

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

[Heiko Carstens]

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 <mark.rutland@arm.com>
> > > ---
> > >  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

[PATCH 3/3] s390: move thread_info into task_struct

[Heiko Carstens]

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

Signed-off-by: Heiko Carstens <heiko.carstens@de.ibm.com>
---
 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

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

[Andy Lutomirski]

On Thu, Oct 13, 2016 at 4:57 AM, Heiko Carstens
<heiko.carstens@de.ibm.com> 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

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

[Andy Lutomirski]

On Thu, Oct 13, 2016 at 4:57 AM, Heiko Carstens
<heiko.carstens@de.ibm.com> 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