Re: [RFC][PATCH] signals: don't copy siginfo_t on dequeue

From: Oleg Nesterov
Date: Thu Feb 26 2009 - 14:18:22 EST


On 02/26, Vegard Nossum wrote:
>
> Instead of copying the siginfo_t whenever a signal is dequeued, just
> get the pointer to the struct sigqueue, which can be freed by the
> caller when the signal has been delivered.

Yes, it would bi nice. But it is not that simple,

> -static void collect_signal(int sig, struct sigpending *list, siginfo_t *info)
> +static struct sigqueue *collect_signal(int sig, struct sigpending *list)
> {
> struct sigqueue *q, *first = NULL;
>
> @@ -377,40 +377,29 @@ static void collect_signal(int sig, struct sigpending *list, siginfo_t *info)
> if (first) {
> still_pending:
> list_del_init(&first->list);
> - copy_siginfo(info, &first->info);
> - __sigqueue_free(first);
> - } else {
> - /* Ok, it wasn't in the queue. This must be
> - a fast-pathed signal or we must have been
> - out of queue space. So zero out the info.
> - */
> - info->si_signo = sig;
> - info->si_errno = 0;
> - info->si_code = 0;
> - info->si_pid = 0;
> - info->si_uid = 0;
> }
> +
> + return first;
> }
>
> -static int __dequeue_signal(struct sigpending *pending, sigset_t *mask,
> - siginfo_t *info)
> +static struct sigqueue *__dequeue_signal(struct sigpending *pending,
> + sigset_t *mask)
> {
> int sig = next_signal(pending, mask);
>
> - if (sig) {
> - if (current->notifier) {
> - if (sigismember(current->notifier_mask, sig)) {
> - if (!(current->notifier)(current->notifier_data)) {
> - clear_thread_flag(TIF_SIGPENDING);
> - return 0;
> - }
> + if (!sig)
> + return NULL;
> +
> + if (current->notifier) {
> + if (sigismember(current->notifier_mask, sig)) {
> + if (!(current->notifier)(current->notifier_data)) {
> + clear_thread_flag(TIF_SIGPENDING);
> + return 0;
> }
> }
> -
> - collect_signal(sig, pending, info);
> }
>
> - return sig;
> + return collect_signal(sig, pending);

So. dequeue_signal() returns NULL if there is no siginfo queued. In that
case we assume that the signal is not pending.

But this is not right. Think about SEND_SIG_FORCED, or __sigqueue_alloc()
failure when the signal is sent. Or look at zap_other_threads() for example,
it just sets the bit in ->pending but doesn't queue siginfo.

Oleg.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/