Re: [RFC][Patch 1/4] kprobe fast unregistration

[fche@redhat.com (Frank Ch. Eigler)]

Hi -

"Keshavamurthy, Anil S" <anil.s.keshavamurthy@intel.com> writes:

> [...]
> > Really?  What possible problems can occur?  The worst that occurs
> > to me is that if someone forgets to call the commit function, the
> > kprobes will still be disabled, but memory won't be recycled for a
> > while.  [...]
>
> Yes, Have you looked at the code? 

A little, but we were talking more about the interface than the
implementation.

> If someone forgets to call the commit function, the kprobe will be
> disabled and yes the memory won't be recycled but the worst problem
> is that if the probe is on a module function then that module can't
> be unloaded at all [...]

I believe there is already a kprobes patch in the queue for
enumerating active probes in some /proc file.  Should a module be
locked into memory for such a reason, finding the culprit should not
be difficult.

> Hence, my suggestion would be to call them as disable_kprobe()
> (instead of unregister_kprobes_fast() which is confusing and error
> prone) and also to provide an opposite function to reenable_kprobe()
> and finally provide unregister_disabled_kprobes() which is
> essentially the same as commit_kprobes().

One problem with this idea is that if the unregister_fast()=disable()
is to become reversible, then the renamed commit_kprobes() will no
longer be indempotent.  There can no longer be a single system-wide
deferred-kprobe-cleanup list, since individual kprobes clients might
want to reinstate their probes in the future.

> > Would it be possible to allay even that concern with an automated
> > deferred/periodic commit?
> > 
> I would recomand that users call unregister_disabled_kprobes() explictly.

But this would solve both problems (memory leaks and outstanding
reference counts on modules).  In this variant,
unregister_kprobes_fast could replace unregister_kprobes outright, and
the (builtin deferred) commit function would need not be exported.

- FChE