Davide Libenzi wrote:
> On Mon, 21 May 2007, Oleg Nesterov wrote:
>
>>> + schedule();
>>> + locked = signalfd_lock(ctx, &lk);
>>> + if (unlikely(!locked)) {
>>> + /*
>>> + * Let the caller read zero byte, ala socket
>>> + * recv() when the peer disconnect. This test
>>> + * must be done before doing a dequeue_signal(),
>>> + * because if the sighand has been orphaned,
>>> + * the dequeue_signal() call is going to crash.
>>> + */
>> Imho, the comment is a bit confusing. dequeue_signal() needs ->siglock
>> even if signalfd_ctx is not orphaned.
>
> The comment looks clear to me. It states:
>
> 1) The policy of returning 0 when the sighand has been detached
>
> 2) That we _must_not_ call dequeue_signal() in case signalfd_lock() fails
>
> #ACK on the code mod below.
>
> - Davide
>
Andrew, please apply on top.
Simplify signalfd locking following suggestions by Oleg Nesterov.
Signed-off-by: Davi E. M. Arnaut <[EMAIL PROTECTED]>
Index: linux-2.6/fs/signalfd.c
===================================================================
--- linux-2.6.orig/fs/signalfd.c
+++ linux-2.6/fs/signalfd.c
@@ -211,13 +211,11 @@ static int signalfd_copyinfo(struct sign
static ssize_t signalfd_dequeue(struct signalfd_ctx *ctx, siginfo_t *info,
int nonblock)
{
- int locked;
ssize_t ret;
struct signalfd_lockctx lk;
DECLARE_WAITQUEUE(wait, current);
- locked = signalfd_lock(ctx, &lk);
- if (!locked)
+ if (!signalfd_lock(ctx, &lk))
return 0;
ret = dequeue_signal(lk.tsk, &ctx->sigmask, info);
@@ -235,24 +233,24 @@ static ssize_t signalfd_dequeue(struct s
for (;;) {
set_current_state(TASK_INTERRUPTIBLE);
ret = dequeue_signal(lk.tsk, &ctx->sigmask, info);
+ signalfd_unlock(&lk);
if (ret != 0)
break;
if (signal_pending(current)) {
ret = -ERESTARTSYS;
break;
}
- signalfd_unlock(&lk);
schedule();
- locked = signalfd_lock(ctx, &lk);
- if (unlikely(!locked)) {
+ ret = signalfd_lock(ctx, &lk);
+ if (unlikely(!ret)) {
/*
* Let the caller read zero byte, ala socket
* recv() when the peer disconnect. This test
* must be done before doing a dequeue_signal(),
* because if the sighand has been orphaned,
- * the dequeue_signal() call is going to crash.
+ * the dequeue_signal() call is going to crash
+ * because ->sighand will be long gone.
*/
- ret = 0;
break;
}
}
@@ -260,9 +258,6 @@ static ssize_t signalfd_dequeue(struct s
remove_wait_queue(&ctx->wqh, &wait);
__set_current_state(TASK_RUNNING);
- if (likely(locked))
- signalfd_unlock(&lk);
-
return ret;
}
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
