Re: [PATCH] Kprobes- robust fault handling for i386

[Keshavamurthy Anil S <anil dot s dot keshavamurthy at intel dot com>]

On Mon, Feb 27, 2006 at 02:55:35PM +0530, Prasanna S Panchamukhi wrote:
> Anil,
> 
> 
> For your convenience I have splitup the patches, please find
> then below.
Thanks for the splitting.
> In general splitting of patches is a good idea, but here I think
> splitting does not make much difference, since post_handler changes
> are only few lines.  Correct me if I am wrong.
Since you are introducing lots of kprobes states it is good 
to split the patches according the pre/post/ss handling as
the reviewer can understand why each kprobes state is needed.
Remember the lesser the states easier to understand.

> 
> Renaming states is a good idea, but we should do it independent of fault
> handling. So how about doing it once we have robust fault handling in
> place.
Sure, can be done later too.
> 
> 
Over all the the logic seems to good, except I did not 
did not see where you are handling multiple sequenital faults
that can happen in pre/post handler. i.e once the fault happens
in say pre_handler, then the status goes to KPROBE_HIT_FAULT,
and say this fault is recovered and the pre_handler continues and
again before returning from pre_handler their can be another fault
and this fault is not being handed currently.
Also I did not see why you are not changing the status back to
original status if the fault is recovered properly. i.e 
KPROBE_HIT_ACTIVE -> KPROBE_HIT_FAULT. In KPROBE_HIT_FAULT state
if this recovers, why not change this back to KPROBE_HIT_ACTIVE?
Anyreason for not doing this?


-Anil