public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: "K.Prasad" <prasad@linux.vnet.ibm.com>
To: Ingo Molnar <mingo@elte.hu>
Cc: Alan Stern <stern@rowland.harvard.edu>,
	Andrew Morton <akpm@linux-foundation.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Roland McGrath <roland@redhat.com>
Subject: Re: [patch 02/11] x86 architecture implementation of Hardware Breakpoint interfaces
Date: Wed, 11 Mar 2009 18:20:13 +0530	[thread overview]
Message-ID: <20090311125013.GA9547@in.ibm.com> (raw)
In-Reply-To: <20090311121220.GI2282@elte.hu>

On Wed, Mar 11, 2009 at 01:12:20PM +0100, Ingo Molnar wrote:
> 
> * Alan Stern <stern@rowland.harvard.edu> wrote:
> 
> > On Tue, 10 Mar 2009, Ingo Molnar wrote:
> > 
> > > > More generally, it's there because kernel & userspace 
> > > > breakpoints can be installed and uninstalled while a task is 
> > > > running -- and yes, this is partially because breakpoints are 
> > > > prioritized.  (Although it's worth pointing out that even your 
> > > > suggestion of always prioritizing kernel breakpoints above 
> > > > userspace breakpoints would have the same effect.)  However 
> > > > the fact that the breakpoints are stored in a list rather than 
> > > > an array doesn't seem to be relevant.
> > > > 
> > > > > A list needs to be maintained and when updated it's 
> > > > > reloaded.
> > > > 
> > > > The same is true of an array.
> > > 
> > > Not if what we do what the previous code did: reloaded the full 
> > > array unconditionally. (it's just 4 entries)
> > 
> > But that array still has to be set up somehow.  It is private 
> > to the task; the only logical place to set it up is when the 
> > CPU switches to that task.
> > 
> > In the old code, it wasn't possible for task B or the kernel 
> > to affect the contents of task A's debug registers.  With 
> > hw-breakpoints it _is_ possible, because the balance between 
> > debug registers allocated to kernel breakpoints and debug 
> > registers allocated to userspace breakpoints can change.  
> > That's why the additional complexity is needed.
> 
> Yes - but we dont really need any scheduler complexity for this.
> 
> An IPI is enough to reload debug registers in an affected task 
> (and calculate the real debug register layout) - and the next 
> context switches will pick up changes automatically.
> 
> Am i missing anything? I'm trying to find the design that has 
> the minimal possible complexity. (without killing any necessary 
> features)
> 
> > > > Yes, kernel breakpoints have to be kept separate from 
> > > > userspace breakpoints.  But even if you focus just on 
> > > > userspace breakpoints, you still need to use a list 
> > > > because debuggers can try to register an arbitrarily large 
> > > > number of breakpoints.
> > > 
> > > That 'arbitrarily large number of breakpoints' worries me. 
> > > It's a pretty broken concept for a 4-items resource that 
> > > cannot be time-shared and hence cannot be overcommitted.
> > 
> > Suppose we never allow callers to register more breakpoints 
> > than will fit in the CPU's registers.  Do we then use a simple 
> > first-come first-served algorithm, with no prioritization?  If 
> > we do prioritize some breakpoint registrations more highly 
> > than others, how do we inform callers that their breakpoint 
> > has been kicked out by one of higher priority?  And how do we 
> > let them know when the higher-priority breakpoint has been 
> > unregistered, so they can try again?
> 
> For an un-shareable resource like this (and this is really a 
> rare case [and we shouldnt even consider switching between user 
> and kernel debug registers at system call time]), the best 
> approach is to have a rigid reservation mechanism with clear, 
> hard, early failures in the overcommit case.
> 
> Silently breaking a user-space debugging sessions just because 
> the admin has a debug register based system-wide profiling 
> running, is pretty much the worst usage model. It does not give 
> user-space any idea about what happened - the breakpoints just 
> "dont work".
> 
> So i'd suggest a really simple scheme (depicted for x86 bug 
> applicable on other architectures too):
> 
>  - we have a system-wide resource of 4 debug registers.
> 
>  - kernel-side can allocate debug registers system-wide (it 
>    takes effect on all CPUs, at once), up to 4 of them. The 5th 
>    allocation will fail.
> 
>  - user-side uses the ptrace APIs - and if it runs into the 
>    limit, ptrace should return a failure.
> 
> There's the following special case: the kernel reserves a debug 
> register when there's tasks in the system that already have 
> reserved all debug registers. I.e. the constraint was not known 
> when the user-space session started, and the kernel violates it 
> afterwards.
> 
> There's a couple of choices here, with various scales of 
> conflict resolution:
> 
>  1- silently override the user-space breakpoint
> 
>  2- notify the user-space task via a signal - SIGXCPU or so.
> 
>  3- reject the kernel-space allocation with a sufficiently 
>     informative log message: "task 123 already uses 4 debug 
>     registers, cannot allocate more kernel breakpoints" - 
>     leaving the resolution of the conflict to the admin.
> 
> #1 isnt particularly good because it brings back a
>    'silentfailure' mode.
> 
> #2 might be too brutal: starting something innocous-looking
>    might kill a debug session. OTOH user-space debuggers could 
>    catch the signal and inform the user.
> 
> #3 is probably the most informative (and hence probably the
>    best) variant. It also leaves policy of how to resolve the 
>    conflict to the admin.
> 

While reserving more discussions after Roland posts his views, I thought
I'd share some of mine here.

The present implementation can be likened to #3 except that the
uninstalled() callback is invoked (the user-space call through ptrace
takes a higher priority and evicts the kernel-space requests even now).

After the task using four debug registers yield the CPU, the
kernel-space breakpoint requests are 'restored' and installed() is
called again.

Even if #3 was implemented as described, we would still retain a
majority of the complexity in balance_kernel_vs_user() to check newer
tasks with requests for breakpoint registers.

Thanks,
K.Prasad


  reply	other threads:[~2009-03-11 12:50 UTC|newest]

Thread overview: 71+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20090305043440.189041194@linux.vnet.ibm.com>
2009-03-05  4:37 ` [patch 01/11] Introducing generic hardware breakpoint handler interfaces prasad
2009-03-10 13:50   ` Ingo Molnar
2009-03-10 14:19     ` Alan Stern
2009-03-10 14:50       ` Ingo Molnar
2009-03-11 12:57         ` K.Prasad
2009-03-11 13:35           ` Ingo Molnar
2009-03-05  4:38 ` [patch 02/11] x86 architecture implementation of Hardware Breakpoint interfaces prasad
2009-03-10 14:09   ` Ingo Molnar
2009-03-10 14:59     ` Alan Stern
2009-03-10 15:18       ` Ingo Molnar
2009-03-10 17:11         ` Alan Stern
2009-03-10 17:26           ` Ingo Molnar
2009-03-10 20:30             ` Alan Stern
2009-03-11 12:12               ` Ingo Molnar
2009-03-11 12:50                 ` K.Prasad [this message]
2009-03-11 13:10                   ` Ingo Molnar
2009-03-14  3:46                     ` Benjamin Herrenschmidt
2009-03-11 16:39                   ` Alan Stern
2009-03-11 16:32                 ` Alan Stern
2009-03-11 17:41                   ` K.Prasad
2009-03-14  3:47                     ` Benjamin Herrenschmidt
2009-03-14  3:43                 ` Benjamin Herrenschmidt
2009-03-14  3:41               ` Benjamin Herrenschmidt
2009-03-14  3:40             ` Benjamin Herrenschmidt
2009-03-12  2:46     ` Roland McGrath
2009-03-13  3:43       ` Ingo Molnar
2009-03-13 14:04         ` Alan Stern
2009-03-13 14:13           ` Ingo Molnar
2009-03-13 19:01             ` K.Prasad
2009-03-13 21:21               ` Alan Stern
2009-03-14 12:24                 ` Ingo Molnar
2009-03-14 16:10                   ` Alan Stern
2009-03-14 16:39                     ` Ingo Molnar
2009-03-14  3:51       ` Benjamin Herrenschmidt
2009-03-05  4:38 ` [patch 03/11] Modifying generic debug exception to use virtual debug registers prasad
2009-03-05  4:38 ` [patch 04/11] Introduce virtual debug register in thread_struct and wrapper-routines around process related functions prasad
2009-03-10 14:35   ` Ingo Molnar
2009-03-10 15:53     ` Alan Stern
2009-03-10 17:06       ` Ingo Molnar
2009-03-12  2:26     ` Roland McGrath
2009-03-05  4:38 ` [patch 05/11] Use wrapper routines around debug registers in processor " prasad
2009-03-05  4:40 ` [patch 06/11] Use virtual debug registers in process/thread handling code prasad
2009-03-10 14:49   ` Ingo Molnar
2009-03-10 16:05     ` Alan Stern
2009-03-10 16:58       ` Ingo Molnar
2009-03-10 17:07       ` Ingo Molnar
2009-03-10 20:10         ` Alan Stern
2009-03-11 11:53           ` Ingo Molnar
2009-03-05  4:40 ` [patch 07/11] Modify signal handling code to refrain from re-enabling HW Breakpoints prasad
2009-03-05  4:40 ` [patch 08/11] Modify Ptrace routines to access breakpoint registers prasad
2009-03-10 14:40   ` Ingo Molnar
2009-03-10 15:54     ` Alan Stern
2009-03-12  3:14     ` Roland McGrath
2009-03-05  4:41 ` [patch 09/11] Cleanup HW Breakpoint registers before kexec prasad
2009-03-10 14:42   ` Ingo Molnar
2009-03-05  4:41 ` [patch 10/11] Sample HW breakpoint over kernel data address prasad
2009-03-05  4:43 ` prasad
2009-03-05  4:43 ` [patch 11/11] ftrace plugin for kernel symbol tracing using HW Breakpoint interfaces prasad
2009-03-05  6:37   ` Frederic Weisbecker
2009-03-05  9:16     ` Ingo Molnar
2009-03-05 13:15       ` K.Prasad
2009-03-05 13:28         ` Ingo Molnar
2009-03-05 11:33     ` K.Prasad
2009-03-05 12:19       ` K.Prasad
2009-03-05 12:30         ` Frederic Weisbecker
2009-03-05 12:28       ` Frederic Weisbecker
2009-03-05 15:00     ` Steven Rostedt
2009-03-05 14:54   ` Steven Rostedt
     [not found] <20090307045120.039324630@linux.vnet.ibm.com>
2009-03-07  5:05 ` [Patch 02/11] x86 architecture implementation of Hardware " prasad
     [not found] <20090319234044.410725944@K.Prasad>
2009-03-19 23:48 ` K.Prasad
     [not found] <20090324152028.754123712@K.Prasad>
2009-03-24 15:25 ` K.Prasad

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20090311125013.GA9547@in.ibm.com \
    --to=prasad@linux.vnet.ibm.com \
    --cc=akpm@linux-foundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=roland@redhat.com \
    --cc=stern@rowland.harvard.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox