[PATCH 00/10] nbd updates

[PATCH 00/10] nbd updates

[Markus Pargmann]

Hi,

This series contains some cleanups for nbd. Also I add a debugfs directory for
nbd and every connected nbd. It contains some information about the internal
state of nbd and should help to debug issues.

The last patch creates uevents for nbd connections. The idea is to let the
userspace know when we are connected.

Best regards,

Markus

Markus Pargmann (10):
  nbd: sock_shutdown, remove conditional lock
  nbd: restructure sock_shutdown
  nbd: Remove 'harderror' and propagate error properly
  nbd: Move clear queue debug message
  nbd: Remove variable 'pid'
  nbd: Add debugfs entries
  nbd: Change 'disconnect' to be boolean
  nbd: Rename functions for clearness of recv/send path
  nbd: flags is a u32 variable
  nbd: Create size change events for userspace

 drivers/block/nbd.c | 346 +++++++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 278 insertions(+), 68 deletions(-)

-- 
2.1.4

[PATCH 01/10] nbd: sock_shutdown, remove conditional lock

[Markus Pargmann]

Move the conditional lock from sock_shutdown into the surrounding code.

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

diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
index 7b9ae7a65c1e..ff59093c5dc1 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -117,18 +117,14 @@ static void nbd_end_request(struct nbd_device *nbd, struct request *req)
 /*
  * Forcibly shutdown the socket causing all listeners to error
  */
-static void sock_shutdown(struct nbd_device *nbd, int lock)
+static void sock_shutdown(struct nbd_device *nbd)
 {
-	if (lock)
-		mutex_lock(&nbd->tx_lock);
 	if (nbd->sock) {
 		dev_warn(disk_to_dev(nbd->disk), "shutting down socket\n");
 		kernel_sock_shutdown(nbd->sock, SHUT_RDWR);
 		nbd->sock = NULL;
 		del_timer_sync(&nbd->timeout_timer);
 	}
-	if (lock)
-		mutex_unlock(&nbd->tx_lock);
 }
 
 static void nbd_xmit_timeout(unsigned long arg)
@@ -427,7 +423,9 @@ static int nbd_do_it(struct nbd_device *nbd)
 		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);
+		mutex_lock(&nbd->tx_lock);
+		sock_shutdown(nbd);
+		mutex_unlock(&nbd->tx_lock);
 		ret = -ETIMEDOUT;
 	}
 
@@ -541,7 +539,9 @@ static int nbd_thread(void *data)
 						  &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);
+			mutex_lock(&nbd->tx_lock);
+			sock_shutdown(nbd);
+			mutex_unlock(&nbd->tx_lock);
 			break;
 		}
 
@@ -735,7 +735,7 @@ static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd,
 		mutex_lock(&nbd->tx_lock);
 		if (error)
 			return error;
-		sock_shutdown(nbd, 0);
+		sock_shutdown(nbd);
 		sock = nbd->sock;
 		nbd->sock = NULL;
 		nbd_clear_que(nbd);
-- 
2.1.4

Re: [PATCH 01/10] nbd: sock_shutdown, remove conditional lock

[Pavel Machek]

On Mon 2015-07-27 09:12:47, Markus Pargmann wrote:
> Move the conditional lock from sock_shutdown into the surrounding code.
> 
> Signed-off-by: Markus Pargmann <mpa@pengutronix.de>

Acked-by: Pavel Machek <pavel@ucw.cz>


-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[PATCH 02/10] nbd: restructure sock_shutdown

[Markus Pargmann]

This patch restructures sock_shutdown to avoid having the main code path
in an if block.

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

diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
index ff59093c5dc1..2c3661e4d364 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -119,12 +119,13 @@ static void nbd_end_request(struct nbd_device *nbd, struct request *req)
  */
 static void sock_shutdown(struct nbd_device *nbd)
 {
-	if (nbd->sock) {
-		dev_warn(disk_to_dev(nbd->disk), "shutting down socket\n");
-		kernel_sock_shutdown(nbd->sock, SHUT_RDWR);
-		nbd->sock = NULL;
-		del_timer_sync(&nbd->timeout_timer);
-	}
+	if (!nbd->sock)
+		return;
+
+	dev_warn(disk_to_dev(nbd->disk), "shutting down socket\n");
+	kernel_sock_shutdown(nbd->sock, SHUT_RDWR);
+	nbd->sock = NULL;
+	del_timer_sync(&nbd->timeout_timer);
 }
 
 static void nbd_xmit_timeout(unsigned long arg)
-- 
2.1.4

Re: [PATCH 02/10] nbd: restructure sock_shutdown

[Pavel Machek]

On Mon 2015-07-27 09:12:48, Markus Pargmann wrote:
> This patch restructures sock_shutdown to avoid having the main code path
> in an if block.
> 
> Signed-off-by: Markus Pargmann <mpa@pengutronix.de>

Acked-by: Pavel Machek <pavel@ucw.cz>


> ---
>  drivers/block/nbd.c | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
> index ff59093c5dc1..2c3661e4d364 100644
> --- a/drivers/block/nbd.c
> +++ b/drivers/block/nbd.c
> @@ -119,12 +119,13 @@ static void nbd_end_request(struct nbd_device *nbd, struct request *req)
>   */
>  static void sock_shutdown(struct nbd_device *nbd)
>  {
> -	if (nbd->sock) {
> -		dev_warn(disk_to_dev(nbd->disk), "shutting down socket\n");
> -		kernel_sock_shutdown(nbd->sock, SHUT_RDWR);
> -		nbd->sock = NULL;
> -		del_timer_sync(&nbd->timeout_timer);
> -	}
> +	if (!nbd->sock)
> +		return;
> +
> +	dev_warn(disk_to_dev(nbd->disk), "shutting down socket\n");
> +	kernel_sock_shutdown(nbd->sock, SHUT_RDWR);
> +	nbd->sock = NULL;
> +	del_timer_sync(&nbd->timeout_timer);
>  }
>  
>  static void nbd_xmit_timeout(unsigned long arg)

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH 02/10] nbd: restructure sock_shutdown

[Markus Pargmann]

[-- Attachment #1: Type: text/plain, Size: 1840 bytes --]

Hi,

On Sun, Aug 02, 2015 at 09:08:44AM +0200, Pavel Machek wrote:
> On Mon 2015-07-27 09:12:48, Markus Pargmann wrote:
> > This patch restructures sock_shutdown to avoid having the main code path
> > in an if block.
> > 
> > Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
> 
> Acked-by: Pavel Machek <pavel@ucw.cz>

Thanks for the reviews.

Best regards,

Markus

> 
> 
> > ---
> >  drivers/block/nbd.c | 13 +++++++------
> >  1 file changed, 7 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
> > index ff59093c5dc1..2c3661e4d364 100644
> > --- a/drivers/block/nbd.c
> > +++ b/drivers/block/nbd.c
> > @@ -119,12 +119,13 @@ static void nbd_end_request(struct nbd_device *nbd, struct request *req)
> >   */
> >  static void sock_shutdown(struct nbd_device *nbd)
> >  {
> > -	if (nbd->sock) {
> > -		dev_warn(disk_to_dev(nbd->disk), "shutting down socket\n");
> > -		kernel_sock_shutdown(nbd->sock, SHUT_RDWR);
> > -		nbd->sock = NULL;
> > -		del_timer_sync(&nbd->timeout_timer);
> > -	}
> > +	if (!nbd->sock)
> > +		return;
> > +
> > +	dev_warn(disk_to_dev(nbd->disk), "shutting down socket\n");
> > +	kernel_sock_shutdown(nbd->sock, SHUT_RDWR);
> > +	nbd->sock = NULL;
> > +	del_timer_sync(&nbd->timeout_timer);
> >  }
> >  
> >  static void nbd_xmit_timeout(unsigned long arg)
> 
> -- 
> (english) http://www.livejournal.com/~pavelmachek
> (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
> 

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[PATCH 03/10] nbd: Remove 'harderror' and propagate error properly

[Markus Pargmann]

Instead of a variable 'harderror' we can simply try to correctly
propagate errors to the userspace.

This patch removes the harderror variable and passes errors through
error pointers and nbd_do_it back to the userspace.

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

diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
index 2c3661e4d364..8bce05d0df5e 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -41,7 +41,6 @@
 
 struct nbd_device {
 	int flags;
-	int harderror;		/* Code of hard error			*/
 	struct socket * sock;	/* If == NULL, device is not ready, yet	*/
 	int magic;
 
@@ -329,26 +328,24 @@ static struct request *nbd_read_stat(struct nbd_device *nbd)
 	if (result <= 0) {
 		dev_err(disk_to_dev(nbd->disk),
 			"Receive control failed (result %d)\n", result);
-		goto harderror;
+		return ERR_PTR(result);
 	}
 
 	if (ntohl(reply.magic) != NBD_REPLY_MAGIC) {
 		dev_err(disk_to_dev(nbd->disk), "Wrong magic (0x%lx)\n",
 				(unsigned long)ntohl(reply.magic));
-		result = -EPROTO;
-		goto harderror;
+		return ERR_PTR(-EPROTO);
 	}
 
 	req = nbd_find_request(nbd, *(struct request **)reply.handle);
 	if (IS_ERR(req)) {
 		result = PTR_ERR(req);
 		if (result != -ENOENT)
-			goto harderror;
+			return ERR_PTR(result);
 
 		dev_err(disk_to_dev(nbd->disk), "Unexpected reply (%p)\n",
 			reply.handle);
-		result = -EBADR;
-		goto harderror;
+		return ERR_PTR(-EBADR);
 	}
 
 	if (ntohl(reply.error)) {
@@ -376,9 +373,6 @@ static struct request *nbd_read_stat(struct nbd_device *nbd)
 		}
 	}
 	return req;
-harderror:
-	nbd->harderror = result;
-	return NULL;
 }
 
 static ssize_t pid_show(struct device *dev,
@@ -413,8 +407,15 @@ static int nbd_do_it(struct nbd_device *nbd)
 
 	nbd->task_recv = current;
 
-	while ((req = nbd_read_stat(nbd)) != NULL)
+	while (1) {
+		req = nbd_read_stat(nbd);
+		if (IS_ERR(req)) {
+			ret = PTR_ERR(req);
+			break;
+		}
+
 		nbd_end_request(nbd, req);
+	}
 
 	nbd->task_recv = NULL;
 
@@ -734,8 +735,7 @@ static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd,
 		kthread_stop(thread);
 
 		mutex_lock(&nbd->tx_lock);
-		if (error)
-			return error;
+
 		sock_shutdown(nbd);
 		sock = nbd->sock;
 		nbd->sock = NULL;
@@ -754,7 +754,7 @@ static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd,
 			blkdev_reread_part(bdev);
 		if (nbd->disconnect) /* user requested, ignore socket errors */
 			return 0;
-		return nbd->harderror;
+		return error;
 	}
 
 	case NBD_CLEAR_QUE:
-- 
2.1.4

Re: [PATCH 03/10] nbd: Remove 'harderror' and propagate error properly

[Pavel Machek]

On Mon 2015-07-27 09:12:49, Markus Pargmann wrote:
> Instead of a variable 'harderror' we can simply try to correctly
> propagate errors to the userspace.
> 
> This patch removes the harderror variable and passes errors through
> error pointers and nbd_do_it back to the userspace.
> 
> Signed-off-by: Markus Pargmann <mpa@pengutronix.de>

Acked-by: Pavel Machek <pavel@ucw.cz>

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[PATCH 04/10] nbd: Move clear queue debug message

[Markus Pargmann]

This message was a warning without a reason. This patch moves it into
nbd_clear_que and transforms it to a debug message.

Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
---
 drivers/block/nbd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
index 8bce05d0df5e..5601523a8336 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -468,6 +468,7 @@ static void nbd_clear_que(struct nbd_device *nbd)
 		req->errors++;
 		nbd_end_request(nbd, req);
 	}
+	dev_dbg(disk_to_dev(nbd->disk), "queue cleared\n");
 }
 
 
@@ -740,7 +741,6 @@ static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd,
 		sock = nbd->sock;
 		nbd->sock = NULL;
 		nbd_clear_que(nbd);
-		dev_warn(disk_to_dev(nbd->disk), "queue cleared\n");
 		kill_bdev(bdev);
 		queue_flag_clear_unlocked(QUEUE_FLAG_DISCARD, nbd->disk->queue);
 		set_device_ro(bdev, false);
-- 
2.1.4

[PATCH 05/10] nbd: Remove variable 'pid'

[Markus Pargmann]

This patch uses nbd->task_recv to determine the value of the previously
used variable 'pid' for sysfs.

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

diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
index 5601523a8336..0075554a05b7 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -55,7 +55,6 @@ struct nbd_device {
 	struct gendisk *disk;
 	int blksize;
 	loff_t bytesize;
-	pid_t pid; /* pid of nbd-client, if attached */
 	int xmit_timeout;
 	int disconnect; /* a disconnect has been requested by user */
 
@@ -379,9 +378,9 @@ static ssize_t pid_show(struct device *dev,
 			struct device_attribute *attr, char *buf)
 {
 	struct gendisk *disk = dev_to_disk(dev);
+	struct nbd_device *nbd = (struct nbd_device *)disk->private_data;
 
-	return sprintf(buf, "%ld\n",
-		(long) ((struct nbd_device *)disk->private_data)->pid);
+	return sprintf(buf, "%d\n", task_pid_nr(nbd->task_recv));
 }
 
 static struct device_attribute pid_attr = {
@@ -397,16 +396,16 @@ static int nbd_do_it(struct nbd_device *nbd)
 	BUG_ON(nbd->magic != NBD_MAGIC);
 
 	sk_set_memalloc(nbd->sock->sk);
-	nbd->pid = task_pid_nr(current);
+
+	nbd->task_recv = current;
+
 	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");
-		nbd->pid = 0;
+		nbd->task_recv = NULL;
 		return ret;
 	}
 
-	nbd->task_recv = current;
-
 	while (1) {
 		req = nbd_read_stat(nbd);
 		if (IS_ERR(req)) {
@@ -417,6 +416,8 @@ static int nbd_do_it(struct nbd_device *nbd)
 		nbd_end_request(nbd, req);
 	}
 
+	device_remove_file(disk_to_dev(nbd->disk), &pid_attr);
+
 	nbd->task_recv = NULL;
 
 	if (signal_pending(current)) {
@@ -431,8 +432,6 @@ static int nbd_do_it(struct nbd_device *nbd)
 		ret = -ETIMEDOUT;
 	}
 
-	device_remove_file(disk_to_dev(nbd->disk), &pid_attr);
-	nbd->pid = 0;
 	return ret;
 }
 
@@ -708,7 +707,7 @@ static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd,
 		struct socket *sock;
 		int error;
 
-		if (nbd->pid)
+		if (nbd->task_recv)
 			return -EBUSY;
 		if (!nbd->sock)
 			return -EINVAL;
-- 
2.1.4

[PATCH 06/10] nbd: Add debugfs entries

[Markus Pargmann]

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

diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
index 0075554a05b7..188fe0d2b1ac 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -33,6 +33,7 @@
 #include <linux/net.h>
 #include <linux/kthread.h>
 #include <linux/types.h>
+#include <linux/debugfs.h>
 
 #include <asm/uaccess.h>
 #include <asm/types.h>
@@ -61,8 +62,18 @@ struct nbd_device {
 	struct timer_list timeout_timer;
 	struct task_struct *task_recv;
 	struct task_struct *task_send;
+
+#if IS_ENABLED(CONFIG_DEBUG_FS)
+	struct dentry *dbg_dir;
+#endif
 };
 
+#if IS_ENABLED(CONFIG_DEBUG_FS)
+static struct dentry *nbd_dbg_dir;
+#endif
+
+#define nbd_name(nbd) ((nbd)->disk->disk_name)
+
 #define NBD_MAGIC 0x68797548
 
 static unsigned int nbds_max = 16;
@@ -609,6 +620,9 @@ static void do_nbd_request(struct request_queue *q)
 	}
 }
 
+static int nbd_dev_dbg_init(struct nbd_device *nbd);
+static void nbd_dev_dbg_close(struct nbd_device *nbd);
+
 /* Must be called with tx_lock held */
 
 static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd,
@@ -725,13 +739,15 @@ static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd,
 			blk_queue_flush(nbd->disk->queue, 0);
 
 		thread = kthread_run(nbd_thread, nbd, "%s",
-				     nbd->disk->disk_name);
+				     nbd_name(nbd));
 		if (IS_ERR(thread)) {
 			mutex_lock(&nbd->tx_lock);
 			return PTR_ERR(thread);
 		}
 
+		nbd_dev_dbg_init(nbd);
 		error = nbd_do_it(nbd);
+		nbd_dev_dbg_close(nbd);
 		kthread_stop(thread);
 
 		mutex_lock(&nbd->tx_lock);
@@ -797,6 +813,161 @@ static const struct block_device_operations nbd_fops =
 	.ioctl =	nbd_ioctl,
 };
 
+#if IS_ENABLED(CONFIG_DEBUG_FS)
+
+static int nbd_dbg_tasks_show(struct seq_file *s, void *unused)
+{
+	struct nbd_device *nbd = s->private;
+
+	if (nbd->task_recv)
+		seq_printf(s, "recv: %d\n", task_pid_nr(nbd->task_recv));
+	if (nbd->task_send)
+		seq_printf(s, "send: %d\n", task_pid_nr(nbd->task_send));
+
+	return 0;
+}
+
+static int nbd_dbg_tasks_open(struct inode *inode, struct file *file)
+{
+	return single_open(file, nbd_dbg_tasks_show, inode->i_private);
+}
+
+static const struct file_operations nbd_dbg_tasks_ops = {
+	.open = nbd_dbg_tasks_open,
+	.read = seq_read,
+	.llseek = seq_lseek,
+	.release = single_release,
+};
+
+static int nbd_dbg_flags_show(struct seq_file *s, void *unused)
+{
+	struct nbd_device *nbd = s->private;
+	u32 flags = nbd->flags;
+
+	seq_printf(s, "Hex: 0x%08x\n\n", flags);
+
+	seq_puts(s, "Known flags:\n");
+
+	if (flags & NBD_FLAG_HAS_FLAGS)
+		seq_puts(s, "NBD_FLAG_HAS_FLAGS\n");
+	if (flags & NBD_FLAG_READ_ONLY)
+		seq_puts(s, "NBD_FLAG_READ_ONLY\n");
+	if (flags & NBD_FLAG_SEND_FLUSH)
+		seq_puts(s, "NBD_FLAG_SEND_FLUSH\n");
+	if (flags & NBD_FLAG_SEND_TRIM)
+		seq_puts(s, "NBD_FLAG_SEND_TRIM\n");
+
+	return 0;
+}
+
+static int nbd_dbg_flags_open(struct inode *inode, struct file *file)
+{
+	return single_open(file, nbd_dbg_flags_show, inode->i_private);
+}
+
+static const struct file_operations nbd_dbg_flags_ops = {
+	.open = nbd_dbg_flags_open,
+	.read = seq_read,
+	.llseek = seq_lseek,
+	.release = single_release,
+};
+
+static int nbd_dev_dbg_init(struct nbd_device *nbd)
+{
+	struct dentry *dir;
+	struct dentry *f;
+
+	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);
+	}
+	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);
+	}
+
+	return 0;
+}
+
+static void nbd_dev_dbg_close(struct nbd_device *nbd)
+{
+	debugfs_remove_recursive(nbd->dbg_dir);
+}
+
+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);
+
+	nbd_dbg_dir = dbg_dir;
+
+	return 0;
+}
+
+static void nbd_dbg_close(void)
+{
+	debugfs_remove_recursive(nbd_dbg_dir);
+}
+
+#else  /* IS_ENABLED(CONFIG_DEBUG_FS) */
+
+static int nbd_dev_dbg_init(struct nbd_device *nbd)
+{
+	return 0;
+}
+
+static void nbd_dev_dbg_close(struct nbd_device *nbd)
+{
+}
+
+static int nbd_dbg_init(void)
+{
+	return 0;
+}
+
+static void nbd_dbg_close(void)
+{
+}
+
+#endif
+
 /*
  * And here should be modules and kernel interface 
  *  (Just smiley confuses emacs :-)
@@ -874,6 +1045,8 @@ static int __init nbd_init(void)
 
 	printk(KERN_INFO "nbd: registered device at major %d\n", NBD_MAJOR);
 
+	nbd_dbg_init();
+
 	for (i = 0; i < nbds_max; i++) {
 		struct gendisk *disk = nbd_dev[i].disk;
 		nbd_dev[i].magic = NBD_MAGIC;
@@ -910,6 +1083,9 @@ out:
 static void __exit nbd_cleanup(void)
 {
 	int i;
+
+	nbd_dbg_close();
+
 	for (i = 0; i < nbds_max; i++) {
 		struct gendisk *disk = nbd_dev[i].disk;
 		nbd_dev[i].magic = 0;
-- 
2.1.4

[PATCH 07/10] nbd: Change 'disconnect' to be boolean

[Markus Pargmann]

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

diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
index 188fe0d2b1ac..44160a9e493e 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -57,7 +57,7 @@ struct nbd_device {
 	int blksize;
 	loff_t bytesize;
 	int xmit_timeout;
-	int disconnect; /* a disconnect has been requested by user */
+	bool disconnect; /* a disconnect has been requested by user */
 
 	struct timer_list timeout_timer;
 	struct task_struct *task_recv;
@@ -145,7 +145,7 @@ static void nbd_xmit_timeout(unsigned long arg)
 	if (list_empty(&nbd->queue_head))
 		return;
 
-	nbd->disconnect = 1;
+	nbd->disconnect = true;
 
 	task = READ_ONCE(nbd->task_recv);
 	if (task)
@@ -646,7 +646,7 @@ static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd,
 		if (!nbd->sock)
 			return -EINVAL;
 
-		nbd->disconnect = 1;
+		nbd->disconnect = true;
 
 		nbd_send_req(nbd, &sreq);
 		return 0;
@@ -674,7 +674,7 @@ static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd,
 			nbd->sock = sock;
 			if (max_part > 0)
 				bdev->bd_invalidated = 1;
-			nbd->disconnect = 0; /* we're connected now */
+			nbd->disconnect = false; /* we're connected now */
 			return 0;
 		}
 		return -EINVAL;
-- 
2.1.4

Re: [PATCH 07/10] nbd: Change 'disconnect' to be boolean

[Pavel Machek]

On Mon 2015-07-27 09:12:53, Markus Pargmann wrote:
> Signed-off-by: Markus Pargmann <mpa@pengutronix.de>

Acked-by: Pavel Machek  <pavel@ucw.cz>

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[PATCH 08/10] nbd: Rename functions for clearness of recv/send path

[Markus Pargmann]

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

diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
index 44160a9e493e..9862c3e64ff2 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -399,7 +399,7 @@ static struct device_attribute pid_attr = {
 	.show = pid_show,
 };
 
-static int nbd_do_it(struct nbd_device *nbd)
+static int nbd_thread_recv(struct nbd_device *nbd)
 {
 	struct request *req;
 	int ret;
@@ -530,7 +530,7 @@ error_out:
 	nbd_end_request(nbd, req);
 }
 
-static int nbd_thread(void *data)
+static int nbd_thread_send(void *data)
 {
 	struct nbd_device *nbd = data;
 	struct request *req;
@@ -584,7 +584,7 @@ static int nbd_thread(void *data)
  *   { printk( "Warning: Ignoring result!\n"); nbd_end_request( req ); }
  */
 
-static void do_nbd_request(struct request_queue *q)
+static void nbd_request_handler(struct request_queue *q)
 		__releases(q->queue_lock) __acquires(q->queue_lock)
 {
 	struct request *req;
@@ -738,7 +738,7 @@ static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd,
 		else
 			blk_queue_flush(nbd->disk->queue, 0);
 
-		thread = kthread_run(nbd_thread, nbd, "%s",
+		thread = kthread_run(nbd_thread_send, nbd, "%s",
 				     nbd_name(nbd));
 		if (IS_ERR(thread)) {
 			mutex_lock(&nbd->tx_lock);
@@ -746,7 +746,7 @@ static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd,
 		}
 
 		nbd_dev_dbg_init(nbd);
-		error = nbd_do_it(nbd);
+		error = nbd_thread_recv(nbd);
 		nbd_dev_dbg_close(nbd);
 		kthread_stop(thread);
 
@@ -1021,7 +1021,7 @@ static int __init nbd_init(void)
 		 * every gendisk to have its very own request_queue struct.
 		 * These structs are big so we dynamically allocate them.
 		 */
-		disk->queue = blk_init_queue(do_nbd_request, &nbd_lock);
+		disk->queue = blk_init_queue(nbd_request_handler, &nbd_lock);
 		if (!disk->queue) {
 			put_disk(disk);
 			goto out;
-- 
2.1.4

[PATCH 09/10] nbd: flags is a u32 variable

[Markus Pargmann]

Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
---
 drivers/block/nbd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
index 9862c3e64ff2..1176a3b27a7e 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -41,7 +41,7 @@
 #include <linux/nbd.h>
 
 struct nbd_device {
-	int flags;
+	u32 flags;
 	struct socket * sock;	/* If == NULL, device is not ready, yet	*/
 	int magic;
 
-- 
2.1.4

[PATCH 10/10] nbd: Create size change events for userspace

[Markus Pargmann]

The userspace needs to know when nbd devices are ready for use.
Currently no events are created for the userspace which doesn't work for
systemd.

See the discussion here: https://github.com/systemd/systemd/pull/358

This patch uses a central point to setup the nbd-internal sizes. A ioctl
to set a size does not lead to a visible size change. The size of the
block device will be kept at 0 until nbd is connected. As soon as it
connects, the size will be changed to the real value and a uevent is
created. When disconnecting, the blockdevice is set to 0 size and
another uevent is generated.

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

diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
index 1176a3b27a7e..95eb2904c324 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -97,6 +97,11 @@ static inline struct device *nbd_to_dev(struct nbd_device *nbd)
 	return disk_to_dev(nbd->disk);
 }
 
+static bool nbd_is_connected(struct nbd_device *nbd)
+{
+	return !!nbd->task_recv;
+}
+
 static const char *nbdcmd_to_ascii(int cmd)
 {
 	switch (cmd) {
@@ -109,6 +114,43 @@ static const char *nbdcmd_to_ascii(int cmd)
 	return "invalid";
 }
 
+static int nbd_size_clear(struct nbd_device *nbd, struct block_device *bdev)
+{
+	bdev->bd_inode->i_size = 0;
+	set_blocksize(bdev, 0);
+	set_capacity(nbd->disk, 0);
+	kobject_uevent(&nbd_to_dev(nbd)->kobj, KOBJ_CHANGE);
+
+	return 0;
+}
+
+static int nbd_size_update(struct nbd_device *nbd, struct block_device *bdev)
+{
+	int ret;
+
+	if (!nbd_is_connected(nbd))
+		return 0;
+
+	ret = set_blocksize(bdev, nbd->blksize);
+	if (ret)
+		return ret;
+
+	bdev->bd_inode->i_size = nbd->bytesize;
+	set_capacity(nbd->disk, nbd->bytesize >> 9);
+	kobject_uevent(&nbd_to_dev(nbd)->kobj, KOBJ_CHANGE);
+
+	return 0;
+}
+
+static int nbd_size_set(struct nbd_device *nbd, struct block_device *bdev,
+			int blocksize, int nr_blocks)
+{
+	nbd->blksize = blocksize;
+	nbd->bytesize = blocksize * nr_blocks;
+
+	return nbd_size_update(nbd, bdev);
+}
+
 static void nbd_end_request(struct nbd_device *nbd, struct request *req)
 {
 	int error = req->errors ? -EIO : 0;
@@ -399,7 +441,7 @@ static struct device_attribute pid_attr = {
 	.show = pid_show,
 };
 
-static int nbd_thread_recv(struct nbd_device *nbd)
+static int nbd_thread_recv(struct nbd_device *nbd, struct block_device *bdev)
 {
 	struct request *req;
 	int ret;
@@ -417,6 +459,8 @@ static int nbd_thread_recv(struct nbd_device *nbd)
 		return ret;
 	}
 
+	nbd_size_update(nbd, bdev);
+
 	while (1) {
 		req = nbd_read_stat(nbd);
 		if (IS_ERR(req)) {
@@ -427,6 +471,8 @@ static int nbd_thread_recv(struct nbd_device *nbd)
 		nbd_end_request(nbd, req);
 	}
 
+	nbd_size_clear(nbd, bdev);
+
 	device_remove_file(disk_to_dev(nbd->disk), &pid_attr);
 
 	nbd->task_recv = NULL;
@@ -681,19 +727,14 @@ static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd,
 	}
 
 	case NBD_SET_BLKSIZE:
-		nbd->blksize = arg;
-		nbd->bytesize &= ~(nbd->blksize-1);
-		bdev->bd_inode->i_size = nbd->bytesize;
-		set_blocksize(bdev, nbd->blksize);
-		set_capacity(nbd->disk, nbd->bytesize >> 9);
-		return 0;
+		return nbd_size_set(nbd, bdev, arg, nbd->bytesize / arg);
 
 	case NBD_SET_SIZE:
-		nbd->bytesize = arg & ~(nbd->blksize-1);
-		bdev->bd_inode->i_size = nbd->bytesize;
-		set_blocksize(bdev, nbd->blksize);
-		set_capacity(nbd->disk, nbd->bytesize >> 9);
-		return 0;
+		return nbd_size_set(nbd, bdev, nbd->blksize,
+				    arg / nbd->blksize);
+
+	case NBD_SET_SIZE_BLOCKS:
+		return nbd_size_set(nbd, bdev, nbd->blksize, arg);
 
 	case NBD_SET_TIMEOUT:
 		nbd->xmit_timeout = arg * HZ;
@@ -709,13 +750,6 @@ static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd,
 		nbd->flags = arg;
 		return 0;
 
-	case NBD_SET_SIZE_BLOCKS:
-		nbd->bytesize = ((u64) arg) * nbd->blksize;
-		bdev->bd_inode->i_size = nbd->bytesize;
-		set_blocksize(bdev, nbd->blksize);
-		set_capacity(nbd->disk, nbd->bytesize >> 9);
-		return 0;
-
 	case NBD_DO_IT: {
 		struct task_struct *thread;
 		struct socket *sock;
@@ -746,7 +780,7 @@ static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd,
 		}
 
 		nbd_dev_dbg_init(nbd);
-		error = nbd_thread_recv(nbd);
+		error = nbd_thread_recv(nbd, bdev);
 		nbd_dev_dbg_close(nbd);
 		kthread_stop(thread);
 
-- 
2.1.4

Re: [PATCH 10/10] nbd: Create size change events for userspace

[Markus Pargmann]

[-- Attachment #1: Type: text/plain, Size: 5419 bytes --]

On Mon, Jul 27, 2015 at 09:12:56AM +0200, Markus Pargmann wrote:
> The userspace needs to know when nbd devices are ready for use.
> Currently no events are created for the userspace which doesn't work for
> systemd.
> 
> See the discussion here: https://github.com/systemd/systemd/pull/358
> 
> This patch uses a central point to setup the nbd-internal sizes. A ioctl
> to set a size does not lead to a visible size change. The size of the
> block device will be kept at 0 until nbd is connected. As soon as it
> connects, the size will be changed to the real value and a uevent is
> created. When disconnecting, the blockdevice is set to 0 size and
> another uevent is generated.
> 
> Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
> ---
>  drivers/block/nbd.c | 74 ++++++++++++++++++++++++++++++++++++++---------------
>  1 file changed, 54 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
> index 1176a3b27a7e..95eb2904c324 100644
> --- a/drivers/block/nbd.c
> +++ b/drivers/block/nbd.c
> @@ -97,6 +97,11 @@ static inline struct device *nbd_to_dev(struct nbd_device *nbd)
>  	return disk_to_dev(nbd->disk);
>  }
>  
> +static bool nbd_is_connected(struct nbd_device *nbd)
> +{
> +	return !!nbd->task_recv;
> +}
> +
>  static const char *nbdcmd_to_ascii(int cmd)
>  {
>  	switch (cmd) {
> @@ -109,6 +114,43 @@ static const char *nbdcmd_to_ascii(int cmd)
>  	return "invalid";
>  }
>  
> +static int nbd_size_clear(struct nbd_device *nbd, struct block_device *bdev)
> +{
> +	bdev->bd_inode->i_size = 0;
> +	set_blocksize(bdev, 0);
> +	set_capacity(nbd->disk, 0);
> +	kobject_uevent(&nbd_to_dev(nbd)->kobj, KOBJ_CHANGE);
> +
> +	return 0;
> +}
> +
> +static int nbd_size_update(struct nbd_device *nbd, struct block_device *bdev)
> +{
> +	int ret;
> +
> +	if (!nbd_is_connected(nbd))
> +		return 0;
> +
> +	ret = set_blocksize(bdev, nbd->blksize);
> +	if (ret)
> +		return ret;
> +
> +	bdev->bd_inode->i_size = nbd->bytesize;
> +	set_capacity(nbd->disk, nbd->bytesize >> 9);
> +	kobject_uevent(&nbd_to_dev(nbd)->kobj, KOBJ_CHANGE);
> +
> +	return 0;
> +}
> +
> +static int nbd_size_set(struct nbd_device *nbd, struct block_device *bdev,
> +			int blocksize, int nr_blocks)
> +{
> +	nbd->blksize = blocksize;
> +	nbd->bytesize = blocksize * nr_blocks;
> +
> +	return nbd_size_update(nbd, bdev);
> +}
> +
>  static void nbd_end_request(struct nbd_device *nbd, struct request *req)
>  {
>  	int error = req->errors ? -EIO : 0;
> @@ -399,7 +441,7 @@ static struct device_attribute pid_attr = {
>  	.show = pid_show,
>  };
>  
> -static int nbd_thread_recv(struct nbd_device *nbd)
> +static int nbd_thread_recv(struct nbd_device *nbd, struct block_device *bdev)
>  {
>  	struct request *req;
>  	int ret;
> @@ -417,6 +459,8 @@ static int nbd_thread_recv(struct nbd_device *nbd)
>  		return ret;
>  	}
>  
> +	nbd_size_update(nbd, bdev);
> +
>  	while (1) {
>  		req = nbd_read_stat(nbd);
>  		if (IS_ERR(req)) {
> @@ -427,6 +471,8 @@ static int nbd_thread_recv(struct nbd_device *nbd)
>  		nbd_end_request(nbd, req);
>  	}
>  
> +	nbd_size_clear(nbd, bdev);
> +
>  	device_remove_file(disk_to_dev(nbd->disk), &pid_attr);
>  
>  	nbd->task_recv = NULL;
> @@ -681,19 +727,14 @@ static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd,
>  	}
>  
>  	case NBD_SET_BLKSIZE:
> -		nbd->blksize = arg;
> -		nbd->bytesize &= ~(nbd->blksize-1);
> -		bdev->bd_inode->i_size = nbd->bytesize;
> -		set_blocksize(bdev, nbd->blksize);
> -		set_capacity(nbd->disk, nbd->bytesize >> 9);
> -		return 0;
> +		return nbd_size_set(nbd, bdev, arg, nbd->bytesize / arg);

This does not work for 32bit systems. Have to use do_div() for this.
Will fix that for the next version.

Best regards,

Markus

>  
>  	case NBD_SET_SIZE:
> -		nbd->bytesize = arg & ~(nbd->blksize-1);
> -		bdev->bd_inode->i_size = nbd->bytesize;
> -		set_blocksize(bdev, nbd->blksize);
> -		set_capacity(nbd->disk, nbd->bytesize >> 9);
> -		return 0;
> +		return nbd_size_set(nbd, bdev, nbd->blksize,
> +				    arg / nbd->blksize);
> +
> +	case NBD_SET_SIZE_BLOCKS:
> +		return nbd_size_set(nbd, bdev, nbd->blksize, arg);
>  
>  	case NBD_SET_TIMEOUT:
>  		nbd->xmit_timeout = arg * HZ;
> @@ -709,13 +750,6 @@ static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd,
>  		nbd->flags = arg;
>  		return 0;
>  
> -	case NBD_SET_SIZE_BLOCKS:
> -		nbd->bytesize = ((u64) arg) * nbd->blksize;
> -		bdev->bd_inode->i_size = nbd->bytesize;
> -		set_blocksize(bdev, nbd->blksize);
> -		set_capacity(nbd->disk, nbd->bytesize >> 9);
> -		return 0;
> -
>  	case NBD_DO_IT: {
>  		struct task_struct *thread;
>  		struct socket *sock;
> @@ -746,7 +780,7 @@ static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd,
>  		}
>  
>  		nbd_dev_dbg_init(nbd);
> -		error = nbd_thread_recv(nbd);
> +		error = nbd_thread_recv(nbd, bdev);
>  		nbd_dev_dbg_close(nbd);
>  		kthread_stop(thread);
>  
> -- 
> 2.1.4
> 
> 

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]