* Oleg Nesterov <o...@redhat.com> wrote:
> On 11/11, Ingo Molnar wrote:
> >
> > * Oleg Nesterov <o...@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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
