lkml.org 
[lkml]   [2013]   [Nov]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] uprobes: Cleanup !CONFIG_UPROBES decls, unexport xol_area

* Oleg Nesterov <oleg@redhat.com> wrote:

> On 11/11, Ingo Molnar wrote:
> >
> > * Oleg Nesterov <oleg@redhat.com> wrote:
> >
> > > +++ b/kernel/events/uprobes.c
> > > @@ -86,6 +86,25 @@ struct return_instance {
> > > };
> > >
> > > /*
> > > + * On a breakpoint hit, thread contests for a slot. It frees the
> > > + * slot after singlestep. Currently a fixed number of slots are
> > > + * allocated.
> > > + */
> > > +struct xol_area {
> >
> > So, my main complaint about the uprobes code isn't functional but
> > documentational, similar to what I outlined a few days ago: what this
> > comment does not explain is exactly what a 'XOL area' is.
> >
> > You guys are changing code that reads like gobbledygook to people
> > reading it for the first time.
>
> Not that I am trying to defense uprobes, but this is equally true for
> any piece of kernel code, at least to me ;)

I'm really not suggesting to do overly much - only for some minimal blurb
like the scheduler has in most places:

/*
* This is the main, per-CPU runqueue data structure.
*
* Locking rule: those places that want to lock multiple runqueues
* (such as the load balancing or the thread migration code), lock
* acquire operations must be ordered by ascending &runqueue.
*/
struct rq {
/* runqueue lock: */
raw_spinlock_t lock;

But the apparent assumption that the reader knows what 'XOL' means
triggered my suggest :-)

> > Maybe even split the XOL code out into kernel/events/uprobes_xol.c or
> > so?
>
> I do not really think a separate uprobes_xol.c makes sense. [...]

Ok - it's your call really.

> [...] I think it would be nice to have the high-level "uprobes design"
> doc in uprobetracer.txt, or

Even better if the best parts are integrated into the source code!

Thanks,

Ingo


\
 
 \ /
  Last update: 2013-11-11 22:21    [W:0.398 / U:0.380 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site