From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 21671 invoked by alias); 17 Aug 2006 09:30:54 -0000 Received: (qmail 21664 invoked by uid 22791); 17 Aug 2006 09:30:53 -0000 X-Spam-Status: No, hits=-2.3 required=5.0 tests=AWL,BAYES_00,SPF_PASS X-Spam-Check-By: sourceware.org Received: from ausmtp06.au.ibm.com (HELO ausmtp06.au.ibm.com) (202.81.18.155) by sourceware.org (qpsmtpd/0.31) with ESMTP; Thu, 17 Aug 2006 09:30:42 +0000 Received: from sd0208e0.au.ibm.com (d23rh904.au.ibm.com [202.81.18.202]) by ausmtp06.au.ibm.com (8.13.6/8.13.6) with ESMTP id k7H9W5PH8765684 for ; Thu, 17 Aug 2006 19:32:05 +1000 Received: from d23av01.au.ibm.com (d23av01.au.ibm.com [9.190.250.242]) by sd0208e0.au.ibm.com (8.13.6/8.13.6/NCO v8.1.1) with ESMTP id k7H9Xkiv038680 for ; Thu, 17 Aug 2006 19:33:54 +1000 Received: from d23av01.au.ibm.com (loopback [127.0.0.1]) by d23av01.au.ibm.com (8.12.11.20060308/8.13.3) with ESMTP id k7H9UJT5012385 for ; Thu, 17 Aug 2006 19:30:19 +1000 Received: from [9.181.133.250] ([9.181.133.250]) by d23av01.au.ibm.com (8.12.11.20060308/8.12.11) with ESMTP id k7H9UHl2012233; Thu, 17 Aug 2006 19:30:18 +1000 Message-ID: <44E4379F.5050005@cn.ibm.com> Date: Thu, 17 Aug 2006 09:30:00 -0000 From: Li Guanglei Organization: IBM CSTL User-Agent: Thunderbird 1.5.0.5 (Windows/20060719) MIME-Version: 1.0 To: systemtap@sourceware.org CC: "Stone, Joshua I" , Manoj S Pattabhiraman Subject: Re: Tapset for Signals.... References: <44E3CBBA.4070704@cn.ibm.com> In-Reply-To: <44E3CBBA.4070704@cn.ibm.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit X-IsSubscribed: yes Mailing-List: contact systemtap-help@sourceware.org; run by ezmlm Precedence: bulk List-Subscribe: List-Post: List-Help: , Sender: systemtap-owner@sourceware.org X-SW-Source: 2006-q3/txt/msg00347.txt.bz2 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