summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorSimon Rettberg2020-11-20 13:47:16 +0100
committerSimon Rettberg2020-11-20 13:47:16 +0100
commitf4ad97dd38c134be53f4bbb30d9fb9cd2ef19844 (patch)
tree7c43fdf49e47529ed6e031502d690e84e2cd3415
parent[BUILD] add support for atomic operations not supported by hardware (diff)
downloaddnbd3-f4ad97dd38c134be53f4bbb30d9fb9cd2ef19844.tar.gz
dnbd3-f4ad97dd38c134be53f4bbb30d9fb9cd2ef19844.tar.xz
dnbd3-f4ad97dd38c134be53f4bbb30d9fb9cd2ef19844.zip
[KERNEL] Cleanup thread cleanup, fix closing of device when busy
-rw-r--r--src/kernel/blk.c73
-rw-r--r--src/kernel/dnbd3_main.c3
-rw-r--r--src/kernel/net.c138
3 files changed, 124 insertions, 90 deletions
diff --git a/src/kernel/blk.c b/src/kernel/blk.c
index 43251d3..7315ccb 100644
--- a/src/kernel/blk.c
+++ b/src/kernel/blk.c
@@ -36,19 +36,25 @@
static int dnbd3_close_device(dnbd3_device_t *dev)
{
int result;
+ if (dev->imgname) {
+ dev_info(dnbd3_device_to_dev(dev), "closing down device.\n");
+ }
+ /* quickly fail all requests */
dnbd3_blk_fail_all_requests(dev);
dev->panic = 0;
dev->discover = 0;
result = dnbd3_net_disconnect(dev);
+ if (dev->imgname) {
+ kfree(dev->imgname);
+ dev->imgname = NULL;
+ }
+ /* new requests might have been queued up, */
+ /* but now that imgname is NULL no new ones can show up */
dnbd3_blk_fail_all_requests(dev);
blk_mq_freeze_queue(dev->queue);
set_capacity(dev->disk, 0);
blk_mq_unfreeze_queue(dev->queue);
- if (dev->imgname)
- {
- kfree(dev->imgname);
- dev->imgname = NULL;
- }
+ dev->reported_size = 0;
return result;
}
@@ -134,7 +140,7 @@ static int dnbd3_blk_ioctl(struct block_device *bdev, fmode_t mode, unsigned int
dev->rid = msg->rid;
dev->use_server_provided_alts = msg->use_server_provided_alts;
-
+ dev_info(dnbd3_device_to_dev(dev), "opening device.\n");
if (blk_queue->backing_dev_info != NULL) {
blk_queue->backing_dev_info->ra_pages = (msg->read_ahead_kb * 1024) / PAGE_SIZE;
}
@@ -159,9 +165,7 @@ static int dnbd3_blk_ioctl(struct block_device *bdev, fmode_t mode, unsigned int
if (dnbd3_net_connect(dev) != 0) {
/* probing server failed, cleanup connection and proceed with next specified server */
- dnbd3_blk_fail_all_requests(dev);
dnbd3_net_disconnect(dev);
- dnbd3_blk_fail_all_requests(dev);
result = -ENOENT;
} else {
/* probing server succeeds, abort probing of other servers */
@@ -230,7 +234,10 @@ static int dnbd3_blk_ioctl(struct block_device *bdev, fmode_t mode, unsigned int
/* specified server is known, so try to switch to it */
if (!is_same_server(&dev->cur_server, alt_server))
{
- /* specified server is not working, so switch to it */
+ if (alt_server->host.type == HOST_IP4)
+ dev_info(dnbd3_device_to_dev(dev), "manual server switch to %pI4\n", alt_server->host.addr);
+ else
+ dev_info(dnbd3_device_to_dev(dev), "manual server switch to [%pI6]\n", alt_server->host.addr);
/* save current working server */
/* lock device to get consistent copy of current working server */
spin_lock_irqsave(&dev->blk_lock, irqflags);
@@ -240,8 +247,6 @@ static int dnbd3_blk_ioctl(struct block_device *bdev, fmode_t mode, unsigned int
/* disconnect old server */
dnbd3_net_disconnect(dev);
- dev_info(dnbd3_device_to_dev(dev), "switching server ...\n");
-
/* connect to new specified server (switching) */
memcpy(&dev->cur_server, alt_server, sizeof(dev->cur_server));
result = dnbd3_net_connect(dev);
@@ -249,13 +254,15 @@ static int dnbd3_blk_ioctl(struct block_device *bdev, fmode_t mode, unsigned int
{
/* reconnect with old server if switching has failed */
memcpy(&dev->cur_server, &old_server, sizeof(dev->cur_server));
- if (dnbd3_net_connect(dev) != 0)
- {
- blk_mq_freeze_queue(dev->queue);
- set_capacity(dev->disk, 0);
- blk_mq_unfreeze_queue(dev->queue);
+ if (dnbd3_net_connect(dev) != 0) {
+ /* we couldn't reconnect to the old server */
+ /* device is dangling now and needs another SWITCH call */
+ dev_warn(dnbd3_device_to_dev(dev), "switching failed and could not switch back to old server - dangling device\n");
+ result = -ECONNABORTED;
+ } else {
+ /* switching didn't work but we are back to the old server */
+ result = -EAGAIN;
}
- result = -ECONNABORTED;
}
}
else
@@ -324,38 +331,23 @@ static blk_status_t dnbd3_queue_rq(struct blk_mq_hw_ctx *hctx, const struct blk_
dnbd3_device_t *dev = rq->q->queuedata;
unsigned long irqflags;
- blk_mq_start_request(rq);
-
if (dev->imgname == NULL)
- {
- blk_mq_end_request(rq, BLK_STS_IOERR);
- goto out;
- }
+ return BLK_STS_IOERR;
if (!(dnbd3_req_fs(rq)))
- {
- blk_mq_end_request(rq, BLK_STS_IOERR);
- goto out;
- }
+ return BLK_STS_IOERR;
if (PROBE_COUNT_TIMEOUT > 0 && dev->panic_count >= PROBE_COUNT_TIMEOUT)
- {
- blk_mq_end_request(rq, BLK_STS_TIMEOUT);
- goto out;
- }
+ return BLK_STS_TIMEOUT;
if (!(dnbd3_req_read(rq)))
- {
- blk_mq_end_request(rq, BLK_STS_NOTSUPP);
- goto out;
- }
+ return BLK_STS_NOTSUPP;
+ blk_mq_start_request(rq);
spin_lock_irqsave(&dev->blk_lock, irqflags);
list_add_tail(&rq->queuelist, &dev->request_queue_send);
spin_unlock_irqrestore(&dev->blk_lock, irqflags);
wake_up(&dev->process_queue_send);
-
-out:
return BLK_STS_OK;
}
@@ -461,6 +453,7 @@ out:
int dnbd3_blk_del_device(dnbd3_device_t *dev)
{
+ dev_dbg(dnbd3_device_to_dev(dev), "dnbd3_blk_del_device called\n");
while (atomic_cmpxchg(&dev->connection_lock, 0, 1) != 0)
schedule();
dnbd3_close_device(dev);
@@ -491,7 +484,8 @@ void dnbd3_blk_fail_all_requests(dnbd3_device_t *dev)
{
if (blk_request == blk_request2)
{
- dev_warn(dnbd3_device_to_dev(dev), "request is in both lists\n");
+ dev_warn(dnbd3_device_to_dev(dev), "same request is in request_queue_receive multiple times\n");
+ BUG();
dup = 1;
break;
}
@@ -510,6 +504,7 @@ void dnbd3_blk_fail_all_requests(dnbd3_device_t *dev)
if (blk_request == blk_request2)
{
dev_warn(dnbd3_device_to_dev(dev), "request is in both lists\n");
+ BUG();
dup = 1;
break;
}
@@ -523,9 +518,7 @@ void dnbd3_blk_fail_all_requests(dnbd3_device_t *dev)
list_del_init(&blk_request->queuelist);
if (dnbd3_req_fs(blk_request))
{
- spin_lock_irqsave(&dev->blk_lock, flags);
blk_mq_end_request(blk_request, BLK_STS_IOERR);
- spin_unlock_irqrestore(&dev->blk_lock, flags);
}
else if (dnbd3_req_special(blk_request))
{
diff --git a/src/kernel/dnbd3_main.c b/src/kernel/dnbd3_main.c
index c44dc44..133c579 100644
--- a/src/kernel/dnbd3_main.c
+++ b/src/kernel/dnbd3_main.c
@@ -94,6 +94,7 @@ static void __exit dnbd3_exit(void)
{
int i;
+ pr_debug("exiting kernel module...\n");
for (i = 0; i < max_devs; i++)
{
dnbd3_blk_del_device(&dnbd3_devices[i]);
@@ -102,7 +103,7 @@ static void __exit dnbd3_exit(void)
unregister_blkdev(major, "dnbd3");
kfree(dnbd3_devices);
- pr_info("exit kernel module\n");
+ pr_info("exit kernel module done\n");
}
module_init(dnbd3_init);
diff --git a/src/kernel/net.c b/src/kernel/net.c
index f460458..d846b64 100644
--- a/src/kernel/net.c
+++ b/src/kernel/net.c
@@ -36,6 +36,19 @@
#define ktime_to_s(kt) ktime_divns(kt, NSEC_PER_SEC)
#endif
+#if 1 // CONFIG_DEBUG_DRIVER
+#define ASSERT(x) \
+do { \
+ if (!(x)) { \
+ printk(KERN_EMERG "assertion failed %s: %d: %s\n", \
+ __FILE__, __LINE__, #x); \
+ BUG(); \
+ } \
+} while (0)
+#else
+#define ASSERT(x) do { } while (0)
+#endif
+
#define dnbd3_sock_create(af,type,proto,sock) sock_create_kern(&init_net, (af) == HOST_IP4 ? AF_INET : AF_INET6, type, proto, sock)
// cmd_flags and cmd_type are merged into cmd_flags now
@@ -108,8 +121,11 @@ static void dnbd3_net_heartbeat(struct timer_list *arg)
// send keepalive
if (req)
{
+ unsigned long irqflags;
dnbd3_cmd_to_priv(req, CMD_KEEPALIVE);
+ spin_lock_irqsave(&dev->blk_lock, irqflags);
list_add_tail(&req->queuelist, &dev->request_queue_send);
+ spin_unlock_irqrestore(&dev->blk_lock, irqflags);
wake_up(&dev->process_queue_send);
}
else
@@ -178,7 +194,7 @@ static int dnbd3_net_discover(void *data)
if (!buf)
{
dev_err(dnbd3_device_to_dev(dev), "kmalloc failed for payload buf (discover)\n");
- return -ENOMEM;
+ return ENOMEM;
}
payload = (serialized_buffer_t *)buf; // Reuse this buffer to save kernel mem
@@ -385,6 +401,7 @@ static int dnbd3_net_discover(void *data)
dev->better_sock = sock; // Pass over socket to take a shortcut in *_connect();
kfree(buf);
+ put_task_struct(dev->thread_discover);
dev->thread_discover = NULL;
dnbd3_net_disconnect(dev);
memcpy(&dev->cur_server, &dev->alt_servers[i], sizeof(dev->cur_server));
@@ -535,6 +552,7 @@ error: ;
(unsigned long long)best_rtt, (unsigned long long)dev->cur_rtt);
kfree(buf);
dev->better_sock = best_sock; // Take shortcut by continuing to use open connection
+ put_task_struct(dev->thread_discover);
dev->thread_discover = NULL;
dnbd3_net_disconnect(dev);
memcpy(&dev->cur_server, &dev->alt_servers[best_server], sizeof(dev->cur_server));
@@ -559,8 +577,11 @@ error: ;
}
kfree(buf);
- dev_dbg(dnbd3_device_to_dev(dev), "kthread dnbd3_net_discover terminated normally\n");
- dev->thread_discover = NULL;
+ if (kthread_should_stop()) {
+ dev_dbg(dnbd3_device_to_dev(dev), "kthread dnbd3_net_discover terminated normally\n");
+ } else {
+ dev_dbg(dnbd3_device_to_dev(dev), "kthread dnbd3_net_discover exited unexpectedly\n");
+ }
return 0;
}
@@ -653,26 +674,25 @@ static int dnbd3_net_send(void *data)
}
dev_dbg(dnbd3_device_to_dev(dev), "kthread dnbd3_net_send terminated normally\n");
- dev->thread_send = NULL;
- return ret;
+ return 0;
cleanup:
- if (dev->sock)
- kernel_sock_shutdown(dev->sock, SHUT_RDWR);
if (!atomic_read(&dev->connection_lock))
{
+ if (dev->sock)
+ kernel_sock_shutdown(dev->sock, SHUT_RDWR);
dev->panic = 1;
dev->discover = 1;
wake_up(&dev->process_queue_discover);
}
- if (!atomic_read(&dev->connection_lock) && ret != 0)
- dev_err(dnbd3_device_to_dev(dev), "kthread dnbd3_net_send terminated abnormally\n");
- else
+ if (kthread_should_stop() || ret == 0 || atomic_read(&dev->connection_lock)) {
dev_dbg(dnbd3_device_to_dev(dev), "kthread dnbd3_net_send terminated normally (cleanup)\n");
+ } else {
+ dev_err(dnbd3_device_to_dev(dev), "kthread dnbd3_net_send terminated abnormally (%d)\n", ret);
+ }
- dev->thread_send = NULL;
- return ret;
+ return 0;
}
static int dnbd3_net_receive(void *data)
@@ -816,8 +836,8 @@ static int dnbd3_net_receive(void *data)
}
spin_lock_irqsave(&dev->blk_lock, irqflags);
list_del_init(&blk_request->queuelist);
- blk_mq_end_request(blk_request, BLK_STS_OK);
spin_unlock_irqrestore(&dev->blk_lock, irqflags);
+ blk_mq_end_request(blk_request, BLK_STS_OK);
continue;
case CMD_GET_SERVERS:
@@ -902,26 +922,24 @@ static int dnbd3_net_receive(void *data)
}
dev_dbg(dnbd3_device_to_dev(dev), "kthread thread_receive terminated normally\n");
- dev->thread_receive = NULL;
- return ret;
+ return 0;
cleanup:
- if (dev->sock)
- kernel_sock_shutdown(dev->sock, SHUT_RDWR);
if (!atomic_read(&dev->connection_lock))
{
+ if (dev->sock)
+ kernel_sock_shutdown(dev->sock, SHUT_RDWR);
dev->panic = 1;
dev->discover = 1;
wake_up(&dev->process_queue_discover);
}
- if (!atomic_read(&dev->connection_lock) && ret != 0)
- dev_err(dnbd3_device_to_dev(dev), "kthread dnbd3_net_receive terminated abnormally\n");
- else
+ if (kthread_should_stop() || ret == 0 || atomic_read(&dev->connection_lock)) {
dev_dbg(dnbd3_device_to_dev(dev), "kthread dnbd3_net_receive terminated normally (cleanup)\n");
-
- dev->thread_receive = NULL;
- return ret;
+ } else {
+ dev_err(dnbd3_device_to_dev(dev), "kthread dnbd3_net_receive terminated abnormally (%d)\n", ret);
+ }
+ return 0;
}
static struct socket* dnbd3_connect(dnbd3_device_t *dev, dnbd3_host_t *host)
@@ -987,7 +1005,7 @@ static struct socket* dnbd3_connect(dnbd3_device_t *dev, dnbd3_host_t *host)
}
}
if (ret != 0) {
- // XXX How can we do a connect with short timeout? This is dumb
+ /* XXX How can we do a connect with short timeout? This is dumb */
ktime_t start = ktime_get_real();
while (ktime_ms_delta(ktime_get_real(), start) < SOCKET_TIMEOUT_CLIENT_DATA * 1000) {
struct sockaddr_storage addr;
@@ -1010,6 +1028,9 @@ error:
int dnbd3_net_connect(dnbd3_device_t *dev)
{
struct request *req1 = NULL;
+ unsigned long irqflags;
+
+ ASSERT(atomic_read(&dev->connection_lock));
// do some checks before connecting
req1 = kmalloc(sizeof(*req1), GFP_ATOMIC);
@@ -1037,11 +1058,16 @@ int dnbd3_net_connect(dnbd3_device_t *dev)
goto error;
}
+ ASSERT(dev->thread_send == NULL);
+ ASSERT(dev->thread_receive == NULL);
+ ASSERT(dev->thread_discover == NULL);
+
dnbd3_dev_dbg_host_cur(dev, "connecting ...\n");
if (dev->better_sock == NULL )
{
// no established connection yet from discovery thread, start new one
+ uint64_t reported_size;
dnbd3_request_t dnbd3_request;
dnbd3_reply_t dnbd3_reply;
struct msghdr msg;
@@ -1132,16 +1158,22 @@ int dnbd3_net_connect(dnbd3_device_t *dev)
goto error;
}
dev->rid = rid;
- dev->reported_size = serializer_get_uint64(&dev->payload_buffer);
- if (dev->reported_size < 4096)
+ reported_size = serializer_get_uint64(&dev->payload_buffer);
+ if (reported_size < 4096)
{
dnbd3_dev_err_host_cur(dev, "reported size by server is < 4096\n");
goto error;
}
- // store image information
- set_capacity(dev->disk, dev->reported_size >> 9); /* 512 Byte blocks */
- dnbd3_dev_dbg_host_cur(dev, "filesize: %llu\n", dev->reported_size);
- dev->update_available = 0;
+ if (dev->reported_size != 0 && dev->reported_size != reported_size) {
+ dnbd3_dev_err_host_cur(dev, "newly connected server reports size %llu, but expected is %llu\n", reported_size, dev->reported_size);
+ goto error;
+ } else if (dev->reported_size == 0) {
+ // store image information
+ dev->reported_size = reported_size;
+ set_capacity(dev->disk, dev->reported_size >> 9); /* 512 Byte blocks */
+ dnbd3_dev_dbg_host_cur(dev, "image size: %llu\n", dev->reported_size);
+ dev->update_available = 0;
+ }
}
else // Switching server, connection is already established and size request was executed
{
@@ -1189,7 +1221,9 @@ int dnbd3_net_connect(dnbd3_device_t *dev)
// Enqueue request to request_queue_send for a fresh list of alt servers
dnbd3_cmd_to_priv(req1, CMD_GET_SERVERS);
+ spin_lock_irqsave(&dev->blk_lock, irqflags);
list_add(&req1->queuelist, &dev->request_queue_send);
+ spin_unlock_irqrestore(&dev->blk_lock, irqflags);
wake_up(&dev->process_queue_send);
@@ -1204,6 +1238,24 @@ int dnbd3_net_connect(dnbd3_device_t *dev)
return 0;
error:
+ if (dev->thread_send)
+ {
+ kthread_stop(dev->thread_send);
+ put_task_struct(dev->thread_send);
+ dev->thread_send = NULL;
+ }
+ if (dev->thread_receive)
+ {
+ kthread_stop(dev->thread_receive);
+ put_task_struct(dev->thread_receive);
+ dev->thread_receive = NULL;
+ }
+ if (dev->thread_discover)
+ {
+ kthread_stop(dev->thread_discover);
+ put_task_struct(dev->thread_discover);
+ dev->thread_discover = NULL;
+ }
if (dev->sock)
{
sock_release(dev->sock);
@@ -1220,11 +1272,10 @@ error:
int dnbd3_net_disconnect(dnbd3_device_t *dev)
{
struct task_struct* thread = NULL;
- bool thread_not_terminated = false;
- int ret = 0;
+ int ret;
dev_dbg(dnbd3_device_to_dev(dev), "disconnecting device ...\n");
-
+ ASSERT(atomic_read(&dev->connection_lock));
dev->discover = 0;
@@ -1246,9 +1297,8 @@ int dnbd3_net_disconnect(dnbd3_device_t *dev)
dev_dbg(dnbd3_device_to_dev(dev), "send thread has never run\n");
} else {
/* thread has run, check if it has terminated successfully */
- if (dev->thread_send != NULL) {
+ if (ret < 0) {
dev_err(dnbd3_device_to_dev(dev), "send thread was not terminated correctly\n");
- thread_not_terminated = true;
}
}
dev->thread_send = NULL;
@@ -1265,9 +1315,8 @@ int dnbd3_net_disconnect(dnbd3_device_t *dev)
dev_dbg(dnbd3_device_to_dev(dev), "receive thread has never run\n");
} else {
/* thread has run, check if it has terminated successfully */
- if (dev->thread_receive != NULL) {
+ if (ret < 0) {
dev_err(dnbd3_device_to_dev(dev), "receive thread was not terminated correctly\n");
- thread_not_terminated = true;
}
}
dev->thread_receive = NULL;
@@ -1284,9 +1333,8 @@ int dnbd3_net_disconnect(dnbd3_device_t *dev)
dev_dbg(dnbd3_device_to_dev(dev), "discover thread has never run\n");
} else {
/* thread has run, check if it has terminated successfully */
- if (dev->thread_discover != NULL) {
- dev_err(dnbd3_device_to_dev(dev), "discover thread was not terminated correctly\n");
- thread_not_terminated = true;
+ if (ret < 0) {
+ dev_err(dnbd3_device_to_dev(dev), "discover thread was not terminated correctly (%d)\n", ret);
}
}
dev->thread_discover = NULL;
@@ -1300,13 +1348,5 @@ int dnbd3_net_disconnect(dnbd3_device_t *dev)
dev->cur_server.host.type = 0;
dev->cur_server.host.port = 0;
- if (thread_not_terminated) {
- dev_err(dnbd3_device_to_dev(dev), "failed to disconnect device\n");
- ret = -ENODEV;
- } else {
- dev_dbg(dnbd3_device_to_dev(dev), "device is disconnected\n");
- ret = 0;
- }
-
- return ret;
+ return 0;
}