All of lore.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.