From: Oleg Nesterov <oleg@redhat.com>
To: Vegard Nossum <vegard.nossum@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>,
Pekka Enberg <penberg@cs.helsinki.fi>,
linux-kernel@vger.kernel.org
Subject: Re: [RFC][PATCH] signals: don't copy siginfo_t on dequeue
Date: Thu, 26 Feb 2009 21:36:53 +0100 [thread overview]
Message-ID: <20090226203653.GA9285@redhat.com> (raw)
In-Reply-To: <19f34abd0902261218u388de998l1204857bde9bbe32@mail.gmail.com>
On 02/26, Vegard Nossum wrote:
>
> 2009/2/26 Oleg Nesterov <oleg@redhat.com>:
> > 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.
>
> I will investigate.
Cough. Well, I must admit I am a bit skeptical about this patch ;) Because
I suspect it will add more complications to the code. And _I think_ avoiding
copy_siginfo() does not buy too much. I will be happy if I am wrong, though.
But. If you are going to do another version, then please note there is another
problem with this patch, SIGQUEUE_PREALLOC.
If collect_signal() returns SIGQUEUE_PREALLOC info, we can not drop ->siglock.
I mean, once we drop ->siglock, this info can be freed, so for example
spin_unlock(&tsk->sighand->siglock);
- do_schedule_next_timer(info);
+ do_schedule_next_timer(&signal->info);
even this part is not safe.
Also. The patch uses __sigqueue_free() to free the delivered siginfo, but
this is not safe without ->siglock, we can race with sigqueue_free().
Oleg.
prev parent reply other threads:[~2009-02-26 20:40 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-02-26 18:44 [RFC][PATCH] signals: don't copy siginfo_t on dequeue Vegard Nossum
2009-02-26 18:50 ` Ingo Molnar
2009-02-26 19:10 ` Vegard Nossum
2009-02-26 20:19 ` Oleg Nesterov
2009-02-26 20:27 ` Ingo Molnar
2009-02-26 19:15 ` Oleg Nesterov
2009-02-26 20:18 ` Vegard Nossum
2009-02-26 20:36 ` Oleg Nesterov [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20090226203653.GA9285@redhat.com \
--to=oleg@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@elte.hu \
--cc=penberg@cs.helsinki.fi \
--cc=vegard.nossum@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.