[PULL] NBD changes

[PULL] NBD changes

[Markus Pargmann]

Hi Jens,

This pull request contains 5 patches which are mainly cleanups.

Patch 1 fixes some unnecessarily complicated code I introduced some versions
ago for debugfs.

Patch 2 removes the criticised signal usage within NBD to kill the NBD threads
after a timeout. This code was used for the last years and is now replaced by
simply killing the tcp connection.

Patches 3-5 are some smaller cleanups.

Happy new year.

Best Regards,

Markus



The following changes since commit 8005c49d9aea74d382f474ce11afbbc7d7130bec:

  Linux 4.4-rc1 (2015-11-15 17:00:27 -0800)

are available in the git repository at:

  git://git.pengutronix.de/git/mpa/linux-nbd.git tags/nbd-for-4.5

for you to fetch changes up to e0f2a340a69db6c70e60bb55c96a75cf0268c9d7:

  nbd: Move flag parsing to a function (2015-11-16 14:44:11 +0100)

----------------------------------------------------------------
NBD for v4.5

----------------------------------------------------------------
Markus Pargmann (5):
      nbd: Fix debugfs error handling
      nbd: Remove signal usage
      nbd: Timeouts are not user requested disconnects
      nbd: Cleanup reset of nbd and bdev after a disconnect
      nbd: Move flag parsing to a function

 drivers/block/nbd.c | 258 +++++++++++++++++++++++-----------------------------
 1 file changed, 114 insertions(+), 144 deletions(-)

-- 
2.6.2

[PATCH 1/5] nbd: Fix debugfs error handling

[Markus Pargmann]

Static checker complains about the implemented error handling. It is
indeed wrong. We don't care about the return values of created debugfs
files.

We only have to check the return values of created dirs for NULL
pointer. If we use a null pointer as parent directory for files, this
may lead to debugfs files in wrong places.

Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
---
 drivers/block/nbd.c | 61 ++++++++++++++++-------------------------------------
 1 file changed, 18 insertions(+), 43 deletions(-)

diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
index 93b3f99b6865..b9e4179a950a 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -891,57 +891,31 @@ static const struct file_operations nbd_dbg_flags_ops = {
 static int nbd_dev_dbg_init(struct nbd_device *nbd)
 {
 	struct dentry *dir;
-	struct dentry *f;
+
+	if (!nbd_dbg_dir)
+		return -EIO;
 
 	dir = debugfs_create_dir(nbd_name(nbd), nbd_dbg_dir);
-	if (IS_ERR_OR_NULL(dir)) {
-		dev_err(nbd_to_dev(nbd), "Failed to create debugfs dir for '%s' (%ld)\n",
-			nbd_name(nbd), PTR_ERR(dir));
-		return PTR_ERR(dir);
+	if (!dir) {
+		dev_err(nbd_to_dev(nbd), "Failed to create debugfs dir for '%s'\n",
+			nbd_name(nbd));
+		return -EIO;
 	}
 	nbd->dbg_dir = dir;
 
-	f = debugfs_create_file("tasks", 0444, dir, nbd, &nbd_dbg_tasks_ops);
-	if (IS_ERR_OR_NULL(f)) {
-		dev_err(nbd_to_dev(nbd), "Failed to create debugfs file 'tasks', %ld\n",
-			PTR_ERR(f));
-		return PTR_ERR(f);
-	}
-
-	f = debugfs_create_u64("size_bytes", 0444, dir, &nbd->bytesize);
-	if (IS_ERR_OR_NULL(f)) {
-		dev_err(nbd_to_dev(nbd), "Failed to create debugfs file 'size_bytes', %ld\n",
-			PTR_ERR(f));
-		return PTR_ERR(f);
-	}
-
-	f = debugfs_create_u32("timeout", 0444, dir, &nbd->xmit_timeout);
-	if (IS_ERR_OR_NULL(f)) {
-		dev_err(nbd_to_dev(nbd), "Failed to create debugfs file 'timeout', %ld\n",
-			PTR_ERR(f));
-		return PTR_ERR(f);
-	}
-
-	f = debugfs_create_u32("blocksize", 0444, dir, &nbd->blksize);
-	if (IS_ERR_OR_NULL(f)) {
-		dev_err(nbd_to_dev(nbd), "Failed to create debugfs file 'blocksize', %ld\n",
-			PTR_ERR(f));
-		return PTR_ERR(f);
-	}
-
-	f = debugfs_create_file("flags", 0444, dir, &nbd, &nbd_dbg_flags_ops);
-	if (IS_ERR_OR_NULL(f)) {
-		dev_err(nbd_to_dev(nbd), "Failed to create debugfs file 'flags', %ld\n",
-			PTR_ERR(f));
-		return PTR_ERR(f);
-	}
+	debugfs_create_file("tasks", 0444, dir, nbd, &nbd_dbg_tasks_ops);
+	debugfs_create_u64("size_bytes", 0444, dir, &nbd->bytesize);
+	debugfs_create_u32("timeout", 0444, dir, &nbd->xmit_timeout);
+	debugfs_create_u32("blocksize", 0444, dir, &nbd->blksize);
+	debugfs_create_file("flags", 0444, dir, &nbd, &nbd_dbg_flags_ops);
 
 	return 0;
 }
 
 static void nbd_dev_dbg_close(struct nbd_device *nbd)
 {
-	debugfs_remove_recursive(nbd->dbg_dir);
+	if (nbd->dbg_dir)
+		debugfs_remove_recursive(nbd->dbg_dir);
 }
 
 static int nbd_dbg_init(void)
@@ -949,8 +923,8 @@ static int nbd_dbg_init(void)
 	struct dentry *dbg_dir;
 
 	dbg_dir = debugfs_create_dir("nbd", NULL);
-	if (IS_ERR(dbg_dir))
-		return PTR_ERR(dbg_dir);
+	if (!dbg_dir)
+		return -EIO;
 
 	nbd_dbg_dir = dbg_dir;
 
@@ -959,7 +933,8 @@ static int nbd_dbg_init(void)
 
 static void nbd_dbg_close(void)
 {
-	debugfs_remove_recursive(nbd_dbg_dir);
+	if (nbd_dbg_dir)
+		debugfs_remove_recursive(nbd_dbg_dir);
 }
 
 #else  /* IS_ENABLED(CONFIG_DEBUG_FS) */
-- 
2.6.2

[PATCH 2/5] nbd: Remove signal usage

[Markus Pargmann]

As discussed on the mailing list, the usage of signals for timeout
handling has a lot of potential issues. The nbd driver used for some
time signals for timeouts. These signals where able to get the threads
out of the blocking socket operations.

This patch removes all signal usage and uses a socket shutdown instead.
The socket descriptor itself is cleared later when the whole nbd device
is closed.

The tasks_lock is removed as we do not depend on this anymore. Instead
a new lock for the socket is introduced so we can safely work with the
socket in the timeout handler outside of the two main threads.

Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Christoph Hellwig <hch@infradead.org>
Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
---
 drivers/block/nbd.c | 126 ++++++++++++++++++++--------------------------------
 1 file changed, 48 insertions(+), 78 deletions(-)

diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
index b9e4179a950a..88762f640527 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -60,7 +60,8 @@ struct nbd_device {
 	bool disconnect; /* a disconnect has been requested by user */
 
 	struct timer_list timeout_timer;
-	spinlock_t tasks_lock;
+	/* protects initialization and shutdown of the socket */
+	spinlock_t sock_lock;
 	struct task_struct *task_recv;
 	struct task_struct *task_send;
 
@@ -129,13 +130,20 @@ static void nbd_end_request(struct nbd_device *nbd, struct request *req)
  */
 static void sock_shutdown(struct nbd_device *nbd)
 {
-	if (!nbd->sock)
+	spin_lock_irq(&nbd->sock_lock);
+
+	if (!nbd->sock) {
+		spin_unlock_irq(&nbd->sock_lock);
 		return;
+	}
 
 	dev_warn(disk_to_dev(nbd->disk), "shutting down socket\n");
 	kernel_sock_shutdown(nbd->sock, SHUT_RDWR);
+	sockfd_put(nbd->sock);
 	nbd->sock = NULL;
-	del_timer_sync(&nbd->timeout_timer);
+	spin_unlock_irq(&nbd->sock_lock);
+
+	del_timer(&nbd->timeout_timer);
 }
 
 static void nbd_xmit_timeout(unsigned long arg)
@@ -148,17 +156,15 @@ static void nbd_xmit_timeout(unsigned long arg)
 
 	nbd->disconnect = true;
 
-	spin_lock_irqsave(&nbd->tasks_lock, flags);
+	spin_lock_irqsave(&nbd->sock_lock, flags);
 
-	if (nbd->task_recv)
-		force_sig(SIGKILL, nbd->task_recv);
 
-	if (nbd->task_send)
-		force_sig(SIGKILL, nbd->task_send);
+	if (nbd->sock)
+		kernel_sock_shutdown(nbd->sock, SHUT_RDWR);
 
-	spin_unlock_irqrestore(&nbd->tasks_lock, flags);
+	spin_unlock_irqrestore(&nbd->sock_lock, flags);
 
-	dev_err(nbd_to_dev(nbd), "Connection timed out, killed receiver and sender, shutting down connection\n");
+	dev_err(nbd_to_dev(nbd), "Connection timed out, shutting down connection\n");
 }
 
 /*
@@ -171,7 +177,6 @@ static int sock_xmit(struct nbd_device *nbd, int send, void *buf, int size,
 	int result;
 	struct msghdr msg;
 	struct kvec iov;
-	sigset_t blocked, oldset;
 	unsigned long pflags = current->flags;
 
 	if (unlikely(!sock)) {
@@ -181,11 +186,6 @@ static int sock_xmit(struct nbd_device *nbd, int send, void *buf, int size,
 		return -EINVAL;
 	}
 
-	/* Allow interception of SIGKILL only
-	 * Don't allow other signals to interrupt the transmission */
-	siginitsetinv(&blocked, sigmask(SIGKILL));
-	sigprocmask(SIG_SETMASK, &blocked, &oldset);
-
 	current->flags |= PF_MEMALLOC;
 	do {
 		sock->sk->sk_allocation = GFP_NOIO | __GFP_MEMALLOC;
@@ -212,7 +212,6 @@ static int sock_xmit(struct nbd_device *nbd, int send, void *buf, int size,
 		buf += result;
 	} while (size > 0);
 
-	sigprocmask(SIG_SETMASK, &oldset, NULL);
 	tsk_restore_flags(current, pflags, PF_MEMALLOC);
 
 	if (!send && nbd->xmit_timeout)
@@ -406,23 +405,18 @@ static int nbd_thread_recv(struct nbd_device *nbd)
 {
 	struct request *req;
 	int ret;
-	unsigned long flags;
 
 	BUG_ON(nbd->magic != NBD_MAGIC);
 
 	sk_set_memalloc(nbd->sock->sk);
 
-	spin_lock_irqsave(&nbd->tasks_lock, flags);
 	nbd->task_recv = current;
-	spin_unlock_irqrestore(&nbd->tasks_lock, flags);
 
 	ret = device_create_file(disk_to_dev(nbd->disk), &pid_attr);
 	if (ret) {
 		dev_err(disk_to_dev(nbd->disk), "device_create_file failed!\n");
 
-		spin_lock_irqsave(&nbd->tasks_lock, flags);
 		nbd->task_recv = NULL;
-		spin_unlock_irqrestore(&nbd->tasks_lock, flags);
 
 		return ret;
 	}
@@ -439,19 +433,7 @@ static int nbd_thread_recv(struct nbd_device *nbd)
 
 	device_remove_file(disk_to_dev(nbd->disk), &pid_attr);
 
-	spin_lock_irqsave(&nbd->tasks_lock, flags);
 	nbd->task_recv = NULL;
-	spin_unlock_irqrestore(&nbd->tasks_lock, flags);
-
-	if (signal_pending(current)) {
-		ret = kernel_dequeue_signal(NULL);
-		dev_warn(nbd_to_dev(nbd), "pid %d, %s, got signal %d\n",
-			 task_pid_nr(current), current->comm, ret);
-		mutex_lock(&nbd->tx_lock);
-		sock_shutdown(nbd);
-		mutex_unlock(&nbd->tx_lock);
-		ret = -ETIMEDOUT;
-	}
 
 	return ret;
 }
@@ -544,11 +526,8 @@ static int nbd_thread_send(void *data)
 {
 	struct nbd_device *nbd = data;
 	struct request *req;
-	unsigned long flags;
 
-	spin_lock_irqsave(&nbd->tasks_lock, flags);
 	nbd->task_send = current;
-	spin_unlock_irqrestore(&nbd->tasks_lock, flags);
 
 	set_user_nice(current, MIN_NICE);
 	while (!kthread_should_stop() || !list_empty(&nbd->waiting_queue)) {
@@ -557,17 +536,6 @@ static int nbd_thread_send(void *data)
 					 kthread_should_stop() ||
 					 !list_empty(&nbd->waiting_queue));
 
-		if (signal_pending(current)) {
-			int ret = kernel_dequeue_signal(NULL);
-
-			dev_warn(nbd_to_dev(nbd), "pid %d, %s, got signal %d\n",
-				 task_pid_nr(current), current->comm, ret);
-			mutex_lock(&nbd->tx_lock);
-			sock_shutdown(nbd);
-			mutex_unlock(&nbd->tx_lock);
-			break;
-		}
-
 		/* extract request */
 		if (list_empty(&nbd->waiting_queue))
 			continue;
@@ -582,13 +550,7 @@ static int nbd_thread_send(void *data)
 		nbd_handle_req(nbd, req);
 	}
 
-	spin_lock_irqsave(&nbd->tasks_lock, flags);
 	nbd->task_send = NULL;
-	spin_unlock_irqrestore(&nbd->tasks_lock, flags);
-
-	/* Clear maybe pending signals */
-	if (signal_pending(current))
-		kernel_dequeue_signal(NULL);
 
 	return 0;
 }
@@ -636,6 +598,25 @@ static void nbd_request_handler(struct request_queue *q)
 	}
 }
 
+static int nbd_set_socket(struct nbd_device *nbd, struct socket *sock)
+{
+	int ret = 0;
+
+	spin_lock_irq(&nbd->sock_lock);
+
+	if (nbd->sock) {
+		ret = -EBUSY;
+		goto out;
+	}
+
+	nbd->sock = sock;
+
+out:
+	spin_unlock_irq(&nbd->sock_lock);
+
+	return ret;
+}
+
 static int nbd_dev_dbg_init(struct nbd_device *nbd);
 static void nbd_dev_dbg_close(struct nbd_device *nbd);
 
@@ -668,32 +649,26 @@ static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd,
 		return 0;
 	}
  
-	case NBD_CLEAR_SOCK: {
-		struct socket *sock = nbd->sock;
-		nbd->sock = NULL;
+	case NBD_CLEAR_SOCK:
+		sock_shutdown(nbd);
 		nbd_clear_que(nbd);
 		BUG_ON(!list_empty(&nbd->queue_head));
 		BUG_ON(!list_empty(&nbd->waiting_queue));
 		kill_bdev(bdev);
-		if (sock)
-			sockfd_put(sock);
 		return 0;
-	}
 
 	case NBD_SET_SOCK: {
-		struct socket *sock;
 		int err;
-		if (nbd->sock)
-			return -EBUSY;
-		sock = sockfd_lookup(arg, &err);
-		if (sock) {
-			nbd->sock = sock;
-			if (max_part > 0)
-				bdev->bd_invalidated = 1;
-			nbd->disconnect = false; /* we're connected now */
-			return 0;
-		}
-		return -EINVAL;
+		struct socket *sock = sockfd_lookup(arg, &err);
+
+		if (!sock)
+			return err;
+
+		err = nbd_set_socket(nbd, sock);
+		if (!err && max_part)
+			bdev->bd_invalidated = 1;
+
+		return err;
 	}
 
 	case NBD_SET_BLKSIZE:
@@ -734,7 +709,6 @@ static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd,
 
 	case NBD_DO_IT: {
 		struct task_struct *thread;
-		struct socket *sock;
 		int error;
 
 		if (nbd->task_recv)
@@ -769,14 +743,10 @@ static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd,
 		mutex_lock(&nbd->tx_lock);
 
 		sock_shutdown(nbd);
-		sock = nbd->sock;
-		nbd->sock = NULL;
 		nbd_clear_que(nbd);
 		kill_bdev(bdev);
 		queue_flag_clear_unlocked(QUEUE_FLAG_DISCARD, nbd->disk->queue);
 		set_device_ro(bdev, false);
-		if (sock)
-			sockfd_put(sock);
 		nbd->flags = 0;
 		nbd->bytesize = 0;
 		bdev->bd_inode->i_size = 0;
@@ -1043,7 +1013,7 @@ static int __init nbd_init(void)
 		nbd_dev[i].magic = NBD_MAGIC;
 		INIT_LIST_HEAD(&nbd_dev[i].waiting_queue);
 		spin_lock_init(&nbd_dev[i].queue_lock);
-		spin_lock_init(&nbd_dev[i].tasks_lock);
+		spin_lock_init(&nbd_dev[i].sock_lock);
 		INIT_LIST_HEAD(&nbd_dev[i].queue_head);
 		mutex_init(&nbd_dev[i].tx_lock);
 		init_timer(&nbd_dev[i].timeout_timer);
-- 
2.6.2

[PATCH 3/5] nbd: Timeouts are not user requested disconnects

[Markus Pargmann]

It may be useful to know in the client that a connection timed out. The
current code returns success for a timeout.

This patch reports the error code -ETIMEDOUT for a timeout.

Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
---
 drivers/block/nbd.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
index 88762f640527..0ba3149c48bb 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -57,6 +57,7 @@ struct nbd_device {
 	int blksize;
 	loff_t bytesize;
 	int xmit_timeout;
+	bool timedout;
 	bool disconnect; /* a disconnect has been requested by user */
 
 	struct timer_list timeout_timer;
@@ -154,10 +155,9 @@ static void nbd_xmit_timeout(unsigned long arg)
 	if (list_empty(&nbd->queue_head))
 		return;
 
-	nbd->disconnect = true;
-
 	spin_lock_irqsave(&nbd->sock_lock, flags);
 
+	nbd->timedout = true;
 
 	if (nbd->sock)
 		kernel_sock_shutdown(nbd->sock, SHUT_RDWR);
@@ -754,7 +754,10 @@ static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd,
 		if (max_part > 0)
 			blkdev_reread_part(bdev);
 		if (nbd->disconnect) /* user requested, ignore socket errors */
-			return 0;
+			error = 0;
+		if (nbd->timedout)
+			error = -ETIMEDOUT;
+
 		return error;
 	}
 
-- 
2.6.2

[PATCH 4/5] nbd: Cleanup reset of nbd and bdev after a disconnect

[Markus Pargmann]

Group all variables that are reset after a disconnect into reset
functions. This patch adds two of these functions, nbd_reset() and
nbd_bdev_reset().

Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
---
 drivers/block/nbd.c | 40 +++++++++++++++++++++++++++++-----------
 1 file changed, 29 insertions(+), 11 deletions(-)

diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
index 0ba3149c48bb..a1fa356e4cbf 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -617,6 +617,30 @@ out:
 	return ret;
 }
 
+/* Reset all properties of an NBD device */
+static void nbd_reset(struct nbd_device *nbd)
+{
+	nbd->disconnect = false;
+	nbd->timedout = false;
+	nbd->blksize = 1024;
+	nbd->bytesize = 0;
+	set_capacity(nbd->disk, 0);
+	nbd->flags = 0;
+	nbd->xmit_timeout = 0;
+	queue_flag_clear_unlocked(QUEUE_FLAG_DISCARD, nbd->disk->queue);
+	del_timer_sync(&nbd->timeout_timer);
+}
+
+static void nbd_bdev_reset(struct block_device *bdev)
+{
+	set_device_ro(bdev, false);
+	bdev->bd_inode->i_size = 0;
+	if (max_part > 0) {
+		blkdev_reread_part(bdev);
+		bdev->bd_invalidated = 1;
+	}
+}
+
 static int nbd_dev_dbg_init(struct nbd_device *nbd);
 static void nbd_dev_dbg_close(struct nbd_device *nbd);
 
@@ -745,19 +769,15 @@ static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd,
 		sock_shutdown(nbd);
 		nbd_clear_que(nbd);
 		kill_bdev(bdev);
-		queue_flag_clear_unlocked(QUEUE_FLAG_DISCARD, nbd->disk->queue);
-		set_device_ro(bdev, false);
-		nbd->flags = 0;
-		nbd->bytesize = 0;
-		bdev->bd_inode->i_size = 0;
-		set_capacity(nbd->disk, 0);
-		if (max_part > 0)
-			blkdev_reread_part(bdev);
+		nbd_bdev_reset(bdev);
+
 		if (nbd->disconnect) /* user requested, ignore socket errors */
 			error = 0;
 		if (nbd->timedout)
 			error = -ETIMEDOUT;
 
+		nbd_reset(nbd);
+
 		return error;
 	}
 
@@ -1024,14 +1044,12 @@ static int __init nbd_init(void)
 		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].waiting_wq);
-		nbd_dev[i].blksize = 1024;
-		nbd_dev[i].bytesize = 0;
 		disk->major = NBD_MAJOR;
 		disk->first_minor = i << part_shift;
 		disk->fops = &nbd_fops;
 		disk->private_data = &nbd_dev[i];
 		sprintf(disk->disk_name, "nbd%d", i);
-		set_capacity(disk, 0);
+		nbd_reset(&nbd_dev[i]);
 		add_disk(disk);
 	}
 
-- 
2.6.2

[PATCH 5/5] nbd: Move flag parsing to a function

[Markus Pargmann]

nbd changes properties of the blockdevice depending on flags that were
received. This patch moves this flag parsing into a separate function
nbd_parse_flags().

Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
---
 drivers/block/nbd.c | 22 +++++++++++++---------
 1 file changed, 13 insertions(+), 9 deletions(-)

diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
index a1fa356e4cbf..81a9be7d176c 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -641,6 +641,18 @@ static void nbd_bdev_reset(struct block_device *bdev)
 	}
 }
 
+static void nbd_parse_flags(struct nbd_device *nbd, struct block_device *bdev)
+{
+	if (nbd->flags & NBD_FLAG_READ_ONLY)
+		set_device_ro(bdev, true);
+	if (nbd->flags & NBD_FLAG_SEND_TRIM)
+		queue_flag_set_unlocked(QUEUE_FLAG_DISCARD, nbd->disk->queue);
+	if (nbd->flags & NBD_FLAG_SEND_FLUSH)
+		blk_queue_flush(nbd->disk->queue, REQ_FLUSH);
+	else
+		blk_queue_flush(nbd->disk->queue, 0);
+}
+
 static int nbd_dev_dbg_init(struct nbd_device *nbd);
 static void nbd_dev_dbg_close(struct nbd_device *nbd);
 
@@ -742,15 +754,7 @@ static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd,
 
 		mutex_unlock(&nbd->tx_lock);
 
-		if (nbd->flags & NBD_FLAG_READ_ONLY)
-			set_device_ro(bdev, true);
-		if (nbd->flags & NBD_FLAG_SEND_TRIM)
-			queue_flag_set_unlocked(QUEUE_FLAG_DISCARD,
-				nbd->disk->queue);
-		if (nbd->flags & NBD_FLAG_SEND_FLUSH)
-			blk_queue_flush(nbd->disk->queue, REQ_FLUSH);
-		else
-			blk_queue_flush(nbd->disk->queue, 0);
+		nbd_parse_flags(nbd, bdev);
 
 		thread = kthread_run(nbd_thread_send, nbd, "%s",
 				     nbd_name(nbd));
-- 
2.6.2

[PATCH] nbd: fix ifnullfree.cocci warnings

[kbuild test robot]

drivers/block/nbd.c:937:2-26: WARNING: NULL check before freeing functions like kfree, debugfs_remove, debugfs_remove_recursive or usb_free_urb is not needed. Maybe consider reorganizing relevant code to avoid passing NULL values.
drivers/block/nbd.c:918:2-26: WARNING: NULL check before freeing functions like kfree, debugfs_remove, debugfs_remove_recursive or usb_free_urb is not needed. Maybe consider reorganizing relevant code to avoid passing NULL values.

 NULL check before some freeing functions is not needed.

 Based on checkpatch warning
 "kfree(NULL) is safe this check is probably not required"
 and kfreeaddr.cocci by Julia Lawall.

Generated by: scripts/coccinelle/free/ifnullfree.cocci

CC: Markus Pargmann <mpa@pengutronix.de>
Signed-off-by: Fengguang Wu <fengguang.wu@intel.com>
---

 nbd.c |    6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -914,8 +914,7 @@ static int nbd_dev_dbg_init(struct nbd_d
 
 static void nbd_dev_dbg_close(struct nbd_device *nbd)
 {
-	if (nbd->dbg_dir)
-		debugfs_remove_recursive(nbd->dbg_dir);
+	debugfs_remove_recursive(nbd->dbg_dir);
 }
 
 static int nbd_dbg_init(void)
@@ -933,8 +932,7 @@ static int nbd_dbg_init(void)
 
 static void nbd_dbg_close(void)
 {
-	if (nbd_dbg_dir)
-		debugfs_remove_recursive(nbd_dbg_dir);
+	debugfs_remove_recursive(nbd_dbg_dir);
 }
 
 #else  /* IS_ENABLED(CONFIG_DEBUG_FS) */

Re: [PATCH 1/5] nbd: Fix debugfs error handling

[kbuild test robot]

Hi Markus,

[auto build test WARNING on block/for-next]
[also build test WARNING on v4.4-rc7 next-20151231]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]

url:    https://github.com/0day-ci/linux/commits/Markus-Pargmann/nbd-Fix-debugfs-error-handling/20160102-182030
base:   https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git for-next


coccinelle warnings: (new ones prefixed by >>)

>> drivers/block/nbd.c:937:2-26: WARNING: NULL check before freeing functions like kfree, debugfs_remove, debugfs_remove_recursive or usb_free_urb is not needed. Maybe consider reorganizing relevant code to avoid passing NULL values.
   drivers/block/nbd.c:918:2-26: WARNING: NULL check before freeing functions like kfree, debugfs_remove, debugfs_remove_recursive or usb_free_urb is not needed. Maybe consider reorganizing relevant code to avoid passing NULL values.

Please review and possibly fold the followup patch.

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

Re: [PATCH 2/5] nbd: Remove signal usage

[Christoph Hellwig]

Hi Markus,

this looks great!

Reviewed-by: Christoph Hellwig <hch@lst.de>

One thing I noticed, which might be a good cleanup in the future:

> -	spin_lock_irqsave(&nbd->tasks_lock, flags);
>  	nbd->task_recv = current;
> -	spin_unlock_irqrestore(&nbd->tasks_lock, flags);

It seems like task_{send,recv} are only used for the files showing
the pids.  Maybe you should just store the pids directly in the
nbd_device structure and also keep the sysfs file around for the
life time of that structure.