Commit 7e2893a1 authored by Markus Pargmann's avatar Markus Pargmann Committed by Jens Axboe

nbd: Fix timeout detection

At the moment the nbd timeout just detects hanging tcp operations. This
is not enough to detect a hanging or bad connection as expected of a
timeout.

This patch redesigns the timeout detection to include some more cases.
The timeout is now in relation to replies from the server. If the server
does not send replies within the timeout the connection will be shut
down.

The patch adds a continous timer 'timeout_timer' that is setup in one of
two cases:
 - The request list is empty and we are sending the first request out to
   the server. We want to have a reply within the given timeout,
   otherwise we consider the connection to be dead.
 - A server response was received. This means the server is still
   communicating with us. The timer is reset to the timeout value.

The timer is not stopped if the list becomes empty. It will just trigger
a timeout which will directly leave the handling routine again as the
request list is empty.

The whole patch does not use any additional explicit locking. The
list_empty() calls are safe to be used concurrently. The timer is locked
internally as we just use mod_timer and del_timer_sync().

The patch is based on the idea of Michal Belczyk with a previous
different implementation.

Cc: Michal Belczyk <belczyk@bsd.krakow.pl>
Cc: Hermann Lauer <Hermann.Lauer@iwr.uni-heidelberg.de>
Signed-off-by: default avatarMarkus Pargmann <mpa@pengutronix.de>
Tested-by: default avatarHermann Lauer <Hermann.Lauer@iwr.uni-heidelberg.de>
Signed-off-by: default avatarJens Axboe <axboe@fb.com>
parent c45f5c99
...@@ -59,6 +59,10 @@ struct nbd_device { ...@@ -59,6 +59,10 @@ struct nbd_device {
pid_t pid; /* pid of nbd-client, if attached */ pid_t pid; /* pid of nbd-client, if attached */
int xmit_timeout; int xmit_timeout;
int disconnect; /* a disconnect has been requested by user */ int disconnect; /* a disconnect has been requested by user */
struct timer_list timeout_timer;
struct task_struct *task_recv;
struct task_struct *task_send;
}; };
#define NBD_MAGIC 0x68797548 #define NBD_MAGIC 0x68797548
...@@ -121,6 +125,7 @@ static void sock_shutdown(struct nbd_device *nbd, int lock) ...@@ -121,6 +125,7 @@ static void sock_shutdown(struct nbd_device *nbd, int lock)
dev_warn(disk_to_dev(nbd->disk), "shutting down socket\n"); dev_warn(disk_to_dev(nbd->disk), "shutting down socket\n");
kernel_sock_shutdown(nbd->sock, SHUT_RDWR); kernel_sock_shutdown(nbd->sock, SHUT_RDWR);
nbd->sock = NULL; nbd->sock = NULL;
del_timer_sync(&nbd->timeout_timer);
} }
if (lock) if (lock)
mutex_unlock(&nbd->tx_lock); mutex_unlock(&nbd->tx_lock);
...@@ -128,11 +133,23 @@ static void sock_shutdown(struct nbd_device *nbd, int lock) ...@@ -128,11 +133,23 @@ static void sock_shutdown(struct nbd_device *nbd, int lock)
static void nbd_xmit_timeout(unsigned long arg) static void nbd_xmit_timeout(unsigned long arg)
{ {
struct task_struct *task = (struct task_struct *)arg; struct nbd_device *nbd = (struct nbd_device *)arg;
struct task_struct *task;
if (list_empty(&nbd->queue_head))
return;
nbd->disconnect = 1;
task = READ_ONCE(nbd->task_recv);
if (task)
force_sig(SIGKILL, task);
printk(KERN_WARNING "nbd: killing hung xmit (%s, pid: %d)\n", task = READ_ONCE(nbd->task_send);
task->comm, task->pid); if (task)
force_sig(SIGKILL, task); force_sig(SIGKILL, nbd->task_send);
dev_err(nbd_to_dev(nbd), "Connection timed out, killed receiver and sender, shutting down connection\n");
} }
/* /*
...@@ -171,33 +188,12 @@ static int sock_xmit(struct nbd_device *nbd, int send, void *buf, int size, ...@@ -171,33 +188,12 @@ static int sock_xmit(struct nbd_device *nbd, int send, void *buf, int size,
msg.msg_controllen = 0; msg.msg_controllen = 0;
msg.msg_flags = msg_flags | MSG_NOSIGNAL; msg.msg_flags = msg_flags | MSG_NOSIGNAL;
if (send) { if (send)
struct timer_list ti;
if (nbd->xmit_timeout) {
init_timer(&ti);
ti.function = nbd_xmit_timeout;
ti.data = (unsigned long)current;
ti.expires = jiffies + nbd->xmit_timeout;
add_timer(&ti);
}
result = kernel_sendmsg(sock, &msg, &iov, 1, size); result = kernel_sendmsg(sock, &msg, &iov, 1, size);
if (nbd->xmit_timeout) else
del_timer_sync(&ti);
} else
result = kernel_recvmsg(sock, &msg, &iov, 1, size, result = kernel_recvmsg(sock, &msg, &iov, 1, size,
msg.msg_flags); msg.msg_flags);
if (signal_pending(current)) {
siginfo_t info;
printk(KERN_WARNING "nbd (pid %d: %s) got signal %d\n",
task_pid_nr(current), current->comm,
dequeue_signal_lock(current, &current->blocked, &info));
result = -EINTR;
sock_shutdown(nbd, !send);
break;
}
if (result <= 0) { if (result <= 0) {
if (result == 0) if (result == 0)
result = -EPIPE; /* short read */ result = -EPIPE; /* short read */
...@@ -210,6 +206,9 @@ static int sock_xmit(struct nbd_device *nbd, int send, void *buf, int size, ...@@ -210,6 +206,9 @@ static int sock_xmit(struct nbd_device *nbd, int send, void *buf, int size,
sigprocmask(SIG_SETMASK, &oldset, NULL); sigprocmask(SIG_SETMASK, &oldset, NULL);
tsk_restore_flags(current, pflags, PF_MEMALLOC); tsk_restore_flags(current, pflags, PF_MEMALLOC);
if (!send && nbd->xmit_timeout)
mod_timer(&nbd->timeout_timer, jiffies + nbd->xmit_timeout);
return result; return result;
} }
...@@ -415,12 +414,26 @@ static int nbd_do_it(struct nbd_device *nbd) ...@@ -415,12 +414,26 @@ static int nbd_do_it(struct nbd_device *nbd)
return ret; return ret;
} }
nbd->task_recv = current;
while ((req = nbd_read_stat(nbd)) != NULL) while ((req = nbd_read_stat(nbd)) != NULL)
nbd_end_request(nbd, req); nbd_end_request(nbd, req);
nbd->task_recv = NULL;
if (signal_pending(current)) {
siginfo_t info;
ret = dequeue_signal_lock(current, &current->blocked, &info);
dev_warn(nbd_to_dev(nbd), "pid %d, %s, got signal %d\n",
task_pid_nr(current), current->comm, ret);
sock_shutdown(nbd, 1);
ret = -ETIMEDOUT;
}
device_remove_file(disk_to_dev(nbd->disk), &pid_attr); device_remove_file(disk_to_dev(nbd->disk), &pid_attr);
nbd->pid = 0; nbd->pid = 0;
return 0; return ret;
} }
static void nbd_clear_que(struct nbd_device *nbd) static void nbd_clear_que(struct nbd_device *nbd)
...@@ -482,6 +495,9 @@ static void nbd_handle_req(struct nbd_device *nbd, struct request *req) ...@@ -482,6 +495,9 @@ static void nbd_handle_req(struct nbd_device *nbd, struct request *req)
nbd->active_req = req; nbd->active_req = req;
if (nbd->xmit_timeout && list_empty_careful(&nbd->queue_head))
mod_timer(&nbd->timeout_timer, jiffies + nbd->xmit_timeout);
if (nbd_send_req(nbd, req) != 0) { if (nbd_send_req(nbd, req) != 0) {
dev_err(disk_to_dev(nbd->disk), "Request send failed\n"); dev_err(disk_to_dev(nbd->disk), "Request send failed\n");
req->errors++; req->errors++;
...@@ -508,6 +524,8 @@ static int nbd_thread(void *data) ...@@ -508,6 +524,8 @@ static int nbd_thread(void *data)
struct nbd_device *nbd = data; struct nbd_device *nbd = data;
struct request *req; struct request *req;
nbd->task_send = current;
set_user_nice(current, MIN_NICE); set_user_nice(current, MIN_NICE);
while (!kthread_should_stop() || !list_empty(&nbd->waiting_queue)) { while (!kthread_should_stop() || !list_empty(&nbd->waiting_queue)) {
/* wait for something to do */ /* wait for something to do */
...@@ -515,6 +533,18 @@ static int nbd_thread(void *data) ...@@ -515,6 +533,18 @@ static int nbd_thread(void *data)
kthread_should_stop() || kthread_should_stop() ||
!list_empty(&nbd->waiting_queue)); !list_empty(&nbd->waiting_queue));
if (signal_pending(current)) {
siginfo_t info;
int ret;
ret = dequeue_signal_lock(current, &current->blocked,
&info);
dev_warn(nbd_to_dev(nbd), "pid %d, %s, got signal %d\n",
task_pid_nr(current), current->comm, ret);
sock_shutdown(nbd, 1);
break;
}
/* extract request */ /* extract request */
if (list_empty(&nbd->waiting_queue)) if (list_empty(&nbd->waiting_queue))
continue; continue;
...@@ -528,6 +558,9 @@ static int nbd_thread(void *data) ...@@ -528,6 +558,9 @@ static int nbd_thread(void *data)
/* handle request */ /* handle request */
nbd_handle_req(nbd, req); nbd_handle_req(nbd, req);
} }
nbd->task_send = NULL;
return 0; return 0;
} }
...@@ -648,6 +681,12 @@ static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd, ...@@ -648,6 +681,12 @@ static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd,
case NBD_SET_TIMEOUT: case NBD_SET_TIMEOUT:
nbd->xmit_timeout = arg * HZ; nbd->xmit_timeout = arg * HZ;
if (arg)
mod_timer(&nbd->timeout_timer,
jiffies + nbd->xmit_timeout);
else
del_timer_sync(&nbd->timeout_timer);
return 0; return 0;
case NBD_SET_FLAGS: case NBD_SET_FLAGS:
...@@ -842,6 +881,9 @@ static int __init nbd_init(void) ...@@ -842,6 +881,9 @@ static int __init nbd_init(void)
spin_lock_init(&nbd_dev[i].queue_lock); spin_lock_init(&nbd_dev[i].queue_lock);
INIT_LIST_HEAD(&nbd_dev[i].queue_head); INIT_LIST_HEAD(&nbd_dev[i].queue_head);
mutex_init(&nbd_dev[i].tx_lock); mutex_init(&nbd_dev[i].tx_lock);
init_timer(&nbd_dev[i].timeout_timer);
nbd_dev[i].timeout_timer.function = nbd_xmit_timeout;
nbd_dev[i].timeout_timer.data = (unsigned long)&nbd_dev[i];
init_waitqueue_head(&nbd_dev[i].active_wq); init_waitqueue_head(&nbd_dev[i].active_wq);
init_waitqueue_head(&nbd_dev[i].waiting_wq); init_waitqueue_head(&nbd_dev[i].waiting_wq);
nbd_dev[i].blksize = 1024; nbd_dev[i].blksize = 1024;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment