LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Alan Stern <stern@rowland.harvard.edu>
To: Christoph Hellwig <hch@infradead.org>
Cc: Roland McGrath <roland@redhat.com>,
	Prasanna S Panchamukhi <prasanna@in.ibm.com>,
	Kernel development list <linux-kernel@vger.kernel.org>
Subject: Re: [RFC] hwbkpt: Hardware breakpoints (was Kwatch)
Date: Mon, 5 Mar 2007 11:16:48 -0500 (EST)	[thread overview]
Message-ID: <Pine.LNX.4.44L0.0703051113350.3401-100000@iolanthe.rowland.org> (raw)
In-Reply-To: <20070305133639.GA6490@infradead.org>

On Mon, 5 Mar 2007, Christoph Hellwig wrote:

> On Sun, Mar 04, 2007 at 11:01:36PM -0800, Roland McGrath wrote:
> > > 	The parts relating to kernel breakpoints could be made conditional
> > > 	on a Kconfig option.  The amount of code space saved would be
> > > 	relatively small; I'm not sure that it would be worthwhile.
> > 
> > In a utrace merge, the user parts can be made conditional on CONFIG_UTRACE.
> > Then with both turned off, the code goes away completely.  It's unlikely it
> > will ever be turned off, but it is a clean way to go about things in case
> > someone wants the smallest possible config for a limited-use installation.
> 
> Making this unconditional is pointless and just makes things harder to
> read, so please don't do it.  (The same is true for utrace, but Roland
> has unfortunately still not replied to my mail mentioning it :P)

Sorry, I don't understand what you're saying.  I would think that making
it _conditional_ would make things harder to read, because of all the
extra "#ifdef" and "#endif" lines plus the need to keep two different
versions of the code in mind.

Did you mean to say "conditional" instead of "unconditional"?

Incidentally, I do believe that for certain applications (embedded
devices, for instance) it makes sense to avoid including all this code.  
The cleanest way to do that would be to make both PTRACE and UTRACE
configurable.


> > > +	void		(*installed)(struct hwbkpt *);
> > > +	void		(*uninstalled)(struct hwbkpt *);
> > 
> > Save space in the struct by having just one function for both installed
> > and uninstalled, taking an argument.  Probably a caller should be able to
> > pass a null function here to say that the registration call should fail if
> > it can't be installed due to higher-priority or no-callback registrations
> > existing, and that its registration cannot be ejected by another (i.e., an
> > ill-behaved user).
> 
> Please not.  That might save a few bytes, but it makes the interface a
> lot harder to understand for users.  We really discourage over-loaded
> interfaces in Linux.

I agree with Christoph.  Plenty of other interfaces in the kernel do the 
same thing.


> > > +struct thread_hwbkpt {		/* HW breakpoint info for a thread */
> 
> Can this and all the file names please get an actually readable name?
> E.g. hw_breakpoint.  We're not IBM managers that needs to save every
> cent on superflous sillables :)

I'll be happy to rename the structures and the files if Roland doesn't 
mind.

Alan Stern


  reply	other threads:[~2007-03-05 16:16 UTC|newest]

Thread overview: 60+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20070207025008.1B11118005D@magilla.sf.frob.com>
2007-02-07 19:22 ` [PATCH] Kwatch: kernel watchpoints using CPU debug registers Alan Stern
2007-02-07 22:08   ` Bob Copeland
2007-02-09 10:21   ` Roland McGrath
2007-02-09 15:54     ` Alan Stern
2007-02-09 23:31       ` Roland McGrath
2007-02-10  4:32         ` Alan Stern
2007-02-18  3:03           ` Roland McGrath
2007-02-21 20:35         ` Alan Stern
2007-02-22 11:43           ` S. P. Prasanna
2007-02-23  2:19           ` Roland McGrath
2007-02-23 16:55             ` Alan Stern
2007-02-24  0:08               ` Roland McGrath
2007-03-02 17:19                 ` [RFC] hwbkpt: Hardware breakpoints (was Kwatch) Alan Stern
2007-03-05  7:01                   ` Roland McGrath
2007-03-05 13:36                     ` Christoph Hellwig
2007-03-05 16:16                       ` Alan Stern [this message]
2007-03-05 16:49                         ` Christoph Hellwig
2007-03-05 22:04                         ` Roland McGrath
2007-03-05 17:25                     ` Alan Stern
2007-03-06  3:13                       ` Roland McGrath
2007-03-06 15:23                         ` Alan Stern
2007-03-07  3:49                           ` Roland McGrath
2007-03-07 19:11                             ` Alan Stern
2007-03-09  6:52                               ` Roland McGrath
2007-03-09 18:40                                 ` Alan Stern
2007-03-13  8:00                                   ` Roland McGrath
2007-03-13 13:07                                     ` Alan Cox
2007-03-13 18:56                                     ` Alan Stern
2007-03-14  3:00                                       ` Roland McGrath
2007-03-14 19:11                                         ` Alan Stern
2007-03-28 21:39                                           ` Roland McGrath
2007-03-29 21:35                                             ` Alan Stern
2007-04-13 21:09                                             ` Alan Stern
2007-05-11 15:25                                             ` Alan Stern
2007-05-13 10:39                                               ` Roland McGrath
2007-05-14 15:42                                                 ` Alan Stern
2007-05-14 21:25                                                   ` Roland McGrath
2007-05-16 19:03                                                     ` Alan Stern
2007-05-23  8:47                                                       ` Roland McGrath
2007-06-01 19:39                                                         ` Alan Stern
2007-06-14  6:48                                                           ` Roland McGrath
2007-06-19 20:35                                                             ` Alan Stern
2007-06-25 10:52                                                               ` Roland McGrath
2007-06-25 15:36                                                                 ` Alan Stern
2007-06-26 20:49                                                                   ` Roland McGrath
2007-06-27  3:26                                                                     ` Alan Stern
2007-06-27 21:04                                                                       ` Roland McGrath
2007-06-29  3:00                                                                         ` Alan Stern
2007-07-11  6:59                                                                           ` Roland McGrath
2007-06-28  3:02                                                                       ` Roland McGrath
2007-06-25 11:32                                                               ` Roland McGrath
2007-06-25 15:37                                                                 ` Alan Stern
2007-06-25 20:51                                                                 ` Alan Stern
2007-06-26 18:17                                                                   ` Roland McGrath
2007-06-27  2:43                                                                     ` Alan Stern
2007-05-17 20:39                                                 ` Alan Stern
2007-03-16 21:07                                         ` Alan Stern
2007-03-22 19:44                                         ` Alan Stern
     [not found] <20070628023100.E46AB4D05E6@magilla.localdomain>
2007-06-29  3:36 ` Alan Stern
2007-07-06 20:48 ` Alan Stern

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=Pine.LNX.4.44L0.0703051113350.3401-100000@iolanthe.rowland.org \
    --to=stern@rowland.harvard.edu \
    --cc=hch@infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=prasanna@in.ibm.com \
    --cc=roland@redhat.com \
    --subject='Re: [RFC] hwbkpt: Hardware breakpoints (was Kwatch)' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).