Message ID | b03212d03f5f112ffa778c30012c7f8301f64929.1481220077.git.jpoimboe@redhat.com (mailing list archive) |
---|---|
State | Superseded, archived |
Headers | show |
On Thu 2016-12-08 12:08:27, Josh Poimboeuf wrote: > The _TIF_ALLWORK_MASK macro automatically includes the least-significant > 16 bits of the thread_info flags, which is less than obvious and tends > to create confusion and surprises when reading or modifying the code. > > Define the flags explicitly. > > Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com> > --- > arch/x86/include/asm/thread_info.h | 9 ++++----- > 1 file changed, 4 insertions(+), 5 deletions(-) > > diff --git a/arch/x86/include/asm/thread_info.h b/arch/x86/include/asm/thread_info.h > index ad6f5eb0..1fe6043 100644 > --- a/arch/x86/include/asm/thread_info.h > +++ b/arch/x86/include/asm/thread_info.h > @@ -73,9 +73,6 @@ struct thread_info { > * thread information flags > * - these are process state flags that various assembly files > * may need to access > - * - pending work-to-be-done flags are in LSW Yup, this is not true because also some flags from the most significant bits are in the _TIF_ALLWORK_MASK. > - * - other flags in MSW > - * Warning: layout of LSW is hardcoded in entry.S > */ > #define TIF_SYSCALL_TRACE 0 /* syscall trace active */ > #define TIF_NOTIFY_RESUME 1 /* callback before returning to user */ > @@ -133,8 +130,10 @@ struct thread_info { > > /* work to do on any return to user space */ > #define _TIF_ALLWORK_MASK \ > - ((0x0000FFFF & ~_TIF_SECCOMP) | _TIF_SYSCALL_TRACEPOINT | \ > - _TIF_NOHZ) > + (_TIF_SYSCALL_TRACE | _TIF_NOTIFY_RESUME | _TIF_SIGPENDING | \ > + _TIF_SINGLESTEP | _TIF_NEED_RESCHED | _TIF_SYSCALL_EMU | \ > + _TIF_SYSCALL_AUDIT | _TIF_USER_RETURN_NOTIFY | _TIF_UPROBE | \ > + _TIF_SYSCALL_TRACEPOINT | _TIF_NOHZ) All flags are sorted by the number except for _TIF_SINGLESTEP and _TIF_NEED_RESCHED ;-) The patch does not change the existing behavior. The same existing flags are listed. Reviewed-by: Petr Mladek <pmladek@suse.com> Best Regards, Petr
On Fri, Dec 16, 2016 at 03:17:35PM +0100, Petr Mladek wrote: > On Thu 2016-12-08 12:08:27, Josh Poimboeuf wrote: > > The _TIF_ALLWORK_MASK macro automatically includes the least-significant > > 16 bits of the thread_info flags, which is less than obvious and tends > > to create confusion and surprises when reading or modifying the code. > > > > Define the flags explicitly. > > > > Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com> > > --- > > arch/x86/include/asm/thread_info.h | 9 ++++----- > > 1 file changed, 4 insertions(+), 5 deletions(-) > > > > diff --git a/arch/x86/include/asm/thread_info.h b/arch/x86/include/asm/thread_info.h > > index ad6f5eb0..1fe6043 100644 > > --- a/arch/x86/include/asm/thread_info.h > > +++ b/arch/x86/include/asm/thread_info.h > > @@ -73,9 +73,6 @@ struct thread_info { > > * thread information flags > > * - these are process state flags that various assembly files > > * may need to access > > - * - pending work-to-be-done flags are in LSW > > Yup, this is not true because also some flags from the most > significant bits are in the _TIF_ALLWORK_MASK. > > > - * - other flags in MSW > > - * Warning: layout of LSW is hardcoded in entry.S > > */ > > #define TIF_SYSCALL_TRACE 0 /* syscall trace active */ > > #define TIF_NOTIFY_RESUME 1 /* callback before returning to user */ > > @@ -133,8 +130,10 @@ struct thread_info { > > > > /* work to do on any return to user space */ > > #define _TIF_ALLWORK_MASK \ > > - ((0x0000FFFF & ~_TIF_SECCOMP) | _TIF_SYSCALL_TRACEPOINT | \ > > - _TIF_NOHZ) > > + (_TIF_SYSCALL_TRACE | _TIF_NOTIFY_RESUME | _TIF_SIGPENDING | \ > > + _TIF_SINGLESTEP | _TIF_NEED_RESCHED | _TIF_SYSCALL_EMU | \ > > + _TIF_SYSCALL_AUDIT | _TIF_USER_RETURN_NOTIFY | _TIF_UPROBE | \ > > + _TIF_SYSCALL_TRACEPOINT | _TIF_NOHZ) > > All flags are sorted by the number except for > _TIF_SINGLESTEP and _TIF_NEED_RESCHED ;-) You're right, I'll swap them :-) > > The patch does not change the existing behavior. The same > existing flags are listed. > > Reviewed-by: Petr Mladek <pmladek@suse.com> > > Best Regards, > Petr
On Thu, 8 Dec 2016, Josh Poimboeuf wrote: > The _TIF_ALLWORK_MASK macro automatically includes the least-significant > 16 bits of the thread_info flags, which is less than obvious and tends > to create confusion and surprises when reading or modifying the code. Yes. > Define the flags explicitly. > > Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com> With _TIF_SINGLESTEP and _TIF_NEED_RESCHED swapped you can add my Reviewed-by: Miroslav Benes <mbenes@suse.cz> Miroslav > --- > arch/x86/include/asm/thread_info.h | 9 ++++----- > 1 file changed, 4 insertions(+), 5 deletions(-) > > diff --git a/arch/x86/include/asm/thread_info.h b/arch/x86/include/asm/thread_info.h > index ad6f5eb0..1fe6043 100644 > --- a/arch/x86/include/asm/thread_info.h > +++ b/arch/x86/include/asm/thread_info.h > @@ -73,9 +73,6 @@ struct thread_info { > * thread information flags > * - these are process state flags that various assembly files > * may need to access > - * - pending work-to-be-done flags are in LSW > - * - other flags in MSW > - * Warning: layout of LSW is hardcoded in entry.S > */ > #define TIF_SYSCALL_TRACE 0 /* syscall trace active */ > #define TIF_NOTIFY_RESUME 1 /* callback before returning to user */ > @@ -133,8 +130,10 @@ struct thread_info { > > /* work to do on any return to user space */ > #define _TIF_ALLWORK_MASK \ > - ((0x0000FFFF & ~_TIF_SECCOMP) | _TIF_SYSCALL_TRACEPOINT | \ > - _TIF_NOHZ) > + (_TIF_SYSCALL_TRACE | _TIF_NOTIFY_RESUME | _TIF_SIGPENDING | \ > + _TIF_SINGLESTEP | _TIF_NEED_RESCHED | _TIF_SYSCALL_EMU | \ > + _TIF_SYSCALL_AUDIT | _TIF_USER_RETURN_NOTIFY | _TIF_UPROBE | \ > + _TIF_SYSCALL_TRACEPOINT | _TIF_NOHZ) > > /* flags to check in __switch_to() */ > #define _TIF_WORK_CTXSW \ > -- > 2.7.4 >
On Thursday 08 December 2016 11:38 PM, Josh Poimboeuf wrote: > The _TIF_ALLWORK_MASK macro automatically includes the least-significant > 16 bits of the thread_info flags, which is less than obvious and tends > to create confusion and surprises when reading or modifying the code. > > Define the flags explicitly. > > Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com> For the version with swapped _TIF_SINGLESTEP and _TIF_NEED_RESCHED flags. Reviewed-by: Kamalesh Babulal <kamalesh@linux.vnet.ibm.com>
diff --git a/arch/x86/include/asm/thread_info.h b/arch/x86/include/asm/thread_info.h index ad6f5eb0..1fe6043 100644 --- a/arch/x86/include/asm/thread_info.h +++ b/arch/x86/include/asm/thread_info.h @@ -73,9 +73,6 @@ struct thread_info { * thread information flags * - these are process state flags that various assembly files * may need to access - * - pending work-to-be-done flags are in LSW - * - other flags in MSW - * Warning: layout of LSW is hardcoded in entry.S */ #define TIF_SYSCALL_TRACE 0 /* syscall trace active */ #define TIF_NOTIFY_RESUME 1 /* callback before returning to user */ @@ -133,8 +130,10 @@ struct thread_info { /* work to do on any return to user space */ #define _TIF_ALLWORK_MASK \ - ((0x0000FFFF & ~_TIF_SECCOMP) | _TIF_SYSCALL_TRACEPOINT | \ - _TIF_NOHZ) + (_TIF_SYSCALL_TRACE | _TIF_NOTIFY_RESUME | _TIF_SIGPENDING | \ + _TIF_SINGLESTEP | _TIF_NEED_RESCHED | _TIF_SYSCALL_EMU | \ + _TIF_SYSCALL_AUDIT | _TIF_USER_RETURN_NOTIFY | _TIF_UPROBE | \ + _TIF_SYSCALL_TRACEPOINT | _TIF_NOHZ) /* flags to check in __switch_to() */ #define _TIF_WORK_CTXSW \
The _TIF_ALLWORK_MASK macro automatically includes the least-significant 16 bits of the thread_info flags, which is less than obvious and tends to create confusion and surprises when reading or modifying the code. Define the flags explicitly. Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com> --- arch/x86/include/asm/thread_info.h | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-)