* [PATCH] sched: unlikely corrupted stack end
@ 2016-06-14 6:43 WANG Chao
2016-06-14 7:43 ` Ingo Molnar
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: WANG Chao @ 2016-06-14 6:43 UTC (permalink / raw)
To: Ingo Molnar, Peter Zijlstra; +Cc: linux-kernel
unlikely() was dropped in commit ce03e41 ("sched/core: Drop unlikely
behind BUG_ON()"), but commit 29d6455 ("sched: panic on corrupted stack
end") dropped BUG_ON() and called panic directly.
Now we should bring unlikely() back for branch prediction.
Signed-off-by: WANG Chao <wcwxyz@gmail.com>
---
kernel/sched/core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 017d539..7db442c 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -3170,7 +3170,7 @@ static noinline void __schedule_bug(struct task_struct *prev)
static inline void schedule_debug(struct task_struct *prev)
{
#ifdef CONFIG_SCHED_STACK_END_CHECK
- if (task_stack_end_corrupted(prev))
+ if (unlikely(task_stack_end_corrupted(prev)))
panic("corrupted stack end detected inside scheduler\n");
#endif
--
2.8.4
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] sched: unlikely corrupted stack end
2016-06-14 6:43 [PATCH] sched: unlikely corrupted stack end WANG Chao
@ 2016-06-14 7:43 ` Ingo Molnar
2016-06-14 8:17 ` Peter Zijlstra
2016-06-14 8:24 ` [PATCH v2] " WANG Chao
2 siblings, 0 replies; 11+ messages in thread
From: Ingo Molnar @ 2016-06-14 7:43 UTC (permalink / raw)
To: WANG Chao; +Cc: Ingo Molnar, Peter Zijlstra, linux-kernel
* WANG Chao <wcwxyz@gmail.com> wrote:
> unlikely() was dropped in commit ce03e41 ("sched/core: Drop unlikely
> behind BUG_ON()"), but commit 29d6455 ("sched: panic on corrupted stack
> end") dropped BUG_ON() and called panic directly.
>
> Now we should bring unlikely() back for branch prediction.
>
> Signed-off-by: WANG Chao <wcwxyz@gmail.com>
> ---
> kernel/sched/core.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index 017d539..7db442c 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -3170,7 +3170,7 @@ static noinline void __schedule_bug(struct task_struct *prev)
> static inline void schedule_debug(struct task_struct *prev)
> {
> #ifdef CONFIG_SCHED_STACK_END_CHECK
> - if (task_stack_end_corrupted(prev))
> + if (unlikely(task_stack_end_corrupted(prev)))
> panic("corrupted stack end detected inside scheduler\n");
> #endif
It would be better and cleaner to push that into the task_stack_end_corrupted()
definition. (and to turn it into an inline function while we are touching it.)
Thanks,
Ingo
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] sched: unlikely corrupted stack end
2016-06-14 6:43 [PATCH] sched: unlikely corrupted stack end WANG Chao
2016-06-14 7:43 ` Ingo Molnar
@ 2016-06-14 8:17 ` Peter Zijlstra
2016-06-14 8:24 ` [PATCH v2] " WANG Chao
2 siblings, 0 replies; 11+ messages in thread
From: Peter Zijlstra @ 2016-06-14 8:17 UTC (permalink / raw)
To: WANG Chao; +Cc: Ingo Molnar, linux-kernel
On Tue, Jun 14, 2016 at 02:43:06PM +0800, WANG Chao wrote:
> unlikely() was dropped in commit ce03e41 ("sched/core: Drop unlikely
> behind BUG_ON()"), but commit 29d6455 ("sched: panic on corrupted stack
> end") dropped BUG_ON() and called panic directly.
Please use git config core.abbrev=12 and try again.
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2] sched: unlikely corrupted stack end
2016-06-14 6:43 [PATCH] sched: unlikely corrupted stack end WANG Chao
2016-06-14 7:43 ` Ingo Molnar
2016-06-14 8:17 ` Peter Zijlstra
@ 2016-06-14 8:24 ` WANG Chao
2016-06-14 8:56 ` Ingo Molnar
2016-06-14 9:08 ` [PATCH v2] " kbuild test robot
2 siblings, 2 replies; 11+ messages in thread
From: WANG Chao @ 2016-06-14 8:24 UTC (permalink / raw)
To: Ingo Molnar, Peter Zijlstra; +Cc: linux-kernel
unlikely() was dropped in commit ce03e4137bb2 ("sched/core: Drop
unlikely behind BUG_ON()"), but commit 29d6455178a0 ("sched: panic on
corrupted stack end") dropped BUG_ON() and called panic directly.
Now we should bring unlikely() back for branch prediction. While we're
at it, it's better and cleaner to turn task_stack_end_corrupted() into
inline function.
Signed-off-by: WANG Chao <wcwxyz@gmail.com>
---
include/linux/sched.h | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/include/linux/sched.h b/include/linux/sched.h
index 6e42ada26345..797ca1975431 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -2997,8 +2997,11 @@ static inline unsigned long *end_of_stack(struct task_struct *p)
}
#endif
-#define task_stack_end_corrupted(task) \
- (*(end_of_stack(task)) != STACK_END_MAGIC)
+
+static inline int task_stack_end_corrupted(struct task_struct *p)
+{
+ return unlikely(*(end_of_stack(p)) != STACK_END_MAGIC);
+}
static inline int object_is_on_stack(void *obj)
{
--
2.8.4
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v2] sched: unlikely corrupted stack end
2016-06-14 8:24 ` [PATCH v2] " WANG Chao
@ 2016-06-14 8:56 ` Ingo Molnar
2016-06-14 10:17 ` WANG Chao
2016-06-14 9:08 ` [PATCH v2] " kbuild test robot
1 sibling, 1 reply; 11+ messages in thread
From: Ingo Molnar @ 2016-06-14 8:56 UTC (permalink / raw)
To: WANG Chao; +Cc: Ingo Molnar, Peter Zijlstra, linux-kernel
* WANG Chao <wcwxyz@gmail.com> wrote:
> unlikely() was dropped in commit ce03e4137bb2 ("sched/core: Drop
> unlikely behind BUG_ON()"), but commit 29d6455178a0 ("sched: panic on
> corrupted stack end") dropped BUG_ON() and called panic directly.
>
> Now we should bring unlikely() back for branch prediction. While we're
> at it, it's better and cleaner to turn task_stack_end_corrupted() into
> inline function.
>
> Signed-off-by: WANG Chao <wcwxyz@gmail.com>
> ---
> include/linux/sched.h | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/sched.h b/include/linux/sched.h
> index 6e42ada26345..797ca1975431 100644
> --- a/include/linux/sched.h
> +++ b/include/linux/sched.h
> @@ -2997,8 +2997,11 @@ static inline unsigned long *end_of_stack(struct task_struct *p)
> }
>
> #endif
> -#define task_stack_end_corrupted(task) \
> - (*(end_of_stack(task)) != STACK_END_MAGIC)
> +
> +static inline int task_stack_end_corrupted(struct task_struct *p)
> +{
> + return unlikely(*(end_of_stack(p)) != STACK_END_MAGIC);
> +}
The passed in pointer should be const, and the extra parentheses around the
end_of_stack() call are not needed anymore (since it's now proper C code now).
Thanks,
Ingo
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] sched: unlikely corrupted stack end
2016-06-14 8:24 ` [PATCH v2] " WANG Chao
2016-06-14 8:56 ` Ingo Molnar
@ 2016-06-14 9:08 ` kbuild test robot
1 sibling, 0 replies; 11+ messages in thread
From: kbuild test robot @ 2016-06-14 9:08 UTC (permalink / raw)
To: WANG Chao; +Cc: kbuild-all, Ingo Molnar, Peter Zijlstra, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 5499 bytes --]
Hi,
[auto build test ERROR on tip/sched/core]
[also build test ERROR on v4.7-rc3 next-20160614]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/WANG-Chao/sched-unlikely-corrupted-stack-end/20160614-162711
config: ia64-allmodconfig (attached as .config)
compiler: ia64-linux-gcc (GCC) 4.9.0
reproduce:
wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=ia64
All error/warnings (new ones prefixed by >>):
warning: (FAULT_INJECTION_STACKTRACE_FILTER && LATENCYTOP && KMEMCHECK && LOCKDEP) selects FRAME_POINTER which has unmet direct dependencies (DEBUG_KERNEL && (CRIS || M68K || FRV || UML || AVR32 || SUPERH || BLACKFIN || MN10300 || METAG) || ARCH_WANT_FRAME_POINTERS)
warning: (FAULT_INJECTION_STACKTRACE_FILTER && LATENCYTOP && KMEMCHECK && LOCKDEP) selects FRAME_POINTER which has unmet direct dependencies (DEBUG_KERNEL && (CRIS || M68K || FRV || UML || AVR32 || SUPERH || BLACKFIN || MN10300 || METAG) || ARCH_WANT_FRAME_POINTERS)
In file included from include/uapi/linux/stddef.h:1:0,
from include/linux/stddef.h:4,
from include/uapi/linux/posix_types.h:4,
from include/uapi/linux/types.h:13,
from include/linux/types.h:5,
from include/uapi/linux/capability.h:16,
from include/linux/capability.h:15,
from include/linux/sched.h:15,
from arch/ia64/kernel/asm-offsets.c:9:
include/linux/sched.h: In function 'task_stack_end_corrupted':
>> arch/ia64/include/asm/ptrace.h:37:29: error: 'IA64_TASK_SIZE' undeclared (first use in this function)
#define IA64_RBS_OFFSET ((IA64_TASK_SIZE + IA64_THREAD_INFO_SIZE + 31) & ~31)
^
include/linux/compiler.h:170:42: note: in definition of macro 'unlikely'
# define unlikely(x) __builtin_expect(!!(x), 0)
^
>> arch/ia64/include/asm/thread_info.h:74:57: note: in expansion of macro 'IA64_RBS_OFFSET'
#define end_of_stack(p) (unsigned long *)((void *)(p) + IA64_RBS_OFFSET)
^
>> include/linux/sched.h:3006:20: note: in expansion of macro 'end_of_stack'
return unlikely(*(end_of_stack(p)) != STACK_END_MAGIC);
^
arch/ia64/include/asm/ptrace.h:37:29: note: each undeclared identifier is reported only once for each function it appears in
#define IA64_RBS_OFFSET ((IA64_TASK_SIZE + IA64_THREAD_INFO_SIZE + 31) & ~31)
^
include/linux/compiler.h:170:42: note: in definition of macro 'unlikely'
# define unlikely(x) __builtin_expect(!!(x), 0)
^
>> arch/ia64/include/asm/thread_info.h:74:57: note: in expansion of macro 'IA64_RBS_OFFSET'
#define end_of_stack(p) (unsigned long *)((void *)(p) + IA64_RBS_OFFSET)
^
>> include/linux/sched.h:3006:20: note: in expansion of macro 'end_of_stack'
return unlikely(*(end_of_stack(p)) != STACK_END_MAGIC);
^
>> arch/ia64/include/asm/ptrace.h:37:46: error: 'IA64_THREAD_INFO_SIZE' undeclared (first use in this function)
#define IA64_RBS_OFFSET ((IA64_TASK_SIZE + IA64_THREAD_INFO_SIZE + 31) & ~31)
^
include/linux/compiler.h:170:42: note: in definition of macro 'unlikely'
# define unlikely(x) __builtin_expect(!!(x), 0)
^
>> arch/ia64/include/asm/thread_info.h:74:57: note: in expansion of macro 'IA64_RBS_OFFSET'
#define end_of_stack(p) (unsigned long *)((void *)(p) + IA64_RBS_OFFSET)
^
>> include/linux/sched.h:3006:20: note: in expansion of macro 'end_of_stack'
return unlikely(*(end_of_stack(p)) != STACK_END_MAGIC);
^
make[2]: *** [arch/ia64/kernel/asm-offsets.s] Error 1
make[2]: Target '__build' not remade because of errors.
make[1]: *** [prepare0] Error 2
make[1]: Target 'prepare' not remade because of errors.
make: *** [sub-make] Error 2
vim +/end_of_stack +3006 include/linux/sched.h
2990 * When the stack grows up, this is the highest address.
2991 * Beyond that position, we corrupt data on the next page.
2992 */
2993 static inline unsigned long *end_of_stack(struct task_struct *p)
2994 {
2995 #ifdef CONFIG_STACK_GROWSUP
2996 return (unsigned long *)((unsigned long)task_thread_info(p) + THREAD_SIZE) - 1;
2997 #else
2998 return (unsigned long *)(task_thread_info(p) + 1);
2999 #endif
3000 }
3001
3002 #endif
3003
3004 static inline int task_stack_end_corrupted(struct task_struct *p)
3005 {
> 3006 return unlikely(*(end_of_stack(p)) != STACK_END_MAGIC);
3007 }
3008
3009 static inline int object_is_on_stack(void *obj)
3010 {
3011 void *stack = task_stack_page(current);
3012
3013 return (obj >= stack) && (obj < (stack + THREAD_SIZE));
3014 }
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 43700 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] sched: unlikely corrupted stack end
2016-06-14 8:56 ` Ingo Molnar
@ 2016-06-14 10:17 ` WANG Chao
2016-06-14 10:26 ` Ingo Molnar
0 siblings, 1 reply; 11+ messages in thread
From: WANG Chao @ 2016-06-14 10:17 UTC (permalink / raw)
To: Ingo Molnar; +Cc: Ingo Molnar, Peter Zijlstra, linux-kernel
> 在 2016年6月14日,下午4:56,Ingo Molnar <mingo@kernel.org> 写道:
>
>
> * WANG Chao <wcwxyz@gmail.com> wrote:
>
>> unlikely() was dropped in commit ce03e4137bb2 ("sched/core: Drop
>> unlikely behind BUG_ON()"), but commit 29d6455178a0 ("sched: panic on
>> corrupted stack end") dropped BUG_ON() and called panic directly.
>>
>> Now we should bring unlikely() back for branch prediction. While we're
>> at it, it's better and cleaner to turn task_stack_end_corrupted() into
>> inline function.
>>
>> Signed-off-by: WANG Chao <wcwxyz@gmail.com>
>> ---
>> include/linux/sched.h | 7 +++++--
>> 1 file changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/include/linux/sched.h b/include/linux/sched.h
>> index 6e42ada26345..797ca1975431 100644
>> --- a/include/linux/sched.h
>> +++ b/include/linux/sched.h
>> @@ -2997,8 +2997,11 @@ static inline unsigned long *end_of_stack(struct task_struct *p)
>> }
>>
>> #endif
>> -#define task_stack_end_corrupted(task) \
>> - (*(end_of_stack(task)) != STACK_END_MAGIC)
>> +
>> +static inline int task_stack_end_corrupted(struct task_struct *p)
>> +{
>> + return unlikely(*(end_of_stack(p)) != STACK_END_MAGIC);
>> +}
>
> The passed in pointer should be const, and the extra parentheses around the
> end_of_stack() call are not needed anymore (since it's now proper C code now).
end_of_stack() will discard const and cause an compiler warning.
Should I add const to end_of_stack()?
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] sched: unlikely corrupted stack end
2016-06-14 10:17 ` WANG Chao
@ 2016-06-14 10:26 ` Ingo Molnar
2016-06-14 16:55 ` WANG Chao
0 siblings, 1 reply; 11+ messages in thread
From: Ingo Molnar @ 2016-06-14 10:26 UTC (permalink / raw)
To: WANG Chao; +Cc: Ingo Molnar, Peter Zijlstra, linux-kernel
* WANG Chao <wcwxyz@gmail.com> wrote:
>
> > 在 2016年6月14日,下午4:56,Ingo Molnar <mingo@kernel.org> 写道:
> >
> >
> > * WANG Chao <wcwxyz@gmail.com> wrote:
> >
> >> unlikely() was dropped in commit ce03e4137bb2 ("sched/core: Drop
> >> unlikely behind BUG_ON()"), but commit 29d6455178a0 ("sched: panic on
> >> corrupted stack end") dropped BUG_ON() and called panic directly.
> >>
> >> Now we should bring unlikely() back for branch prediction. While we're
> >> at it, it's better and cleaner to turn task_stack_end_corrupted() into
> >> inline function.
> >>
> >> Signed-off-by: WANG Chao <wcwxyz@gmail.com>
> >> ---
> >> include/linux/sched.h | 7 +++++--
> >> 1 file changed, 5 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/include/linux/sched.h b/include/linux/sched.h
> >> index 6e42ada26345..797ca1975431 100644
> >> --- a/include/linux/sched.h
> >> +++ b/include/linux/sched.h
> >> @@ -2997,8 +2997,11 @@ static inline unsigned long *end_of_stack(struct task_struct *p)
> >> }
> >>
> >> #endif
> >> -#define task_stack_end_corrupted(task) \
> >> - (*(end_of_stack(task)) != STACK_END_MAGIC)
> >> +
> >> +static inline int task_stack_end_corrupted(struct task_struct *p)
> >> +{
> >> + return unlikely(*(end_of_stack(p)) != STACK_END_MAGIC);
> >> +}
> >
> > The passed in pointer should be const, and the extra parentheses around the
> > end_of_stack() call are not needed anymore (since it's now proper C code now).
>
> end_of_stack() will discard const and cause an compiler warning.
> Should I add const to end_of_stack()?
Yes. Also make sure ia64 still builds and such.
Thanks,
Ingo
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] sched: unlikely corrupted stack end
2016-06-14 10:26 ` Ingo Molnar
@ 2016-06-14 16:55 ` WANG Chao
2016-06-15 8:25 ` Ingo Molnar
0 siblings, 1 reply; 11+ messages in thread
From: WANG Chao @ 2016-06-14 16:55 UTC (permalink / raw)
To: Ingo Molnar; +Cc: Ingo Molnar, Peter Zijlstra, linux-kernel
> 在 2016年6月14日,下午6:26,Ingo Molnar <mingo@kernel.org> 写道:
>
>
> * WANG Chao <wcwxyz@gmail.com> wrote:
>
>>
>>> 在 2016年6月14日,下午4:56,Ingo Molnar <mingo@kernel.org> 写道:
>>>
>>>
>>> * WANG Chao <wcwxyz@gmail.com> wrote:
>>>
>>>> unlikely() was dropped in commit ce03e4137bb2 ("sched/core: Drop
>>>> unlikely behind BUG_ON()"), but commit 29d6455178a0 ("sched: panic on
>>>> corrupted stack end") dropped BUG_ON() and called panic directly.
>>>>
>>>> Now we should bring unlikely() back for branch prediction. While we're
>>>> at it, it's better and cleaner to turn task_stack_end_corrupted() into
>>>> inline function.
>>>>
>>>> Signed-off-by: WANG Chao <wcwxyz@gmail.com>
>>>> ---
>>>> include/linux/sched.h | 7 +++++--
>>>> 1 file changed, 5 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/include/linux/sched.h b/include/linux/sched.h
>>>> index 6e42ada26345..797ca1975431 100644
>>>> --- a/include/linux/sched.h
>>>> +++ b/include/linux/sched.h
>>>> @@ -2997,8 +2997,11 @@ static inline unsigned long *end_of_stack(struct task_struct *p)
>>>> }
>>>>
>>>> #endif
>>>> -#define task_stack_end_corrupted(task) \
>>>> - (*(end_of_stack(task)) != STACK_END_MAGIC)
>>>> +
>>>> +static inline int task_stack_end_corrupted(struct task_struct *p)
>>>> +{
>>>> + return unlikely(*(end_of_stack(p)) != STACK_END_MAGIC);
>>>> +}
>>>
>>> The passed in pointer should be const, and the extra parentheses around the
>>> end_of_stack() call are not needed anymore (since it's now proper C code now).
>>
>> end_of_stack() will discard const and cause an compiler warning.
>> Should I add const to end_of_stack()?
>
> Yes. Also make sure ia64 still builds and such.
It seems convert task_stack_end_corrupted() into inline isn’t trivial.
In ia64, end_of_stack() is expanded to:
(unsigned long *)((void *)(p) + ((IA64_TASK_SIZE + IA64_THREAD_INFO_SIZE + 31) & ~31))
IA64_TASK_SIZE and IA64_THREAD_INFO_SIZE is defined in arch/ia64/kernel/asm-offsets.c,
which needs to include linux/sched.h.
So the problem is task_stack_end_corrupted() doesn’t compile before asm-offsets.c is compiled.
asm-offsets.c also needs to include linux/sched.h to compile. I think maybe that’s why
task_stack_end_corrupted() is introduced as marco, not inline.
Any idea?
Thanks
WANG Chao
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] sched: unlikely corrupted stack end
2016-06-14 16:55 ` WANG Chao
@ 2016-06-15 8:25 ` Ingo Molnar
2016-06-15 8:53 ` [PATCH v3] " WANG Chao
0 siblings, 1 reply; 11+ messages in thread
From: Ingo Molnar @ 2016-06-15 8:25 UTC (permalink / raw)
To: WANG Chao; +Cc: Ingo Molnar, Peter Zijlstra, linux-kernel
* WANG Chao <wcwxyz@gmail.com> wrote:
>
> > 在 2016年6月14日,下午6:26,Ingo Molnar <mingo@kernel.org> 写道:
> >
> >
> > * WANG Chao <wcwxyz@gmail.com> wrote:
> >
> >>
> >>> 在 2016年6月14日,下午4:56,Ingo Molnar <mingo@kernel.org> 写道:
> >>>
> >>>
> >>> * WANG Chao <wcwxyz@gmail.com> wrote:
> >>>
> >>>> unlikely() was dropped in commit ce03e4137bb2 ("sched/core: Drop
> >>>> unlikely behind BUG_ON()"), but commit 29d6455178a0 ("sched: panic on
> >>>> corrupted stack end") dropped BUG_ON() and called panic directly.
> >>>>
> >>>> Now we should bring unlikely() back for branch prediction. While we're
> >>>> at it, it's better and cleaner to turn task_stack_end_corrupted() into
> >>>> inline function.
> >>>>
> >>>> Signed-off-by: WANG Chao <wcwxyz@gmail.com>
> >>>> ---
> >>>> include/linux/sched.h | 7 +++++--
> >>>> 1 file changed, 5 insertions(+), 2 deletions(-)
> >>>>
> >>>> diff --git a/include/linux/sched.h b/include/linux/sched.h
> >>>> index 6e42ada26345..797ca1975431 100644
> >>>> --- a/include/linux/sched.h
> >>>> +++ b/include/linux/sched.h
> >>>> @@ -2997,8 +2997,11 @@ static inline unsigned long *end_of_stack(struct task_struct *p)
> >>>> }
> >>>>
> >>>> #endif
> >>>> -#define task_stack_end_corrupted(task) \
> >>>> - (*(end_of_stack(task)) != STACK_END_MAGIC)
> >>>> +
> >>>> +static inline int task_stack_end_corrupted(struct task_struct *p)
> >>>> +{
> >>>> + return unlikely(*(end_of_stack(p)) != STACK_END_MAGIC);
> >>>> +}
> >>>
> >>> The passed in pointer should be const, and the extra parentheses around the
> >>> end_of_stack() call are not needed anymore (since it's now proper C code now).
> >>
> >> end_of_stack() will discard const and cause an compiler warning.
> >> Should I add const to end_of_stack()?
> >
> > Yes. Also make sure ia64 still builds and such.
>
> It seems convert task_stack_end_corrupted() into inline isn’t trivial.
>
> In ia64, end_of_stack() is expanded to:
>
> (unsigned long *)((void *)(p) + ((IA64_TASK_SIZE + IA64_THREAD_INFO_SIZE + 31) & ~31))
>
> IA64_TASK_SIZE and IA64_THREAD_INFO_SIZE is defined in arch/ia64/kernel/asm-offsets.c,
> which needs to include linux/sched.h.
>
> So the problem is task_stack_end_corrupted() doesn’t compile before asm-offsets.c is compiled.
> asm-offsets.c also needs to include linux/sched.h to compile. I think maybe that’s why
> task_stack_end_corrupted() is introduced as marco, not inline.
>
> Any idea?
Oh well ...
I guess we'll have to add the unlikely() to the macro itself.
Thanks,
Ingo
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v3] sched: unlikely corrupted stack end
2016-06-15 8:25 ` Ingo Molnar
@ 2016-06-15 8:53 ` WANG Chao
0 siblings, 0 replies; 11+ messages in thread
From: WANG Chao @ 2016-06-15 8:53 UTC (permalink / raw)
To: Ingo Molnar, Peter Zijlstra; +Cc: linux-kernel
unlikely() was dropped in commit ce03e4137bb2 ("sched/core: Drop
unlikely behind BUG_ON()"), but commit 29d6455178a0 ("sched: panic on
corrupted stack end") dropped BUG_ON() and called panic directly.
Now we should bring unlikely() back for branch prediction. While we're
at it, it's better and cleaner to add unlikely() to
task_stack_end_corrupted() macro.
Signed-off-by: WANG Chao <wcwxyz@gmail.com>
---
include/linux/sched.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/sched.h b/include/linux/sched.h
index 6e42ada26345..74a02bf30827 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -2998,7 +2998,7 @@ static inline unsigned long *end_of_stack(struct task_struct *p)
#endif
#define task_stack_end_corrupted(task) \
- (*(end_of_stack(task)) != STACK_END_MAGIC)
+ (unlikely(*(end_of_stack(task)) != STACK_END_MAGIC))
static inline int object_is_on_stack(void *obj)
{
--
2.9.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
end of thread, other threads:[~2016-06-15 8:54 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-14 6:43 [PATCH] sched: unlikely corrupted stack end WANG Chao
2016-06-14 7:43 ` Ingo Molnar
2016-06-14 8:17 ` Peter Zijlstra
2016-06-14 8:24 ` [PATCH v2] " WANG Chao
2016-06-14 8:56 ` Ingo Molnar
2016-06-14 10:17 ` WANG Chao
2016-06-14 10:26 ` Ingo Molnar
2016-06-14 16:55 ` WANG Chao
2016-06-15 8:25 ` Ingo Molnar
2016-06-15 8:53 ` [PATCH v3] " WANG Chao
2016-06-14 9:08 ` [PATCH v2] " kbuild test robot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).