[PATCH] nbd.c:sock_xmit: cleanup signal related code

[PATCH] nbd.c:sock_xmit: cleanup signal related code

[Oleg Nesterov]

sock_xmit() re-implements sigprocmask() and dequeue_signal_lock().

Note: I can't understand this dequeue_signal(), it can dequeue SIGKILL
for the user-space task doing nbd_ioctl() ?

Signed-off-by: Oleg Nesterov <oleg@tv-sign.ru>

--- sig/drivers/block/nbd.c~1_nbd	2007-05-21 13:57:50.000000000 +0400
+++ sig/drivers/block/nbd.c	2007-06-14 19:39:55.000000000 +0400
@@ -122,17 +122,12 @@ static int sock_xmit(struct socket *sock
 	int result;
 	struct msghdr msg;
 	struct kvec iov;
-	unsigned long flags;
-	sigset_t oldset;
+	sigset_t blocked, oldset;
 
 	/* Allow interception of SIGKILL only
 	 * Don't allow other signals to interrupt the transmission */
-	spin_lock_irqsave(&current->sighand->siglock, flags);
-	oldset = current->blocked;
-	sigfillset(&current->blocked);
-	sigdelsetmask(&current->blocked, sigmask(SIGKILL));
-	recalc_sigpending();
-	spin_unlock_irqrestore(&current->sighand->siglock, flags);
+	siginitsetinv(&blocked, sigmask(SIGKILL));
+	sigprocmask(SIG_SETMASK, &blocked, &oldset);
 
 	do {
 		sock->sk->sk_allocation = GFP_NOIO;
@@ -151,11 +146,9 @@ static int sock_xmit(struct socket *sock
 
 		if (signal_pending(current)) {
 			siginfo_t info;
-			spin_lock_irqsave(&current->sighand->siglock, flags);
 			printk(KERN_WARNING "nbd (pid %d: %s) got signal %d\n",
-				current->pid, current->comm, 
-				dequeue_signal(current, &current->blocked, &info));
-			spin_unlock_irqrestore(&current->sighand->siglock, flags);
+				current->pid, current->comm,
+				dequeue_signal_lock(current, &current->blocked, &info));
 			result = -EINTR;
 			break;
 		}
@@ -169,10 +162,7 @@ static int sock_xmit(struct socket *sock
 		buf += result;
 	} while (size > 0);
 
-	spin_lock_irqsave(&current->sighand->siglock, flags);
-	current->blocked = oldset;
-	recalc_sigpending();
-	spin_unlock_irqrestore(&current->sighand->siglock, flags);
+	sigprocmask(SIG_SETMASK, &oldset, NULL);
 
 	return result;
 }

Re: [PATCH] nbd.c:sock_xmit: cleanup signal related code

[Paul Clements]

Oleg Nesterov wrote:
> sock_xmit() re-implements sigprocmask() and dequeue_signal_lock().

Yeah, that code was written before those existed. Thanks for the clean up.

> Note: I can't understand this dequeue_signal(), it can dequeue SIGKILL
> for the user-space task doing nbd_ioctl() ?

So we can interrupt an nbd transmission without waiting for a TCP 
timeout (when we know the network is down).

> Signed-off-by: Oleg Nesterov <oleg@tv-sign.ru>

Patch looks good to me.

--
Paul

Re: [PATCH] nbd.c:sock_xmit: cleanup signal related code

[Oleg Nesterov]

On 06/14, Paul Clements wrote:
>
> Oleg Nesterov wrote:
> 
> >Note: I can't understand this dequeue_signal(), it can dequeue SIGKILL
> >for the user-space task doing nbd_ioctl() ?
> 
> So we can interrupt an nbd transmission without waiting for a TCP 
> timeout (when we know the network is down).

Yes, but that task should not survive after SIGKILL ? Note that SIGNAL_GROUP_EXIT
is set, this is not good.

Oleg.