[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20091230222837.GI6322@nowhere>
Date: Wed, 30 Dec 2009 23:28:39 +0100
From: Frederic Weisbecker <fweisbec@...il.com>
To: "K.Prasad" <prasad@...ux.vnet.ibm.com>
Cc: Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
"mingo@...e.hu" <mingo@...e.hu>,
Peter Zijlstra <peterz@...radead.org>
Subject: Re: [Patch 1/1] Introduce register_user_hbp_by_pid() and
unregister_user_hbp_by_pid()
On Tue, Dec 22, 2009 at 12:16:31AM +0530, K.Prasad wrote:
> On Fri, Dec 18, 2009 at 09:47:48PM +0100, Frederic Weisbecker wrote:
> > > +
> > > + do {
> > > + mutex_lock(&start->perf_event_mutex);
> > > + list_for_each_entry_safe(bp, temp_bp, &start->perf_event_list,
> > > + owner_entry) {
> > > + if (bp->attr.type != PERF_TYPE_BREAKPOINT)
> > > + continue;
> > > + unregister_hw_breakpoint(bp);
> > > + break;
> >
> > Do you really want to unregister all of them? What about those
> > that may have been registered using perf syscall?
> >
>
> Seems that I got influenced heavily by the PPC64 design (was coding
> simultaneously for it :-)...will accept a "struct perf_event_attr *" and
> use that to identify appropriate breakpoint.
:)
> > And this function needs rcu too.
> >
> > I don't see any in-kernel user for this new feature.
> > That would be required to integrate it.
> >
>
> The proposed interfaces, as obvious, are mere wrappers over existing
> (un)register_user_* interfaces, and don't do anything vastly different
> in order to demonstrate them separately.
>
> I can get a sample kernel module ready - that consumes pid and user-space
> address to track write accesses, if you prefer it.
Ok. The code looks good and useful.
But the usual philosophy in the kernel is to not add code
that is left unused upstream. And samples don't substitute a user.
I'm not sure this is a good idea to merge this.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists