* [PATCH 1/3] sched/core,x86: make struct thread_info arch specific again
2016-10-19 18:28 [PATCH 0/3] THREAD_INFO_IN_TASK prep work for arm64+s390 Mark Rutland
@ 2016-10-19 18:28 ` Mark Rutland
2016-10-19 23:19 ` Andy Lutomirski
2016-10-21 5:50 ` [tip:x86/urgent] sched/core, x86: Make " tip-bot for Heiko Carstens
2016-10-19 18:28 ` [PATCH 2/3] thread_info: factor out restart_block Mark Rutland
` (2 subsequent siblings)
3 siblings, 2 replies; 15+ messages in thread
From: Mark Rutland @ 2016-10-19 18:28 UTC (permalink / raw)
To: linux-kernel
Cc: linux-arch, akpm, heiko.carstens, hpa, keescook, luto,
mark.rutland, mingo, tglx
From: Heiko Carstens <heiko.carstens@de.ibm.com>
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>
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
---
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 2aaca53..ad6f5eb0 100644
--- a/arch/x86/include/asm/thread_info.h
+++ b/arch/x86/include/asm/thread_info.h
@@ -52,6 +52,15 @@
#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 45f004e..2873baf 100644
--- a/include/linux/thread_info.h
+++ b/include/linux/thread_info.h
@@ -14,17 +14,6 @@
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
--
1.9.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 1/3] sched/core,x86: make struct thread_info arch specific again
2016-10-19 18:28 ` [PATCH 1/3] sched/core,x86: make struct thread_info arch specific again Mark Rutland
@ 2016-10-19 23:19 ` Andy Lutomirski
2016-10-20 6:40 ` Ingo Molnar
2016-10-21 5:50 ` [tip:x86/urgent] sched/core, x86: Make " tip-bot for Heiko Carstens
1 sibling, 1 reply; 15+ messages in thread
From: Andy Lutomirski @ 2016-10-19 23:19 UTC (permalink / raw)
To: Mark Rutland
Cc: linux-kernel, linux-arch, Andrew Morton, Heiko Carstens,
H. Peter Anvin, Kees Cook, Andrew Lutomirski, Ingo Molnar,
Thomas Gleixner
On Wed, Oct 19, 2016 at 11:28 AM, Mark Rutland <mark.rutland@arm.com> wrote:
> From: Heiko Carstens <heiko.carstens@de.ibm.com>
>
> 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.
Acked-by: Andy Lutomirski <luto@kernel.org>
Ingo, there's a (somewhat weak) argument for sending this via
tip/urgent: it doesn't change generated code at all, and I think it
will avoid a silly depedency or possible conflict for the next merge
window, since both arm64 and s390 are going to need it.
--Andy
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/3] sched/core,x86: make struct thread_info arch specific again
2016-10-19 23:19 ` Andy Lutomirski
@ 2016-10-20 6:40 ` Ingo Molnar
2016-10-20 9:33 ` Mark Rutland
0 siblings, 1 reply; 15+ messages in thread
From: Ingo Molnar @ 2016-10-20 6:40 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Mark Rutland, linux-kernel, linux-arch, Andrew Morton,
Heiko Carstens, H. Peter Anvin, Kees Cook, Andrew Lutomirski,
Ingo Molnar, Thomas Gleixner
* Andy Lutomirski <luto@amacapital.net> wrote:
> On Wed, Oct 19, 2016 at 11:28 AM, Mark Rutland <mark.rutland@arm.com> wrote:
> > From: Heiko Carstens <heiko.carstens@de.ibm.com>
> >
> > 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.
>
> Acked-by: Andy Lutomirski <luto@kernel.org>
>
> Ingo, there's a (somewhat weak) argument for sending this via
> tip/urgent: it doesn't change generated code at all, and I think it
> will avoid a silly depedency or possible conflict for the next merge
> window, since both arm64 and s390 are going to need it.
Can certainly do it if this is the final version of the patch. Mark?
Thanks,
Ingo
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/3] sched/core,x86: make struct thread_info arch specific again
2016-10-20 6:40 ` Ingo Molnar
@ 2016-10-20 9:33 ` Mark Rutland
0 siblings, 0 replies; 15+ messages in thread
From: Mark Rutland @ 2016-10-20 9:33 UTC (permalink / raw)
To: Ingo Molnar
Cc: Andy Lutomirski, linux-kernel, linux-arch, Andrew Morton,
Heiko Carstens, H. Peter Anvin, Kees Cook, Andrew Lutomirski,
Ingo Molnar, Thomas Gleixner
On Thu, Oct 20, 2016 at 08:40:45AM +0200, Ingo Molnar wrote:
>
> * Andy Lutomirski <luto@amacapital.net> wrote:
>
> > On Wed, Oct 19, 2016 at 11:28 AM, Mark Rutland <mark.rutland@arm.com> wrote:
> > > From: Heiko Carstens <heiko.carstens@de.ibm.com>
> > >
> > > 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.
> >
> > Acked-by: Andy Lutomirski <luto@kernel.org>
> >
> > Ingo, there's a (somewhat weak) argument for sending this via
> > tip/urgent: it doesn't change generated code at all, and I think it
> > will avoid a silly depedency or possible conflict for the next merge
> > window, since both arm64 and s390 are going to need it.
>
> Can certainly do it if this is the final version of the patch. Mark?
Yes; this is the final version of this patch.
I can rebase the other two core patches atop, assuming this goes in for
a v4.9-rc* tag soon.
Thanks,
Mark.
^ permalink raw reply [flat|nested] 15+ messages in thread
* [tip:x86/urgent] sched/core, x86: Make struct thread_info arch specific again
2016-10-19 18:28 ` [PATCH 1/3] sched/core,x86: make struct thread_info arch specific again Mark Rutland
2016-10-19 23:19 ` Andy Lutomirski
@ 2016-10-21 5:50 ` tip-bot for Heiko Carstens
1 sibling, 0 replies; 15+ messages in thread
From: tip-bot for Heiko Carstens @ 2016-10-21 5:50 UTC (permalink / raw)
To: linux-tip-commits
Cc: mark.rutland, hpa, torvalds, mingo, tglx, heiko.carstens,
linux-kernel, akpm, luto, peterz
Commit-ID: c8061485a0d7569a865a3cc3c63347b0f42b3765
Gitweb: http://git.kernel.org/tip/c8061485a0d7569a865a3cc3c63347b0f42b3765
Author: Heiko Carstens <heiko.carstens@de.ibm.com>
AuthorDate: Wed, 19 Oct 2016 19:28:11 +0100
Committer: Ingo Molnar <mingo@kernel.org>
CommitDate: Thu, 20 Oct 2016 13:27:47 +0200
sched/core, x86: Make struct thread_info arch specific again
The following 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 CONFIG_THREAD_INFO_IN_TASK_STRUCT=y 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>
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Acked-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: keescook@chromium.org
Cc: linux-arch@vger.kernel.org
Link: http://lkml.kernel.org/r/1476901693-8492-2-git-send-email-mark.rutland@arm.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
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 2aaca53..ad6f5eb0 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 45f004e..2873baf 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
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 2/3] thread_info: factor out restart_block
2016-10-19 18:28 [PATCH 0/3] THREAD_INFO_IN_TASK prep work for arm64+s390 Mark Rutland
2016-10-19 18:28 ` [PATCH 1/3] sched/core,x86: make struct thread_info arch specific again Mark Rutland
@ 2016-10-19 18:28 ` Mark Rutland
2016-10-19 23:31 ` Andy Lutomirski
2016-10-19 18:28 ` [PATCH 3/3] thread_info: include <current.h> for THREAD_INFO_IN_TASK Mark Rutland
2016-10-24 10:12 ` [PATCH 0/3] THREAD_INFO_IN_TASK prep work for arm64+s390 Mark Rutland
3 siblings, 1 reply; 15+ messages in thread
From: Mark Rutland @ 2016-10-19 18:28 UTC (permalink / raw)
To: linux-kernel
Cc: linux-arch, akpm, heiko.carstens, hpa, keescook, luto,
mark.rutland, mingo, tglx
Since commit f56141e3e2d9aabf ("all arches, signal: move restart_block
to struct task_struct"), thread_info and restart_block have been
logically distinct, yet struct restart_block is still defined in
<linux/thread_info.h>.
At least one architecture (erroneously) uses restart_block as part of
its thread_info, and thus the definition of restart_block must come
before the include of <asm/thread_info>. Subsequent patches in this
series need to shuffle the order of includes and definitions in
<linux/thread_info.h>, and will make this ordering fragile.
This patch moves the definition of restart_block out to its own header.
This serves as generic cleanup, logically separating thread_info and
restart_block, and also makes it easier to avoid fragility.
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Heiko Carstens <heiko.carstens@de.ibm.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: linux-kernel@vger.kernel.org
---
include/linux/restart_block.h | 51 +++++++++++++++++++++++++++++++++++++++++++
include/linux/thread_info.h | 41 +---------------------------------
2 files changed, 52 insertions(+), 40 deletions(-)
create mode 100644 include/linux/restart_block.h
diff --git a/include/linux/restart_block.h b/include/linux/restart_block.h
new file mode 100644
index 0000000..0d905d8
--- /dev/null
+++ b/include/linux/restart_block.h
@@ -0,0 +1,51 @@
+/*
+ * Common syscall restarting data
+ */
+#ifndef __LINUX_RESTART_BLOCK_H
+#define __LINUX_RESTART_BLOCK_H
+
+#include <linux/compiler.h>
+#include <linux/types.h>
+
+struct timespec;
+struct compat_timespec;
+struct pollfd;
+
+/*
+ * System call restart block.
+ */
+struct restart_block {
+ long (*fn)(struct restart_block *);
+ union {
+ /* For futex_wait and futex_wait_requeue_pi */
+ struct {
+ u32 __user *uaddr;
+ u32 val;
+ u32 flags;
+ u32 bitset;
+ u64 time;
+ u32 __user *uaddr2;
+ } futex;
+ /* For nanosleep */
+ struct {
+ clockid_t clockid;
+ struct timespec __user *rmtp;
+#ifdef CONFIG_COMPAT
+ struct compat_timespec __user *compat_rmtp;
+#endif
+ u64 expires;
+ } nanosleep;
+ /* For poll */
+ struct {
+ struct pollfd __user *ufds;
+ int nfds;
+ int has_timeout;
+ unsigned long tv_sec;
+ unsigned long tv_nsec;
+ } poll;
+ };
+};
+
+extern long do_no_restart_syscall(struct restart_block *parm);
+
+#endif /* __LINUX_RESTART_BLOCK_H */
diff --git a/include/linux/thread_info.h b/include/linux/thread_info.h
index 2873baf..c75c6ab 100644
--- a/include/linux/thread_info.h
+++ b/include/linux/thread_info.h
@@ -9,51 +9,12 @@
#include <linux/types.h>
#include <linux/bug.h>
-
-struct timespec;
-struct compat_timespec;
+#include <linux/restart_block.h>
#ifdef CONFIG_THREAD_INFO_IN_TASK
#define current_thread_info() ((struct thread_info *)current)
#endif
-/*
- * System call restart block.
- */
-struct restart_block {
- long (*fn)(struct restart_block *);
- union {
- /* For futex_wait and futex_wait_requeue_pi */
- struct {
- u32 __user *uaddr;
- u32 val;
- u32 flags;
- u32 bitset;
- u64 time;
- u32 __user *uaddr2;
- } futex;
- /* For nanosleep */
- struct {
- clockid_t clockid;
- struct timespec __user *rmtp;
-#ifdef CONFIG_COMPAT
- struct compat_timespec __user *compat_rmtp;
-#endif
- u64 expires;
- } nanosleep;
- /* For poll */
- struct {
- struct pollfd __user *ufds;
- int nfds;
- int has_timeout;
- unsigned long tv_sec;
- unsigned long tv_nsec;
- } poll;
- };
-};
-
-extern long do_no_restart_syscall(struct restart_block *parm);
-
#include <linux/bitops.h>
#include <asm/thread_info.h>
--
1.9.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 2/3] thread_info: factor out restart_block
2016-10-19 18:28 ` [PATCH 2/3] thread_info: factor out restart_block Mark Rutland
@ 2016-10-19 23:31 ` Andy Lutomirski
2016-10-24 9:45 ` Mark Rutland
0 siblings, 1 reply; 15+ messages in thread
From: Andy Lutomirski @ 2016-10-19 23:31 UTC (permalink / raw)
To: Mark Rutland
Cc: linux-kernel, linux-arch, Andrew Morton, Heiko Carstens,
H. Peter Anvin, Kees Cook, Andrew Lutomirski, Ingo Molnar,
Thomas Gleixner
On Wed, Oct 19, 2016 at 11:28 AM, Mark Rutland <mark.rutland@arm.com> wrote:
> Since commit f56141e3e2d9aabf ("all arches, signal: move restart_block
> to struct task_struct"), thread_info and restart_block have been
> logically distinct, yet struct restart_block is still defined in
> <linux/thread_info.h>.
>
> At least one architecture (erroneously) uses restart_block as part of
> its thread_info, and thus the definition of restart_block must come
> before the include of <asm/thread_info>. Subsequent patches in this
> series need to shuffle the order of includes and definitions in
> <linux/thread_info.h>, and will make this ordering fragile.
>
> This patch moves the definition of restart_block out to its own header.
> This serves as generic cleanup, logically separating thread_info and
> restart_block, and also makes it easier to avoid fragility.
Looks entirely reasonable to me.
Reviewed-by: Andy Lutomirski <luto@kernel.org>
--Andy
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/3] thread_info: factor out restart_block
2016-10-19 23:31 ` Andy Lutomirski
@ 2016-10-24 9:45 ` Mark Rutland
0 siblings, 0 replies; 15+ messages in thread
From: Mark Rutland @ 2016-10-24 9:45 UTC (permalink / raw)
To: Andy Lutomirski
Cc: linux-kernel, linux-arch, Andrew Morton, Heiko Carstens,
H. Peter Anvin, Kees Cook, Andrew Lutomirski, Ingo Molnar,
Thomas Gleixner
On Wed, Oct 19, 2016 at 04:31:02PM -0700, Andy Lutomirski wrote:
> On Wed, Oct 19, 2016 at 11:28 AM, Mark Rutland <mark.rutland@arm.com> wrote:
> > Since commit f56141e3e2d9aabf ("all arches, signal: move restart_block
> > to struct task_struct"), thread_info and restart_block have been
> > logically distinct, yet struct restart_block is still defined in
> > <linux/thread_info.h>.
> >
> > At least one architecture (erroneously) uses restart_block as part of
> > its thread_info, and thus the definition of restart_block must come
> > before the include of <asm/thread_info>. Subsequent patches in this
> > series need to shuffle the order of includes and definitions in
> > <linux/thread_info.h>, and will make this ordering fragile.
> >
> > This patch moves the definition of restart_block out to its own header.
> > This serves as generic cleanup, logically separating thread_info and
> > restart_block, and also makes it easier to avoid fragility.
>
> Looks entirely reasonable to me.
>
> Reviewed-by: Andy Lutomirski <luto@kernel.org>
Thanks, that's much appreciated.
Now that Heiko's patch is in -rc2 I'd like to be able to put these two
patches into a stable branch.
Before I do that, would you also be happy to ack/review patch 3?
Thanks,
Mark.
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 3/3] thread_info: include <current.h> for THREAD_INFO_IN_TASK
2016-10-19 18:28 [PATCH 0/3] THREAD_INFO_IN_TASK prep work for arm64+s390 Mark Rutland
2016-10-19 18:28 ` [PATCH 1/3] sched/core,x86: make struct thread_info arch specific again Mark Rutland
2016-10-19 18:28 ` [PATCH 2/3] thread_info: factor out restart_block Mark Rutland
@ 2016-10-19 18:28 ` Mark Rutland
2016-10-20 10:29 ` Heiko Carstens
2016-10-27 23:13 ` Andy Lutomirski
2016-10-24 10:12 ` [PATCH 0/3] THREAD_INFO_IN_TASK prep work for arm64+s390 Mark Rutland
3 siblings, 2 replies; 15+ messages in thread
From: Mark Rutland @ 2016-10-19 18:28 UTC (permalink / raw)
To: linux-kernel
Cc: linux-arch, akpm, heiko.carstens, hpa, keescook, luto,
mark.rutland, mingo, tglx
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>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Heiko Carstens <heiko.carstens@de.ibm.com>
Cc: Kees Cook <keescook@chromium.org>
---
include/linux/thread_info.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/include/linux/thread_info.h b/include/linux/thread_info.h
index c75c6ab..ef1f4b0 100644
--- a/include/linux/thread_info.h
+++ b/include/linux/thread_info.h
@@ -12,6 +12,7 @@
#include <linux/restart_block.h>
#ifdef CONFIG_THREAD_INFO_IN_TASK
+#include <asm/current.h>
#define current_thread_info() ((struct thread_info *)current)
#endif
--
1.9.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 3/3] thread_info: include <current.h> for THREAD_INFO_IN_TASK
2016-10-19 18:28 ` [PATCH 3/3] thread_info: include <current.h> for THREAD_INFO_IN_TASK Mark Rutland
@ 2016-10-20 10:29 ` Heiko Carstens
2016-10-24 9:49 ` Mark Rutland
2016-10-27 23:13 ` Andy Lutomirski
1 sibling, 1 reply; 15+ messages in thread
From: Heiko Carstens @ 2016-10-20 10:29 UTC (permalink / raw)
To: Mark Rutland
Cc: linux-kernel, linux-arch, akpm, hpa, keescook, luto, mingo, tglx
On Wed, Oct 19, 2016 at 07:28:13PM +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>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Andy Lutomirski <luto@kernel.org>
> Cc: Heiko Carstens <heiko.carstens@de.ibm.com>
> Cc: Kees Cook <keescook@chromium.org>
> ---
> include/linux/thread_info.h | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/include/linux/thread_info.h b/include/linux/thread_info.h
> index c75c6ab..ef1f4b0 100644
> --- a/include/linux/thread_info.h
> +++ b/include/linux/thread_info.h
> @@ -12,6 +12,7 @@
> #include <linux/restart_block.h>
>
> #ifdef CONFIG_THREAD_INFO_IN_TASK
> +#include <asm/current.h>
> #define current_thread_info() ((struct thread_info *)current)
> #endif
FWIW:
Acked-by: Heiko Carstens <heiko.carstens@de.ibm.com>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/3] thread_info: include <current.h> for THREAD_INFO_IN_TASK
2016-10-20 10:29 ` Heiko Carstens
@ 2016-10-24 9:49 ` Mark Rutland
0 siblings, 0 replies; 15+ messages in thread
From: Mark Rutland @ 2016-10-24 9:49 UTC (permalink / raw)
To: Heiko Carstens
Cc: linux-kernel, linux-arch, akpm, hpa, keescook, luto, mingo, tglx
On Thu, Oct 20, 2016 at 12:29:26PM +0200, Heiko Carstens wrote:
> On Wed, Oct 19, 2016 at 07:28:13PM +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>
> > Cc: Andrew Morton <akpm@linux-foundation.org>
> > Cc: Andy Lutomirski <luto@kernel.org>
> > Cc: Heiko Carstens <heiko.carstens@de.ibm.com>
> > Cc: Kees Cook <keescook@chromium.org>
> > ---
> > include/linux/thread_info.h | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/include/linux/thread_info.h b/include/linux/thread_info.h
> > index c75c6ab..ef1f4b0 100644
> > --- a/include/linux/thread_info.h
> > +++ b/include/linux/thread_info.h
> > @@ -12,6 +12,7 @@
> > #include <linux/restart_block.h>
> >
> > #ifdef CONFIG_THREAD_INFO_IN_TASK
> > +#include <asm/current.h>
> > #define current_thread_info() ((struct thread_info *)current)
> > #endif
>
> FWIW:
> Acked-by: Heiko Carstens <heiko.carstens@de.ibm.com>
Cheers!
Would you be happy to do the same for patch 2?
Once these have a couple more acks I'll drop these on a (hopefully)
stable branch, tagged so that we can both use them as a base.
Thanks,
Mark.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/3] thread_info: include <current.h> for THREAD_INFO_IN_TASK
2016-10-19 18:28 ` [PATCH 3/3] thread_info: include <current.h> for THREAD_INFO_IN_TASK Mark Rutland
2016-10-20 10:29 ` Heiko Carstens
@ 2016-10-27 23:13 ` Andy Lutomirski
2016-10-28 10:48 ` Mark Rutland
1 sibling, 1 reply; 15+ messages in thread
From: Andy Lutomirski @ 2016-10-27 23:13 UTC (permalink / raw)
To: Mark Rutland
Cc: linux-kernel, linux-arch, Andrew Morton, Heiko Carstens,
H. Peter Anvin, Kees Cook, Andrew Lutomirski, Ingo Molnar,
Thomas Gleixner
On Wed, Oct 19, 2016 at 11:28 AM, Mark Rutland <mark.rutland@arm.com> 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.
Reviewed-by: Andy Lutomirski <luto@kernel.org>
although it would be nice if you moved your description of why the
include is conditional into a comment.
--Andy
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/3] thread_info: include <current.h> for THREAD_INFO_IN_TASK
2016-10-27 23:13 ` Andy Lutomirski
@ 2016-10-28 10:48 ` Mark Rutland
0 siblings, 0 replies; 15+ messages in thread
From: Mark Rutland @ 2016-10-28 10:48 UTC (permalink / raw)
To: Andy Lutomirski
Cc: linux-kernel, linux-arch, Andrew Morton, Heiko Carstens,
H. Peter Anvin, Kees Cook, Andrew Lutomirski, Ingo Molnar,
Thomas Gleixner
On Thu, Oct 27, 2016 at 04:13:49PM -0700, Andy Lutomirski wrote:
> On Wed, Oct 19, 2016 at 11:28 AM, Mark Rutland <mark.rutland@arm.com> 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.
>
> Reviewed-by: Andy Lutomirski <luto@kernel.org>
Cheers!
> although it would be nice if you moved your description of why the
> include is conditional into a comment.
Agreed. I've folded in the below:
#ifdef CONFIG_THREAD_INFO_IN_TASK
/*
* For CONFIG_THREAD_INFO_IN_TASK kernels we need <asm/current.h> for the
* definition of current, but for !CONFIG_THREAD_INFO_IN_TASK kernels,
* including <asm/current.h> can cause a circular dependency on some platforms.
*/
#include <asm/current.h>
#define current_thread_info() ((struct thread_info *)current)
#endif
Thanks,
Mark.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 0/3] THREAD_INFO_IN_TASK prep work for arm64+s390
2016-10-19 18:28 [PATCH 0/3] THREAD_INFO_IN_TASK prep work for arm64+s390 Mark Rutland
` (2 preceding siblings ...)
2016-10-19 18:28 ` [PATCH 3/3] thread_info: include <current.h> for THREAD_INFO_IN_TASK Mark Rutland
@ 2016-10-24 10:12 ` Mark Rutland
3 siblings, 0 replies; 15+ messages in thread
From: Mark Rutland @ 2016-10-24 10:12 UTC (permalink / raw)
To: linux-kernel, akpm
Cc: linux-arch, heiko.carstens, hpa, keescook, luto, mingo, tglx
Hi Andrew,
On Wed, Oct 19, 2016 at 07:28:10PM +0100, Mark Rutland wrote:
> Heiko and I have been working on THREAD_INFO_IN_TASK for s390 and arm64
> respectively, and we're both targetting v4.10.
>
> These are the common core changes which we both require, which happen to
> touch x86 and some core headers. We'd either need these merged for
> v4.9-rc*, or placed on a stable branch/tag that we can both base atop
> of.
>
> I've put together a branch [1,2] based on v4.9-rc1, but the patches themselves
> are sorely lacking in relevant acks. Are people willing to help change that? ;)
Would you be happy to ack patches 2 and 3?
Ingo took patch 1 for 4.9-rc2, and these are the only core parts s390
and arm64 need to implement THREAD_INFO_IN_TASK for v4.10.
I'd like to put them on a stable branch.
Thanks,
Mark.
> [1] git://git.kernel.org/pub/scm/linux/kernel/git/mark/linux.git core/ti-stack-split
> [2] https://git.kernel.org/cgit/linux/kernel/git/mark/linux.git/log/?h=core/ti-stack-split
>
> Heiko Carstens (1):
> sched/core,x86: make struct thread_info arch specific again
>
> Mark Rutland (2):
> thread_info: factor out restart_block
> thread_info: include <current.h> for THREAD_INFO_IN_TASK
>
> arch/x86/include/asm/thread_info.h | 9 +++++++
> include/linux/restart_block.h | 51 ++++++++++++++++++++++++++++++++++++
> include/linux/thread_info.h | 53 ++------------------------------------
> 3 files changed, 62 insertions(+), 51 deletions(-)
> create mode 100644 include/linux/restart_block.h
>
> --
> 1.9.1
>
^ permalink raw reply [flat|nested] 15+ messages in thread