Oleg,
On Tue, Aug 18, 2009 at 1:45 PM, Oleg Nesterov<o...@redhat.com> wrote:
> On 08/18, stephane eranian wrote:
>> > In any case. We should not look at SA_SIGINFO at all. If sys_sigaction()
>> > was
>> > called without SA_SIGINFO, then it doesn'matter if we send SEND_SIG_PRIV or
>> > siginfo_t with the correct si_fd/etc.
>> >
>> What's the official role of SA_SIGINFO? Pass a siginfo struct?
>>
>> Does POSIX describe the rules governing the content of si_fd?
>> Or is si_fd a Linux-ony extension in which case it goes with F_SETSIG.
>
> Not sure I understand your concern...
>
> OK. You suggest to pass siginfo_t with .si_fd/etc when we detect SA_SIGINFO.
>
The reason I refer to SA_SIGINFO is simply because of the excerpt from the
sigaction man page:
If SA_SIGINFO is specified in sa_flags, then sa_sigaction (instead of
sa_handler) specifies the signal-handling function for signum. This
function receives the signal number as its first argument, a pointer to
a siginfo_t as its second argument and a pointer to a ucontext_t (cast
to void *) as its third argument.
In other words, I use this to emphasize the fact that to get a siginfo
struct, you need to pass SA_SIGINFO and use sa_sigaction instead of
sa_handler. That's all I am saying.
> But, in that case we can _always_ pass siginfo_t, regardless of SA_SIGINFO.
> If the task has a signal handler and sigaction() was called without
> SA_SIGINFO, then the handler must not look into *info (the second arg of
> sigaction->sa_sigaction). And in fact, __setup_rt_frame() doesn't even
> copy the info to the user-space:
>
> if (ka->sa.sa_flags & SA_SIGINFO) {
> if (copy_siginfo_to_user(&frame->info, info))
> return -EFAULT;
> }
>
> OK? Or I missed something?
>
No, I think we are in agreement. To get meaningful siginfo use SA_SIGINFO.
> Or. Suppose that some app does:
>
> void io_handler(int sig, siginfo_t *info, void *u)
> {
> if ((info->si_code & __SI_MASK) != SI_POLL) {
> // RT signal failed! sig MUST be == SIGIO
> recover();
> } else {
> do_something(info->si_fd);
> }
> }
>
> int main(void)
> {
> sigaction(SIGRTMIN, { SA_SIGINFO, io_handler });
> sigaction(SIGIO, { SA_SIGINFO, io_handler });
> ...
> }
>
I don't think you can check si_code without first checking which
signal it is in si_signo. The values for si_code overlap between
the different struct in siginfo->_sifields.
>> It would seem natural that in the siginfo passed to the handler of SIGIO, the
>> file descriptor be passed by default. That is all I am trying to say here.
>
> Completely agreed! I was always puzzled by send_sigio_to_task(). I was never
> able to understand why it looks so strange.
>
> So, I think it should be
>
> static void send_sigio_to_task(struct task_struct *p,
> struct fown_struct *fown,
> int fd,
> int reason)
> {
> siginfo_t si;
> /*
> * F_SETSIG can change ->signum lockless in parallel, make
> * sure we read it once and use the same value throughout.
> */
> int signum = ACCESS_ONCE(fown->signum) ?: SIGIO;
>
> if (!sigio_perm(p, fown, signum))
> return;
>
> si.si_signo = signum;
> si.si_errno = 0;
> si.si_code = reason;
> si.si_fd = fd;
> /* Make sure we are called with one of the POLL_*
> reasons, otherwise we could leak kernel stack into
> userspace. */
> BUG_ON((reason & __SI_MASK) != __SI_POLL);
> if (reason - POLL_IN >= NSIGPOLL)
> si.si_band = ~0L;
> else
> si.si_band = band_table[reason - POLL_IN];
>
> /* Failure to queue an rt signal must be reported as SIGIO */
> if (!group_send_sig_info(signum, &si, p))
> group_send_sig_info(SIGIO, SEND_SIG_PRIV, p);
> }
>
> (except it should be on top of fcntl-add-f_etown_ex.patch).
> This way, at least we don't break the "detect RT signal failed" above.
>
> What do you think?
>
> But let me repeat: I just can't convince myself we have a good reason
> to change the strange, but carefully documented behaviour.
>
I agree with you. Given the existing documentation in the man page
of fcntl() and the various code examples. I think it is possible for
developers to figure out how to get the si_fd in the handler. This
problem is not specific to perf_counters nor perfmon. Any SIGIO-based
program may be interested in getting si_fd, therefore I am assuming the
solution is well understood at this point.
------------------------------------------------------------------------------
Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day
trial. Simplify your report design, integration and deployment - and focus on
what you do best, core application coding. Discover what's new with
Crystal Reports now. http://p.sf.net/sfu/bobj-july
_______________________________________________
perfmon2-devel mailing list
perfmon2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/perfmon2-devel
