From: Roland McGrath <roland@redhat.com>
To: Ingo Molnar <mingo@elte.hu>
Cc: prasad@linux.vnet.ibm.com,
Andrew Morton <akpm@linux-foundation.org>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
Alan Stern <stern@rowland.harvard.edu>
Subject: Re: [patch 04/11] Introduce virtual debug register in thread_struct and wrapper-routines around process related functions
Date: Wed, 11 Mar 2009 19:26:50 -0700 (PDT) [thread overview]
Message-ID: <20090312022650.C00DDFC3B6@magilla.sf.frob.com> (raw)
In-Reply-To: Ingo Molnar's message of Tuesday, 10 March 2009 15:35:43 +0100 <20090310143543.GE3850@elte.hu>
> detached from thread_struct? There's a lot of complications
> (alloc/free, locking, etc.) from this for no good reason - the
> hardware-breakpoints info structure is alway per thread and is
> quite small, so there's no reason not to embedd it directly
> inside thread_struct.
I do certainly think it's worthwhile to use a coherent struct type here
rather than many fields in thread_struct, independent of the allocation for
it being direct or indirect (not that you objected to that). That makes
the code read cleaner, and should make it a minor change to most of the
code later if the allocation plan changes.
I think in the original effort another motivating factor was concern about
bloating the size of thread_struct. The struct thread_hw_breakpoint is at
least a few times the size of the set old fields it replaces. We were
concerned that inflating every task's thread_struct for the benefit of
these rarely-used fancy new features might meet resistance from arch
maintainers like you. If that issue doesn't hold you back from taking the
new code, then I think we are more than happy to start with thread_struct
directly containing a struct thread_hw_breakpoint member.
I do think we'll want to make it a pointer with later incremental changes.
(But those may not need to come very soon.) Firstly, the size reduction to
task_struct is fairly compelling since it's for the vast majority of tasks
which never need to allocate it. The hair potential is really not very
much at least to begin with, if you just make it allocate on first setup
and never free (no locking et al, just if (thread->hwbkpt) ...). Second,
eventually we'd like to have the possibility of sharing the struct among
threads. This will come later on when we have higher-level things that
would like to set common watchpoints on a whole group of threads (what
debuggers usually really do). Such APIs are far improved and optimized by
updating many threads together, and when a big app has thousands of threads
to which all the same watchpoints apply, sharing at low level makes many of
those intraprocess context switches quicker. As I said, all in the future.
But it's far from being entirely baseless to think a pointer makes good sense.
Thanks,
Roland
next prev parent reply other threads:[~2009-03-12 2:28 UTC|newest]
Thread overview: 69+ 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
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 [this message]
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:06 ` [Patch 04/11] Introduce virtual debug register in thread_struct and wrapper-routines around process related functions 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=20090312022650.C00DDFC3B6@magilla.sf.frob.com \
--to=roland@redhat.com \
--cc=akpm@linux-foundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@elte.hu \
--cc=prasad@linux.vnet.ibm.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
all inboxes | Powered by JetHome®