Tapset for Signals....

[Manoj S Pattabhiraman <mpattabh at in dot ibm dot com>]

Hi *,

Find the updated signal tapset,

        1. As per the suggestions, i have removed the argstr from the 
probe points.
        2. Added some checks to find whether the signals generated are 
USER or Kernel Mode in signal_handle probe.

I will start the work on Man pages (stapprobes) and the test script, once 
the signal tapset is finalised/completed.

Since i dont have cvs write access, can some one commit the latest signal 
tapset to cvs. And also please lemme know 
whats the procedure for getting write access to cvs.



Please review and lemme know your suggestions.

Thanks & Regards,    Manoj
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Manoj S Pattabhiraman, ADTools, Linux on zSeries, Ph : +91 80 51776449 T/L 
 92 46951, e-mail: mpattabh@in.ibm.com



Li Guanglei <guanglei@cn.ibm.com> 
08/17/2006 03:02 PM

To
systemtap@sourceware.org
cc
"Stone, Joshua I" <joshua.i.stone@intel.com>, Manoj S 
Pattabhiraman/India/IBM@IBMIN
Subject
Re: Tapset for Signals....






I committed the following changes based on our discussion:

   1. add "send2queue" and "name" variable for signal.send.part*
   2. add signal.send.return probe alias
   3. add signal.checkperm and signal.checkperm.return probe alias
   4. comment out signal.handle_stop
   5. alias all signal syscalls to syscall tapsets.

- Guanglei

Li Guanglei wrote:
> Stone, Joshua I wrote:
>> On Wednesday, August 16, 2006 1:36 AM, Li Guanglei wrote:
>>> Below are my questions while I am trying to add signal trace hooks
>>> into LKET:
>>
>> Thanks for reviewing.
>>
>>> 1. group_send_sig_info() will check the permissions for sending the
>>> signal before calling __group_send_sig_info(). So probing
>>> __group_send_sig_info() will omit the situation that a signal has been
>>> actually sent out but was rejected due to wrong permission.
>>>
>>> But __group_send_sig_info() will be also called by the following
>>> functions besides group_send_sig_info(),:
>>>    1> do_notify_parent() when a process exits
>>>    2> check_process_timers() for POSIX timers
>>>    3> do_notify_parent_cldstop()
>>>    4> kill_proc_info_as_uid()
>>>
>>> So which one do you prefer to probe, __group_send_sig_info or
>>> group_send_sig_info?
>>
>> I prefer __group_signal_sig_info, because it catches the additional
>> signals that you noted.  Also, consider the parallel case of
>> specific_send_sig_info, which is called by sys_tkill and sys_tgkill. In
>> that case, the permission check happens in the sys_* functions, before
>> specific_send_sig_info is called.  Thus on that path we are only
>> capturing the signals that will actually be sent (though possibly
>> ignored).
>>
> 
> Yes. Since do_tkill will also call check_kill_permission before calling 
> specific_send_sig_info(), __group_send_sig_info() looks more like a 
> parallel of specific_send_sig_info() than group_send_sig_info().
> 
>> This establishes a pretty consistent view of what signal.send means: if
>> a process successfully sends a signal, that will trigger signal.send.
>> Signals that are rejected because of permissions will return a failure
>> to the sender, and thus shouldn't trigger signal.send.  Signals that 
are
>> ignored will still report success to the sender, even though the
>> recipient dropped it on the floor, so it should trigger signal.send.
> 
> Yes. From the source codes, sending signal will return success(0) even 
> if it is ignored or is a duplication of an existing signal in the 
> pending queue. And it will return failure if failed permission checking.
> 
> So I agree with you that __group_send_sig_info() is a more suitable 
> probe point than group_send_sig_info().  Thanks for your suggestion.
> 
>>
>> If you want to find signals that are rejected due to permissions, then 
a
>> different probe point will be needed, like perhaps a return probe on
>> check_kill_permissions.
> 
> Yes. I am considering adding signal.check_perm. It will also probe the 
> entry of check_kill_permissions() to get more info about the signals 
> besides the return value.
> 
>>
>>> 2. send_sigqueue() and send_group_sigqueue() is only used for POSIX
>>> timers. If we choose to probe them, then I think it's better to add a
>>> local variable like "send_to_queue=1" to indicate that the signal is
>>> generated by posix timers. Then by looking at "send_to_queue" and
>>> "shared" local variables, we can determine which function actually
>>> triggers the probe handler.
>>
>> Ok, that's a good suggestion.
>>
>>>> * handle_stop_signal: Your comment says "fires when a stop signal is
>>>> sent", but this is incorrect.  This function is called to _check_
>>>> whether this signal is a stop/cont, and if so take special action.
>>>> Thus, this will get called for almost every signal, many of which
>>>> will not be stop signals.
>>> Yes. You are right. But the comment is still unchanged. :-)
>>
>> Indeed -- I didn't take it on myself to fix *all* of my gripes.  :)  I
>> didn't fix this one because I'm not convinced that it's needed, or what
>> useful information can be gotten from probing handle_stop_signal.  If
>> you really want, we could make a probe that does what the comment says,
>> something like this:
> 
> Yes. I see. At the time that handle_stop_signal() is called, it doesn't 
> know whether current signal is STOP/COUNT. So the calling of 
> handle_stop_signal() doesn't mean that the Kernel is now processing the 
> STOP/COUNT signal.
> 
>>
>> probe signal.send_stop = signal.send {
>>     if (sig != SIGSTOP) next;
>> }
>>
>> But users can do this themselves without much trouble, and with better
>> granularity.  It doesn't make sense for us to enumerate send_stop,
>> send_cont, send_int, etc., when the user can just check the sig value
>> manually.
> 
> Yes. The user can just look at the sig variable in signal.send to get 
> enough information.
> 
>>
>>>> * syscall duplication: some of your probes are duplicating points
>>>> from the syscall tapset (signal.syskill, etc.).  Do we really need
>>>> these?  If you really want these, it would be preferable to make use
>>>>     of the syscall tapset, e.g.: probe signal.syskill = syscall.kill
>>> Yes. Agree.
>>
>> Agree that they're not needed?  Or do you want the latter, where we 
keep
>> the probepoint, but alias it to the syscall tapset.
> 
> I don't have a obvious preference towards whether to keep or remove 
> them. I just think that keeping and alias them to syscall tapset will be 

> a little convenient when a user want to trace only the signal 
activities.
> 
> - Guanglei

Attachment: signal.stp
Description: Binary data