[ I noticed this whilst hacking up SH support, applies to the current
tracing/hw-breakpoints topic branch. ]
flush_thread() tries to do a TIF_DEBUG check before calling in to
flush_thread_hw_breakpoint() (which subsequently clears the thread flag),
but for some reason, the x86 code is manually clearing TIF_DEBUG
immediately before the test, so this path will never be taken.
This kills off the erroneous clear_tsk_thread_flag() and lets
flush_thread_hw_breakpoint() actually get invoked.
Presumably folks were getting lucky with testing and the
free_thread_info() -> free_thread_xstate() path was taking care of the
flush there.
Signed-off-by: Paul Mundt <[email protected]>
---
arch/x86/kernel/process.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
index 1092a1a..c62f647 100644
--- a/arch/x86/kernel/process.c
+++ b/arch/x86/kernel/process.c
@@ -110,8 +110,6 @@ void flush_thread(void)
}
#endif
- clear_tsk_thread_flag(tsk, TIF_DEBUG);
-
if (unlikely(test_tsk_thread_flag(tsk, TIF_DEBUG)))
flush_thread_hw_breakpoint(tsk);
memset(tsk->thread.tls_array, 0, sizeof(tsk->thread.tls_array));
On Mon, Oct 05, 2009 at 07:23:06PM +0900, Paul Mundt wrote:
> [ I noticed this whilst hacking up SH support, applies to the current
> tracing/hw-breakpoints topic branch. ]
>
> flush_thread() tries to do a TIF_DEBUG check before calling in to
> flush_thread_hw_breakpoint() (which subsequently clears the thread flag),
> but for some reason, the x86 code is manually clearing TIF_DEBUG
> immediately before the test, so this path will never be taken.
>
> This kills off the erroneous clear_tsk_thread_flag() and lets
> flush_thread_hw_breakpoint() actually get invoked.
>
Yes, this is wrong and we missed it. Thanks for fixing!
> Presumably folks were getting lucky with testing and the
> free_thread_info() -> free_thread_xstate() path was taking care of the
> flush there.
>
> Signed-off-by: Paul Mundt <[email protected]>
Acked-by: K.Prasad <[email protected]>
On Mon, Oct 05, 2009 at 04:19:55PM +0530, K.Prasad wrote:
> On Mon, Oct 05, 2009 at 07:23:06PM +0900, Paul Mundt wrote:
> > [ I noticed this whilst hacking up SH support, applies to the current
> > tracing/hw-breakpoints topic branch. ]
> >
> > flush_thread() tries to do a TIF_DEBUG check before calling in to
> > flush_thread_hw_breakpoint() (which subsequently clears the thread flag),
> > but for some reason, the x86 code is manually clearing TIF_DEBUG
> > immediately before the test, so this path will never be taken.
> >
> > This kills off the erroneous clear_tsk_thread_flag() and lets
> > flush_thread_hw_breakpoint() actually get invoked.
> >
>
> Yes, this is wrong and we missed it. Thanks for fixing!
>
> > Presumably folks were getting lucky with testing and the
> > free_thread_info() -> free_thread_xstate() path was taking care of the
> > flush there.
> >
> > Signed-off-by: Paul Mundt <[email protected]>
>
> Acked-by: K.Prasad <[email protected]>
>
>
Ingo, are you planning on picking this up? Or is there a hw-breakpoints
tree somewhere outside of -tip that I'm unaware of?
On Fri, Oct 09, 2009 at 12:40:04PM +0900, Paul Mundt wrote:
> On Mon, Oct 05, 2009 at 04:19:55PM +0530, K.Prasad wrote:
> > On Mon, Oct 05, 2009 at 07:23:06PM +0900, Paul Mundt wrote:
> > > [ I noticed this whilst hacking up SH support, applies to the current
> > > tracing/hw-breakpoints topic branch. ]
> > >
> > > flush_thread() tries to do a TIF_DEBUG check before calling in to
> > > flush_thread_hw_breakpoint() (which subsequently clears the thread flag),
> > > but for some reason, the x86 code is manually clearing TIF_DEBUG
> > > immediately before the test, so this path will never be taken.
> > >
> > > This kills off the erroneous clear_tsk_thread_flag() and lets
> > > flush_thread_hw_breakpoint() actually get invoked.
> > >
> >
> > Yes, this is wrong and we missed it. Thanks for fixing!
> >
> > > Presumably folks were getting lucky with testing and the
> > > free_thread_info() -> free_thread_xstate() path was taking care of the
> > > flush there.
> > >
> > > Signed-off-by: Paul Mundt <[email protected]>
> >
> > Acked-by: K.Prasad <[email protected]>
> >
> >
> Ingo, are you planning on picking this up? Or is there a hw-breakpoints
> tree somewhere outside of -tip that I'm unaware of?
Joining: Acked-by: Frederic Weisbecker <[email protected]>
Even if a rewrite on top of perf event is planned, this is still a fix
on the current state. Something that the rewrite won't need to handle.
Thanks.
On Fri, Oct 09, 2009 at 12:40:04PM +0900, Paul Mundt wrote:
> On Mon, Oct 05, 2009 at 04:19:55PM +0530, K.Prasad wrote:
> > On Mon, Oct 05, 2009 at 07:23:06PM +0900, Paul Mundt wrote:
> > > [ I noticed this whilst hacking up SH support, applies to the current
> > > tracing/hw-breakpoints topic branch. ]
> > >
> > > flush_thread() tries to do a TIF_DEBUG check before calling in to
> > > flush_thread_hw_breakpoint() (which subsequently clears the thread flag),
> > > but for some reason, the x86 code is manually clearing TIF_DEBUG
> > > immediately before the test, so this path will never be taken.
> > >
> > > This kills off the erroneous clear_tsk_thread_flag() and lets
> > > flush_thread_hw_breakpoint() actually get invoked.
> > >
> >
> > Yes, this is wrong and we missed it. Thanks for fixing!
> >
> > > Presumably folks were getting lucky with testing and the
> > > free_thread_info() -> free_thread_xstate() path was taking care of the
> > > flush there.
> > >
> > > Signed-off-by: Paul Mundt <[email protected]>
> >
> > Acked-by: K.Prasad <[email protected]>
> >
> >
> Ingo, are you planning on picking this up? Or is there a hw-breakpoints
> tree somewhere outside of -tip that I'm unaware of?
I'm picking it.
Thanks.
On Sun, Oct 18, 2009 at 01:42:53AM +0200, Frederic Weisbecker wrote:
> On Fri, Oct 09, 2009 at 12:40:04PM +0900, Paul Mundt wrote:
> > On Mon, Oct 05, 2009 at 04:19:55PM +0530, K.Prasad wrote:
> > > On Mon, Oct 05, 2009 at 07:23:06PM +0900, Paul Mundt wrote:
> > > > [ I noticed this whilst hacking up SH support, applies to the current
> > > > tracing/hw-breakpoints topic branch. ]
> > > >
> > > > flush_thread() tries to do a TIF_DEBUG check before calling in to
> > > > flush_thread_hw_breakpoint() (which subsequently clears the thread flag),
> > > > but for some reason, the x86 code is manually clearing TIF_DEBUG
> > > > immediately before the test, so this path will never be taken.
> > > >
> > > > This kills off the erroneous clear_tsk_thread_flag() and lets
> > > > flush_thread_hw_breakpoint() actually get invoked.
> > > >
> > >
> > > Yes, this is wrong and we missed it. Thanks for fixing!
> > >
> > > > Presumably folks were getting lucky with testing and the
> > > > free_thread_info() -> free_thread_xstate() path was taking care of the
> > > > flush there.
> > > >
> > > > Signed-off-by: Paul Mundt <[email protected]>
> > >
> > > Acked-by: K.Prasad <[email protected]>
> > >
> > >
> > Ingo, are you planning on picking this up? Or is there a hw-breakpoints
> > tree somewhere outside of -tip that I'm unaware of?
>
>
> I'm picking it.
> Thanks.
>
Thanks. If you're setting up a tree somewhere can you let me know where
it will be hosted? I'm pretty much done with the SH support for the
existing topic branch in -tip, but if it's being reworked, then I'd like
to get my changes reworked and integrated as soon as possible.
On Sun, Oct 18, 2009 at 03:25:58PM +0900, Paul Mundt wrote:
> On Sun, Oct 18, 2009 at 01:42:53AM +0200, Frederic Weisbecker wrote:
> > On Fri, Oct 09, 2009 at 12:40:04PM +0900, Paul Mundt wrote:
> > > On Mon, Oct 05, 2009 at 04:19:55PM +0530, K.Prasad wrote:
> > > > On Mon, Oct 05, 2009 at 07:23:06PM +0900, Paul Mundt wrote:
> > > > > [ I noticed this whilst hacking up SH support, applies to the current
> > > > > tracing/hw-breakpoints topic branch. ]
> > > > >
> > > > > flush_thread() tries to do a TIF_DEBUG check before calling in to
> > > > > flush_thread_hw_breakpoint() (which subsequently clears the thread flag),
> > > > > but for some reason, the x86 code is manually clearing TIF_DEBUG
> > > > > immediately before the test, so this path will never be taken.
> > > > >
> > > > > This kills off the erroneous clear_tsk_thread_flag() and lets
> > > > > flush_thread_hw_breakpoint() actually get invoked.
> > > > >
> > > >
> > > > Yes, this is wrong and we missed it. Thanks for fixing!
> > > >
> > > > > Presumably folks were getting lucky with testing and the
> > > > > free_thread_info() -> free_thread_xstate() path was taking care of the
> > > > > flush there.
> > > > >
> > > > > Signed-off-by: Paul Mundt <[email protected]>
> > > >
> > > > Acked-by: K.Prasad <[email protected]>
> > > >
> > > >
> > > Ingo, are you planning on picking this up? Or is there a hw-breakpoints
> > > tree somewhere outside of -tip that I'm unaware of?
> >
> >
> > I'm picking it.
> > Thanks.
> >
> Thanks. If you're setting up a tree somewhere can you let me know where
> it will be hosted? I'm pretty much done with the SH support for the
> existing topic branch in -tip, but if it's being reworked, then I'd like
> to get my changes reworked and integrated as soon as possible.
I'm working on the v2 of the hardware breakpoints through perf integration.
I've included your patch inside, I'll Cc you on the patches.
On Sun, Oct 18, 2009 at 03:25:58PM +0900, Paul Mundt wrote:
> Thanks. If you're setting up a tree somewhere can you let me know where
> it will be hosted? I'm pretty much done with the SH support for the
> existing topic branch in -tip, but if it's being reworked, then I'd like
> to get my changes reworked and integrated as soon as possible.
Oh you have based SH breakpoint support on top of the current breakpoint
API in -tip?
Hm, you should have talked to us about that since it's being reworked.
I'll probably post soon the rebase on top of perf events. And even if
it gets merged in -tip, it should still be considered as very unstable.
May be should you better wait for it to get upstream?
On Mon, Oct 19, 2009 at 09:48:45PM +0200, Frederic Weisbecker wrote:
> On Sun, Oct 18, 2009 at 03:25:58PM +0900, Paul Mundt wrote:
> > Thanks. If you're setting up a tree somewhere can you let me know where
> > it will be hosted? I'm pretty much done with the SH support for the
> > existing topic branch in -tip, but if it's being reworked, then I'd like
> > to get my changes reworked and integrated as soon as possible.
>
>
> Oh you have based SH breakpoint support on top of the current breakpoint
> API in -tip?
>
> Hm, you should have talked to us about that since it's being reworked.
>
Strange, I thought that's what I was doing ;-)
> I'll probably post soon the rebase on top of perf events. And even if
> it gets merged in -tip, it should still be considered as very unstable.
>
> May be should you better wait for it to get upstream?
>
I wanted to get the initial support out of the way early to see if there
was anything in the generic code that needed work, but apart from some
minor stuff it's been kept fairly well isolated. The SH support I have
works fine, but isn't a lot of code, and can trivially be reworked on top
of whatever the new interface looks like. I've left out things like
ptrace and kgdb stub integration precisely because I knew the code was
being reworked.
Given that, I'm not in any particular rush, so whenever the reworked
patches are available, I'll give them a run and rebase accordingly.
On Tue, Oct 20, 2009 at 08:24:32AM +0900, Paul Mundt wrote:
> > Oh you have based SH breakpoint support on top of the current breakpoint
> > API in -tip?
> >
> > Hm, you should have talked to us about that since it's being reworked.
> >
> Strange, I thought that's what I was doing ;-)
[...]
> I wanted to get the initial support out of the way early to see if there
> was anything in the generic code that needed work, but apart from some
> minor stuff it's been kept fairly well isolated. The SH support I have
> works fine, but isn't a lot of code, and can trivially be reworked on top
> of whatever the new interface looks like. I've left out things like
> ptrace and kgdb stub integration precisely because I knew the code was
> being reworked.
>
> Given that, I'm not in any particular rush, so whenever the reworked
> patches are available, I'll give them a run and rebase accordingly.
Ah ok. I was just worrying you thought the current state is stable.
Ok, no problem then, Thanks. :)
Commit-ID: 41a48d14f6991020c9bb6b93e289ca5b411ed09a
Gitweb: http://git.kernel.org/tip/41a48d14f6991020c9bb6b93e289ca5b411ed09a
Author: Paul Mundt <[email protected]>
AuthorDate: Mon, 5 Oct 2009 19:23:06 +0900
Committer: Frederic Weisbecker <[email protected]>
CommitDate: Tue, 3 Nov 2009 18:05:44 +0100
x86/hw-breakpoints: Actually flush thread breakpoints in flush_thread().
flush_thread() tries to do a TIF_DEBUG check before calling in to
flush_thread_hw_breakpoint() (which subsequently clears the thread flag),
but for some reason, the x86 code is manually clearing TIF_DEBUG
immediately before the test, so this path will never be taken.
This kills off the erroneous clear_tsk_thread_flag() and lets
flush_thread_hw_breakpoint() actually get invoked.
Presumably folks were getting lucky with testing and the
free_thread_info() -> free_thread_xstate() path was taking care of the
flush there.
Signed-off-by: Paul Mundt <[email protected]>
Acked-by: "K.Prasad" <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Alan Stern <[email protected]>
LKML-Reference: <[email protected]>
Signed-off-by: Frederic Weisbecker <[email protected]>
---
arch/x86/kernel/process.c | 2 --
1 files changed, 0 insertions(+), 2 deletions(-)
diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
index 2275ce5..cf8ee00 100644
--- a/arch/x86/kernel/process.c
+++ b/arch/x86/kernel/process.c
@@ -107,8 +107,6 @@ void flush_thread(void)
}
#endif
- clear_tsk_thread_flag(tsk, TIF_DEBUG);
-
if (unlikely(test_tsk_thread_flag(tsk, TIF_DEBUG)))
flush_thread_hw_breakpoint(tsk);
memset(tsk->thread.tls_array, 0, sizeof(tsk->thread.tls_array));