Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1421618 > unrolled thread

[PATCH] sched: unlikely corrupted stack end

Started byWANG Chao <wcwxyz@gmail.com>
First post2016-06-14 08:50 +0200
Last post2016-06-15 11:00 +0200
Articles 10 — 3 participants

Back to article view | Back to linux.kernel

Contents

  [PATCH] sched: unlikely corrupted stack end WANG Chao <wcwxyz@gmail.com> - 2016-06-14 08:50 +0200
    Re: [PATCH] sched: unlikely corrupted stack end Ingo Molnar <mingo@kernel.org> - 2016-06-14 09:50 +0200
    Re: [PATCH] sched: unlikely corrupted stack end Peter Zijlstra <peterz@infradead.org> - 2016-06-14 10:20 +0200
    [PATCH v2] sched: unlikely corrupted stack end WANG Chao <wcwxyz@gmail.com> - 2016-06-14 10:30 +0200
      Re: [PATCH v2] sched: unlikely corrupted stack end Ingo Molnar <mingo@kernel.org> - 2016-06-14 11:00 +0200
        Re: [PATCH v2] sched: unlikely corrupted stack end WANG Chao <wcwxyz@gmail.com> - 2016-06-14 12:20 +0200
          Re: [PATCH v2] sched: unlikely corrupted stack end Ingo Molnar <mingo@kernel.org> - 2016-06-14 12:30 +0200
            Re: [PATCH v2] sched: unlikely corrupted stack end WANG Chao <wcwxyz@gmail.com> - 2016-06-14 19:00 +0200
              Re: [PATCH v2] sched: unlikely corrupted stack end Ingo Molnar <mingo@kernel.org> - 2016-06-15 10:30 +0200
                [PATCH v3] sched: unlikely corrupted stack end WANG Chao <wcwxyz@gmail.com> - 2016-06-15 11:00 +0200

#1421618 — [PATCH] sched: unlikely corrupted stack end

FromWANG Chao <wcwxyz@gmail.com>
Date2016-06-14 08:50 +0200
Subject[PATCH] sched: unlikely corrupted stack end
Message-ID<rJQkG-16U-11@gated-at.bofh.it>
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

[toc] | [next] | [standalone]

#1421635

FromIngo Molnar <mingo@kernel.org>
Date2016-06-14 09:50 +0200
Message-ID<rJRgJ-1Ld-3@gated-at.bofh.it>
In reply to#1421618
* 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

[toc] | [prev] | [next] | [standalone]

#1421645

FromPeter Zijlstra <peterz@infradead.org>
Date2016-06-14 10:20 +0200
Message-ID<rJRJL-2cK-1@gated-at.bofh.it>
In reply to#1421618
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.

[toc] | [prev] | [next] | [standalone]

#1421663 — [PATCH v2] sched: unlikely corrupted stack end

FromWANG Chao <wcwxyz@gmail.com>
Date2016-06-14 10:30 +0200
Subject[PATCH v2] sched: unlikely corrupted stack end
Message-ID<rJRTs-2gr-11@gated-at.bofh.it>
In reply to#1421618
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

[toc] | [prev] | [next] | [standalone]

#1421678 — Re: [PATCH v2] sched: unlikely corrupted stack end

FromIngo Molnar <mingo@kernel.org>
Date2016-06-14 11:00 +0200
SubjectRe: [PATCH v2] sched: unlikely corrupted stack end
Message-ID<rJSmt-2tW-5@gated-at.bofh.it>
In reply to#1421663
* 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

[toc] | [prev] | [next] | [standalone]

#1421748 — Re: [PATCH v2] sched: unlikely corrupted stack end

FromWANG Chao <wcwxyz@gmail.com>
Date2016-06-14 12:20 +0200
SubjectRe: [PATCH v2] sched: unlikely corrupted stack end
Message-ID<rJTBU-3uB-25@gated-at.bofh.it>
In reply to#1421678
> 在 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()?

[toc] | [prev] | [next] | [standalone]

#1421753 — Re: [PATCH v2] sched: unlikely corrupted stack end

FromIngo Molnar <mingo@kernel.org>
Date2016-06-14 12:30 +0200
SubjectRe: [PATCH v2] sched: unlikely corrupted stack end
Message-ID<rJTLz-3z3-27@gated-at.bofh.it>
In reply to#1421748
* 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

[toc] | [prev] | [next] | [standalone]

#1422091 — Re: [PATCH v2] sched: unlikely corrupted stack end

FromWANG Chao <wcwxyz@gmail.com>
Date2016-06-14 19:00 +0200
SubjectRe: [PATCH v2] sched: unlikely corrupted stack end
Message-ID<rJZQZ-7rp-9@gated-at.bofh.it>
In reply to#1421753
> 在 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

[toc] | [prev] | [next] | [standalone]

#1422778 — Re: [PATCH v2] sched: unlikely corrupted stack end

FromIngo Molnar <mingo@kernel.org>
Date2016-06-15 10:30 +0200
SubjectRe: [PATCH v2] sched: unlikely corrupted stack end
Message-ID<rKen0-6S-29@gated-at.bofh.it>
In reply to#1422091
* 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

[toc] | [prev] | [next] | [standalone]

#1422796 — [PATCH v3] sched: unlikely corrupted stack end

FromWANG Chao <wcwxyz@gmail.com>
Date2016-06-15 11:00 +0200
Subject[PATCH v3] sched: unlikely corrupted stack end
Message-ID<rKeQ2-iF-9@gated-at.bofh.it>
In reply to#1422778
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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web