Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1398763 > unrolled thread

[PATCH v4 00/18] nbd: fixes for might_sleep warning, checkpatch warning and device wait.

Back to article view | Back to linux.kernel

Contents

  [PATCH v4 00/18] nbd: fixes for might_sleep warning, checkpatch warning and device wait. "Pranay Kr. Srivastava" <pranjas@gmail.com> - 2016-05-11 10:20 +0200
    [PATCH v4 01/18] nbd: Fix might_sleep warning on xmit timeout "Pranay Kr. Srivastava" <pranjas@gmail.com> - 2016-05-11 10:20 +0200
      Re: [PATCH v4 01/18] nbd: Fix might_sleep warning on xmit timeout Markus Pargmann <mpa@pengutronix.de> - 2016-05-12 11:50 +0200
    [PATCH v4 07/18] nbd: fix checkpatch split string warning. "Pranay Kr. Srivastava" <pranjas@gmail.com> - 2016-05-11 10:20 +0200
      Re: [PATCH v4 07/18] nbd: fix checkpatch split string warning. Markus Pargmann <mpa@pengutronix.de> - 2016-05-12 10:40 +0200
    [PATCH v4 03/18] nbd: fix checkpatch warning use linux/uaccess.h "Pranay Kr. Srivastava" <pranjas@gmail.com> - 2016-05-11 10:30 +0200
    [PATCH v4 09/18] nbd: fix checkpatch trailing whitespace warning. "Pranay Kr. Srivastava" <pranjas@gmail.com> - 2016-05-11 10:30 +0200
    [PATCH v4 16/18] nbd: fix checkpatch no new line after decleration warning "Pranay Kr. Srivastava" <pranjas@gmail.com> - 2016-05-11 10:30 +0200
    [PATCH v4 15/18] nbd: fix checkpatch printk warning to pr_info "Pranay Kr. Srivastava" <pranjas@gmail.com> - 2016-05-11 10:30 +0200
    [PATCH v4 10/18] nbd: fix checkpatch trailing whitespace warning. "Pranay Kr. Srivastava" <pranjas@gmail.com> - 2016-05-11 10:30 +0200
    [PATCH v4 18/18] make nbd device wait for its users in case of timeout "Pranay Kr. Srivastava" <pranjas@gmail.com> - 2016-05-11 10:30 +0200
      Re: [PATCH v4 18/18] make nbd device wait for its users in case of timeout Markus Pargmann <mpa@pengutronix.de> - 2016-05-12 11:20 +0200
    [PATCH v4 11/18] nbd : fix checkpatch structure declaration braces on next line warning. "Pranay Kr. Srivastava" <pranjas@gmail.com> - 2016-05-11 10:30 +0200
    [PATCH v4 06/18] nbd: fix checkpatch warning no newline after decleration. "Pranay Kr. Srivastava" <pranjas@gmail.com> - 2016-05-11 10:30 +0200
    [PATCH v4 17/18] nbd: fix checkpatch printk warning to pr_info "Pranay Kr. Srivastava" <pranjas@gmail.com> - 2016-05-11 10:30 +0200
    [PATCH v4 14/18] nbd: fix checkpatch no extra line after decleration warning "Pranay Kr. Srivastava" <pranjas@gmail.com> - 2016-05-11 10:30 +0200
    [PATCH v4 13/18] nbd : fix checkpatch printk warning "Pranay Kr. Srivastava" <pranjas@gmail.com> - 2016-05-11 10:30 +0200
    [PATCH v4 12/18] nbd : fix checkpatch trailing whitespace warning "Pranay Kr. Srivastava" <pranjas@gmail.com> - 2016-05-11 10:30 +0200

#1398763 — [PATCH v4 00/18] nbd: fixes for might_sleep warning, checkpatch warning and device wait.

Changes in v4
	- make nbd device wait for users to release
	  device instead of hard reset on a timeout
	  error as not all filesystems expect their
	  block device to vanish under them.

Changes in v3
	- Split checkpatch changes into single patch per change.

Changes in v2
	- add checkpatch changes in a single patch.

Changes in v1
	- fix might_sleep warning on xmit_timeout.



Pranay Kr. Srivastava (18):
  nbd: Fix might_sleep warning on xmit timeout
  nbd: fix checkpatch trailing space warning.
  nbd: fix checkpatch warning use linux/uaccess.h
  nbd : fix checkpatch pointer declaration warning
  nbd: fix checkpatch warning no newline after decleration.
  nbd: fix checkpatch warning no newline after decleration.
  nbd: fix checkpatch split string warning.
  nbd : fix checkpatch line over 80 char warning
  nbd: fix checkpatch trailing whitespace warning.
  nbd: fix checkpatch trailing whitespace warning.
  nbd : fix checkpatch structure declaration braces on next line
    warning.
  nbd : fix checkpatch trailing whitespace warning
  nbd : fix checkpatch printk warning
  nbd: fix checkpatch no extra line after decleration warning
  nbd: fix checkpatch printk warning to pr_info
  nbd: fix checkpatch no new line after decleration warning
  nbd: fix checkpatch printk warning to pr_info
  make nbd device wait for its users in case of timeout

 drivers/block/nbd.c      | 131 +++++++++++++++++++++++++++++++++--------------
 include/uapi/linux/nbd.h |   1 +
 2 files changed, 94 insertions(+), 38 deletions(-)

-- 
2.6.2

[toc] | [next] | [standalone]

#1398764 — [PATCH v4 01/18] nbd: Fix might_sleep warning on xmit timeout

This patch fixes the warning generated when a timeout occurs
on the request and socket is closed from a non-sleep context
by

1. Moving the socket closing on a timeout to nbd_thread_send

2. Make sock lock to be a mutex instead of a spin lock, since
   nbd_xmit_timeout doesn't need to hold it anymore.

Signed-off-by: Pranay Kr. Srivastava <pranjas@gmail.com>
---
 drivers/block/nbd.c | 65 ++++++++++++++++++++++++++++++++---------------------
 1 file changed, 39 insertions(+), 26 deletions(-)

diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
index 31e73a7..c79bcd7 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -57,12 +57,12 @@ struct nbd_device {
 	int blksize;
 	loff_t bytesize;
 	int xmit_timeout;
-	bool timedout;
+	atomic_t timedout;
 	bool disconnect; /* a disconnect has been requested by user */
 
 	struct timer_list timeout_timer;
 	/* protects initialization and shutdown of the socket */
-	spinlock_t sock_lock;
+	struct mutex sock_lock;
 	struct task_struct *task_recv;
 	struct task_struct *task_send;
 
@@ -172,10 +172,9 @@ static void nbd_end_request(struct nbd_device *nbd, struct request *req)
  */
 static void sock_shutdown(struct nbd_device *nbd)
 {
-	spin_lock_irq(&nbd->sock_lock);
-
+	mutex_lock(&nbd->sock_lock);
 	if (!nbd->sock) {
-		spin_unlock_irq(&nbd->sock_lock);
+		mutex_unlock(&nbd->sock_lock);
 		return;
 	}
 
@@ -183,27 +182,19 @@ static void sock_shutdown(struct nbd_device *nbd)
 	kernel_sock_shutdown(nbd->sock, SHUT_RDWR);
 	sockfd_put(nbd->sock);
 	nbd->sock = NULL;
-	spin_unlock_irq(&nbd->sock_lock);
-
+	mutex_unlock(&nbd->sock_lock);
 	del_timer(&nbd->timeout_timer);
 }
 
 static void nbd_xmit_timeout(unsigned long arg)
 {
 	struct nbd_device *nbd = (struct nbd_device *)arg;
-	unsigned long flags;
 
 	if (list_empty(&nbd->queue_head))
 		return;
 
-	spin_lock_irqsave(&nbd->sock_lock, flags);
-
-	nbd->timedout = true;
-
-	if (nbd->sock)
-		kernel_sock_shutdown(nbd->sock, SHUT_RDWR);
-
-	spin_unlock_irqrestore(&nbd->sock_lock, flags);
+	atomic_inc(&nbd->timedout);
+	wake_up(&nbd->waiting_wq);
 
 	dev_err(nbd_to_dev(nbd), "Connection timed out, shutting down connection\n");
 }
@@ -579,7 +570,27 @@ static int nbd_thread_send(void *data)
 		/* wait for something to do */
 		wait_event_interruptible(nbd->waiting_wq,
 					 kthread_should_stop() ||
-					 !list_empty(&nbd->waiting_queue));
+					 !list_empty(&nbd->waiting_queue) ||
+					 atomic_read(&nbd->timedout));
+
+		if (atomic_read(&nbd->timedout)) {
+			mutex_lock(&nbd->sock_lock);
+			if (nbd->sock) {
+				struct request sreq;
+
+				blk_rq_init(NULL, &sreq);
+				sreq.cmd_type = REQ_TYPE_DRV_PRIV;
+				mutex_lock(&nbd->tx_lock);
+				nbd->disconnect = true;
+				nbd_send_req(nbd, &sreq);
+				mutex_unlock(&nbd->tx_lock);
+				dev_err(disk_to_dev(nbd->disk),
+					"Device Timeout occured.Shutting down"
+					" socket.");
+			}
+			mutex_unlock(&nbd->sock_lock);
+			sock_shutdown(nbd);
+		}
 
 		/* extract request */
 		if (list_empty(&nbd->waiting_queue))
@@ -592,7 +603,11 @@ static int nbd_thread_send(void *data)
 		spin_unlock_irq(&nbd->queue_lock);
 
 		/* handle request */
-		nbd_handle_req(nbd, req);
+		if (atomic_read(&nbd->timedout)) {
+			req->errors++;
+			nbd_end_request(nbd, req);
+		} else
+			nbd_handle_req(nbd, req);
 	}
 
 	nbd->task_send = NULL;
@@ -647,7 +662,7 @@ static int nbd_set_socket(struct nbd_device *nbd, struct socket *sock)
 {
 	int ret = 0;
 
-	spin_lock_irq(&nbd->sock_lock);
+	mutex_lock(&nbd->sock_lock);
 
 	if (nbd->sock) {
 		ret = -EBUSY;
@@ -657,7 +672,7 @@ static int nbd_set_socket(struct nbd_device *nbd, struct socket *sock)
 	nbd->sock = sock;
 
 out:
-	spin_unlock_irq(&nbd->sock_lock);
+	mutex_unlock(&nbd->sock_lock);
 
 	return ret;
 }
@@ -666,7 +681,7 @@ out:
 static void nbd_reset(struct nbd_device *nbd)
 {
 	nbd->disconnect = false;
-	nbd->timedout = false;
+	atomic_set(&nbd->timedout, 0);
 	nbd->blksize = 1024;
 	nbd->bytesize = 0;
 	set_capacity(nbd->disk, 0);
@@ -803,17 +818,15 @@ static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd,
 		error = nbd_thread_recv(nbd, bdev);
 		nbd_dev_dbg_close(nbd);
 		kthread_stop(thread);
-
-		mutex_lock(&nbd->tx_lock);
-
 		sock_shutdown(nbd);
+		mutex_lock(&nbd->tx_lock);
 		nbd_clear_que(nbd);
 		kill_bdev(bdev);
 		nbd_bdev_reset(bdev);
 
 		if (nbd->disconnect) /* user requested, ignore socket errors */
 			error = 0;
-		if (nbd->timedout)
+		if (atomic_read(&nbd->timedout))
 			error = -ETIMEDOUT;
 
 		nbd_reset(nbd);
@@ -1075,7 +1088,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].sock_lock);
+		mutex_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

[toc] | [prev] | [next] | [standalone]

#1399840 — Re: [PATCH v4 01/18] nbd: Fix might_sleep warning on xmit timeout

[Multipart message — attachments visible in raw view] — view raw

Hi,

On Wednesday 11 May 2016 11:18:29 Pranay Kr. Srivastava wrote:
> This patch fixes the warning generated when a timeout occurs
> on the request and socket is closed from a non-sleep context
> by
> 
> 1. Moving the socket closing on a timeout to nbd_thread_send
> 
> 2. Make sock lock to be a mutex instead of a spin lock, since
>    nbd_xmit_timeout doesn't need to hold it anymore.

This patch seems quite big and complicated. Isn't the main issue that a
socket shutdown is called from within a spinlock?

When the issue got reported by Mikulas Patocka I created a patch but
forgot to send it, sorry. It is a bit simpler by simpling moving the
socket shutdown out of the spinlock. I will send it as reply. Please
have a look.

Thanks,

Markus

> 
> Signed-off-by: Pranay Kr. Srivastava <pranjas@gmail.com>
> ---
>  drivers/block/nbd.c | 65 ++++++++++++++++++++++++++++++++---------------------
>  1 file changed, 39 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
> index 31e73a7..c79bcd7 100644
> --- a/drivers/block/nbd.c
> +++ b/drivers/block/nbd.c
> @@ -57,12 +57,12 @@ struct nbd_device {
>  	int blksize;
>  	loff_t bytesize;
>  	int xmit_timeout;
> -	bool timedout;
> +	atomic_t timedout;
>  	bool disconnect; /* a disconnect has been requested by user */
>  
>  	struct timer_list timeout_timer;
>  	/* protects initialization and shutdown of the socket */
> -	spinlock_t sock_lock;
> +	struct mutex sock_lock;
>  	struct task_struct *task_recv;
>  	struct task_struct *task_send;
>  
> @@ -172,10 +172,9 @@ static void nbd_end_request(struct nbd_device *nbd, struct request *req)
>   */
>  static void sock_shutdown(struct nbd_device *nbd)
>  {
> -	spin_lock_irq(&nbd->sock_lock);
> -
> +	mutex_lock(&nbd->sock_lock);
>  	if (!nbd->sock) {
> -		spin_unlock_irq(&nbd->sock_lock);
> +		mutex_unlock(&nbd->sock_lock);
>  		return;
>  	}
>  
> @@ -183,27 +182,19 @@ static void sock_shutdown(struct nbd_device *nbd)
>  	kernel_sock_shutdown(nbd->sock, SHUT_RDWR);
>  	sockfd_put(nbd->sock);
>  	nbd->sock = NULL;
> -	spin_unlock_irq(&nbd->sock_lock);
> -
> +	mutex_unlock(&nbd->sock_lock);
>  	del_timer(&nbd->timeout_timer);
>  }
>  
>  static void nbd_xmit_timeout(unsigned long arg)
>  {
>  	struct nbd_device *nbd = (struct nbd_device *)arg;
> -	unsigned long flags;
>  
>  	if (list_empty(&nbd->queue_head))
>  		return;
>  
> -	spin_lock_irqsave(&nbd->sock_lock, flags);
> -
> -	nbd->timedout = true;
> -
> -	if (nbd->sock)
> -		kernel_sock_shutdown(nbd->sock, SHUT_RDWR);
> -
> -	spin_unlock_irqrestore(&nbd->sock_lock, flags);
> +	atomic_inc(&nbd->timedout);
> +	wake_up(&nbd->waiting_wq);
>  
>  	dev_err(nbd_to_dev(nbd), "Connection timed out, shutting down connection\n");
>  }
> @@ -579,7 +570,27 @@ static int nbd_thread_send(void *data)
>  		/* wait for something to do */
>  		wait_event_interruptible(nbd->waiting_wq,
>  					 kthread_should_stop() ||
> -					 !list_empty(&nbd->waiting_queue));
> +					 !list_empty(&nbd->waiting_queue) ||
> +					 atomic_read(&nbd->timedout));
> +
> +		if (atomic_read(&nbd->timedout)) {
> +			mutex_lock(&nbd->sock_lock);
> +			if (nbd->sock) {
> +				struct request sreq;
> +
> +				blk_rq_init(NULL, &sreq);
> +				sreq.cmd_type = REQ_TYPE_DRV_PRIV;
> +				mutex_lock(&nbd->tx_lock);
> +				nbd->disconnect = true;
> +				nbd_send_req(nbd, &sreq);
> +				mutex_unlock(&nbd->tx_lock);
> +				dev_err(disk_to_dev(nbd->disk),
> +					"Device Timeout occured.Shutting down"
> +					" socket.");
> +			}
> +			mutex_unlock(&nbd->sock_lock);
> +			sock_shutdown(nbd);
> +		}
>  
>  		/* extract request */
>  		if (list_empty(&nbd->waiting_queue))
> @@ -592,7 +603,11 @@ static int nbd_thread_send(void *data)
>  		spin_unlock_irq(&nbd->queue_lock);
>  
>  		/* handle request */
> -		nbd_handle_req(nbd, req);
> +		if (atomic_read(&nbd->timedout)) {
> +			req->errors++;
> +			nbd_end_request(nbd, req);
> +		} else
> +			nbd_handle_req(nbd, req);
>  	}
>  
>  	nbd->task_send = NULL;
> @@ -647,7 +662,7 @@ static int nbd_set_socket(struct nbd_device *nbd, struct socket *sock)
>  {
>  	int ret = 0;
>  
> -	spin_lock_irq(&nbd->sock_lock);
> +	mutex_lock(&nbd->sock_lock);
>  
>  	if (nbd->sock) {
>  		ret = -EBUSY;
> @@ -657,7 +672,7 @@ static int nbd_set_socket(struct nbd_device *nbd, struct socket *sock)
>  	nbd->sock = sock;
>  
>  out:
> -	spin_unlock_irq(&nbd->sock_lock);
> +	mutex_unlock(&nbd->sock_lock);
>  
>  	return ret;
>  }
> @@ -666,7 +681,7 @@ out:
>  static void nbd_reset(struct nbd_device *nbd)
>  {
>  	nbd->disconnect = false;
> -	nbd->timedout = false;
> +	atomic_set(&nbd->timedout, 0);
>  	nbd->blksize = 1024;
>  	nbd->bytesize = 0;
>  	set_capacity(nbd->disk, 0);
> @@ -803,17 +818,15 @@ static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd,
>  		error = nbd_thread_recv(nbd, bdev);
>  		nbd_dev_dbg_close(nbd);
>  		kthread_stop(thread);
> -
> -		mutex_lock(&nbd->tx_lock);
> -
>  		sock_shutdown(nbd);
> +		mutex_lock(&nbd->tx_lock);
>  		nbd_clear_que(nbd);
>  		kill_bdev(bdev);
>  		nbd_bdev_reset(bdev);
>  
>  		if (nbd->disconnect) /* user requested, ignore socket errors */
>  			error = 0;
> -		if (nbd->timedout)
> +		if (atomic_read(&nbd->timedout))
>  			error = -ETIMEDOUT;
>  
>  		nbd_reset(nbd);
> @@ -1075,7 +1088,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].sock_lock);
> +		mutex_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);
> 

-- 
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 |

[toc] | [prev] | [next] | [standalone]

#1398765 — [PATCH v4 07/18] nbd: fix checkpatch split string warning.

Signed-off-by: Pranay Kr. Srivastava <pranjas@gmail.com>
---
 drivers/block/nbd.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
index 6a4dc3a..7a5b8ef 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -587,8 +587,7 @@ static int nbd_thread_send(void *data)
 				nbd_send_req(nbd, &sreq);
 				mutex_unlock(&nbd->tx_lock);
 				dev_err(disk_to_dev(nbd->disk),
-					"Device Timeout occured.Shutting down"
-					" socket.");
+				"Device Timeout occured.Shutting down socket.");
 			}
 			mutex_unlock(&nbd->sock_lock);
 			sock_shutdown(nbd);
-- 
2.6.2

[toc] | [prev] | [next] | [standalone]

#1399754 — Re: [PATCH v4 07/18] nbd: fix checkpatch split string warning.

[Multipart message — attachments visible in raw view] — view raw

Hi,

On Wednesday 11 May 2016 11:18:35 Pranay Kr. Srivastava wrote:
> Signed-off-by: Pranay Kr. Srivastava <pranjas@gmail.com>
> ---
>  drivers/block/nbd.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
> index 6a4dc3a..7a5b8ef 100644
> --- a/drivers/block/nbd.c
> +++ b/drivers/block/nbd.c
> @@ -587,8 +587,7 @@ static int nbd_thread_send(void *data)
>  				nbd_send_req(nbd, &sreq);
>  				mutex_unlock(&nbd->tx_lock);
>  				dev_err(disk_to_dev(nbd->disk),
> -					"Device Timeout occured.Shutting down"
> -					" socket.");
> +				"Device Timeout occured.Shutting down socket.");

Please keep the indentation to the opening bracket here. Shouldn't this
create a checkpatch warning as well?

Regards,

Markus

>  			}
>  			mutex_unlock(&nbd->sock_lock);
>  			sock_shutdown(nbd);
> 

-- 
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 |

[toc] | [prev] | [next] | [standalone]

#1398766 — [PATCH v4 03/18] nbd: fix checkpatch warning use linux/uaccess.h

Signed-off-by: Pranay Kr. Srivastava <pranjas@gmail.com>
---
 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 82aac42..c7ccde7 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -35,7 +35,7 @@
 #include <linux/types.h>
 #include <linux/debugfs.h>
 
-#include <asm/uaccess.h>
+#include <linux/uaccess.h>
 #include <asm/types.h>
 
 #include <linux/nbd.h>
-- 
2.6.2

[toc] | [prev] | [next] | [standalone]

#1398767 — [PATCH v4 09/18] nbd: fix checkpatch trailing whitespace warning.

Signed-off-by: Pranay Kr. Srivastava <pranjas@gmail.com>
---
 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 224b44eb..2f1e5d0 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -627,7 +627,7 @@ static void nbd_request_handler(struct request_queue *q)
 		__releases(q->queue_lock) __acquires(q->queue_lock)
 {
 	struct request *req;
-	
+
 	while ((req = blk_fetch_request(q)) != NULL) {
 		struct nbd_device *nbd;
 
-- 
2.6.2

[toc] | [prev] | [next] | [standalone]

#1398768 — [PATCH v4 16/18] nbd: fix checkpatch no new line after decleration warning

Signed-off-by: Pranay Kr. Srivastava <pranjas@gmail.com>
---
 drivers/block/nbd.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
index 9ce350b..e308f8b 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -1086,6 +1086,7 @@ static int __init nbd_init(void)
 
 	for (i = 0; i < nbds_max; i++) {
 		struct gendisk *disk = nbd_dev[i].disk;
+
 		nbd_dev[i].magic = NBD_MAGIC;
 		INIT_LIST_HEAD(&nbd_dev[i].waiting_queue);
 		spin_lock_init(&nbd_dev[i].queue_lock);
-- 
2.6.2

[toc] | [prev] | [next] | [standalone]

#1398769 — [PATCH v4 15/18] nbd: fix checkpatch printk warning to pr_info

Signed-off-by: Pranay Kr. Srivastava <pranjas@gmail.com>
---
 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 6633ab2..9ce350b 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -1080,7 +1080,7 @@ static int __init nbd_init(void)
 		goto out;
 	}
 
-	printk(KERN_INFO "nbd: registered device at major %d\n", NBD_MAJOR);
+	pr_info("nbd: registered device at major %d\n", NBD_MAJOR);
 
 	nbd_dbg_init();
 
-- 
2.6.2

[toc] | [prev] | [next] | [standalone]

#1398770 — [PATCH v4 10/18] nbd: fix checkpatch trailing whitespace warning.

Signed-off-by: Pranay Kr. Srivastava <pranjas@gmail.com>
---
 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 2f1e5d0..0bc73dd 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -745,7 +745,7 @@ static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd,
 		nbd_send_req(nbd, &sreq);
 		return 0;
 	}
- 
+
 	case NBD_CLEAR_SOCK:
 		sock_shutdown(nbd);
 		nbd_clear_que(nbd);
-- 
2.6.2

[toc] | [prev] | [next] | [standalone]

#1398771 — [PATCH v4 18/18] make nbd device wait for its users in case of timeout

When a timeout occurs or a recv fails, then
instead of abruplty killing nbd block device
wait for it's users to finish.

This is more required when filesystem(s) like
ext2 or ext3 don't expect their buffer heads to
disappear while the filesystem is mounted.

The change is described below:
a) Add a users count to nbd_device structure.
b) Add a bit flag to nbd_device structure of unsigned long.

If the current user count is not 1 then make nbd-client wait
for the in_use bit to be cleared.

Signed-off-by: Pranay Kr. Srivastava <pranjas@gmail.com>
---
 drivers/block/nbd.c      | 40 ++++++++++++++++++++++++++++++++++++++++
 include/uapi/linux/nbd.h |  1 +
 2 files changed, 41 insertions(+)

diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
index 482a3c0..9b024d8 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -59,6 +59,7 @@ struct nbd_device {
 	int xmit_timeout;
 	atomic_t timedout;
 	bool disconnect; /* a disconnect has been requested by user */
+	u32 users;
 
 	struct timer_list timeout_timer;
 	/* protects initialization and shutdown of the socket */
@@ -69,6 +70,7 @@ struct nbd_device {
 #if IS_ENABLED(CONFIG_DEBUG_FS)
 	struct dentry *dbg_dir;
 #endif
+	unsigned long bflags;	/* word size bit flags for use. */
 };
 
 #if IS_ENABLED(CONFIG_DEBUG_FS)
@@ -822,6 +824,15 @@ static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd,
 		sock_shutdown(nbd);
 		mutex_lock(&nbd->tx_lock);
 		nbd_clear_que(nbd);
+		/*
+		 * Wait for any users currently using
+		 * this block device.
+		 */
+		mutex_unlock(&nbd->tx_lock);
+		pr_info("Waiting for users to release device %s ...\n",
+						bdev->bd_disk->disk_name);
+		wait_on_bit(&nbd->bflags, NBD_BFLAG_INUSE_BIT, TASK_INTERRUPTIBLE);
+		mutex_lock(&nbd->tx_lock);
 		kill_bdev(bdev);
 		nbd_bdev_reset(bdev);
 
@@ -870,10 +881,39 @@ static int nbd_ioctl(struct block_device *bdev, fmode_t mode,
 	return error;
 }
 
+static int nbd_open(struct block_device *bdev, fmode_t mode)
+{
+	struct nbd_device *nbd_dev = bdev->bd_disk->private_data;
+	nbd_dev->users++;
+	pr_debug("Opening nbd_dev %s. Active users = %u\n",
+			bdev->bd_disk->disk_name, nbd_dev->users);
+	if (nbd_dev->users > 1)
+	{
+		set_bit(NBD_BFLAG_INUSE_BIT, &nbd_dev->bflags);
+	}
+	return 0;
+}
+
+static void nbd_release(struct gendisk *disk, fmode_t mode)
+{
+	struct nbd_device *nbd_dev = disk->private_data;
+	nbd_dev->users--;
+	pr_debug("Closing nbd_dev %s. Active users = %u\n",
+			disk->disk_name, nbd_dev->users);
+	if (nbd_dev->users == 1)
+	{
+		clear_bit(NBD_BFLAG_INUSE_BIT, &nbd_dev->bflags);
+		smp_mb();
+		wake_up_bit(&nbd_dev->bflags, NBD_BFLAG_INUSE_BIT);
+	}
+}
+
 static const struct block_device_operations nbd_fops = {
 	.owner =	THIS_MODULE,
 	.ioctl =	nbd_ioctl,
 	.compat_ioctl =	nbd_ioctl,
+	.open = 	nbd_open,
+	.release = 	nbd_release
 };
 
 #if IS_ENABLED(CONFIG_DEBUG_FS)
diff --git a/include/uapi/linux/nbd.h b/include/uapi/linux/nbd.h
index e08e413..8f3d3f0 100644
--- a/include/uapi/linux/nbd.h
+++ b/include/uapi/linux/nbd.h
@@ -44,6 +44,7 @@ enum {
 /* there is a gap here to match userspace */
 #define NBD_FLAG_SEND_TRIM    (1 << 5) /* send trim/discard */
 
+#define NBD_BFLAG_INUSE_BIT	(1) /* bit number for bflags */
 /* userspace doesn't need the nbd_device structure */
 
 /* These are sent over the network in the request/reply magic fields */
-- 
2.6.2

[toc] | [prev] | [next] | [standalone]

#1399814 — Re: [PATCH v4 18/18] make nbd device wait for its users in case of timeout

[Multipart message — attachments visible in raw view] — view raw

Hi,

On Wednesday 11 May 2016 11:18:46 Pranay Kr. Srivastava wrote:
> When a timeout occurs or a recv fails, then
> instead of abruplty killing nbd block device
> wait for it's users to finish.
> 
> This is more required when filesystem(s) like
> ext2 or ext3 don't expect their buffer heads to
> disappear while the filesystem is mounted.
> 
> The change is described below:
> a) Add a users count to nbd_device structure.
> b) Add a bit flag to nbd_device structure of unsigned long.
> 
> If the current user count is not 1 then make nbd-client wait
> for the in_use bit to be cleared.

Thanks, I like this approach much more.

> 
> Signed-off-by: Pranay Kr. Srivastava <pranjas@gmail.com>
> ---
>  drivers/block/nbd.c      | 40 ++++++++++++++++++++++++++++++++++++++++
>  include/uapi/linux/nbd.h |  1 +
>  2 files changed, 41 insertions(+)
> 
> diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
> index 482a3c0..9b024d8 100644
> --- a/drivers/block/nbd.c
> +++ b/drivers/block/nbd.c
> @@ -59,6 +59,7 @@ struct nbd_device {
>  	int xmit_timeout;
>  	atomic_t timedout;
>  	bool disconnect; /* a disconnect has been requested by user */
> +	u32 users;

Perhaps it is better to use kref for this?

>  
>  	struct timer_list timeout_timer;
>  	/* protects initialization and shutdown of the socket */
> @@ -69,6 +70,7 @@ struct nbd_device {
>  #if IS_ENABLED(CONFIG_DEBUG_FS)
>  	struct dentry *dbg_dir;
>  #endif
> +	unsigned long bflags;	/* word size bit flags for use. */

Maybe it is better to use a completion instead of a bitfield.

>  };
>  
>  #if IS_ENABLED(CONFIG_DEBUG_FS)
> @@ -822,6 +824,15 @@ static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd,
>  		sock_shutdown(nbd);
>  		mutex_lock(&nbd->tx_lock);
>  		nbd_clear_que(nbd);
> +		/*
> +		 * Wait for any users currently using
> +		 * this block device.
> +		 */
> +		mutex_unlock(&nbd->tx_lock);
> +		pr_info("Waiting for users to release device %s ...\n",
> +						bdev->bd_disk->disk_name);
> +		wait_on_bit(&nbd->bflags, NBD_BFLAG_INUSE_BIT, TASK_INTERRUPTIBLE);
> +		mutex_lock(&nbd->tx_lock);
>  		kill_bdev(bdev);
>  		nbd_bdev_reset(bdev);
>  
> @@ -870,10 +881,39 @@ static int nbd_ioctl(struct block_device *bdev, fmode_t mode,
>  	return error;
>  }
>  
> +static int nbd_open(struct block_device *bdev, fmode_t mode)
> +{
> +	struct nbd_device *nbd_dev = bdev->bd_disk->private_data;

Here is a new line missing otherwise checkpatch will probably warn about
this?

Should we check here if we are connected here? And check whether the
connection is about to be closed?

Best Regards,

Markus

> +	nbd_dev->users++;
> +	pr_debug("Opening nbd_dev %s. Active users = %u\n",
> +			bdev->bd_disk->disk_name, nbd_dev->users);
> +	if (nbd_dev->users > 1)
> +	{
> +		set_bit(NBD_BFLAG_INUSE_BIT, &nbd_dev->bflags);
> +	}
> +	return 0;
> +}
> +
> +static void nbd_release(struct gendisk *disk, fmode_t mode)
> +{
> +	struct nbd_device *nbd_dev = disk->private_data;
> +	nbd_dev->users--;
> +	pr_debug("Closing nbd_dev %s. Active users = %u\n",
> +			disk->disk_name, nbd_dev->users);
> +	if (nbd_dev->users == 1)
> +	{
> +		clear_bit(NBD_BFLAG_INUSE_BIT, &nbd_dev->bflags);
> +		smp_mb();
> +		wake_up_bit(&nbd_dev->bflags, NBD_BFLAG_INUSE_BIT);
> +	}
> +}
> +
>  static const struct block_device_operations nbd_fops = {
>  	.owner =	THIS_MODULE,
>  	.ioctl =	nbd_ioctl,
>  	.compat_ioctl =	nbd_ioctl,
> +	.open = 	nbd_open,
> +	.release = 	nbd_release
>  };
>  
>  #if IS_ENABLED(CONFIG_DEBUG_FS)
> diff --git a/include/uapi/linux/nbd.h b/include/uapi/linux/nbd.h
> index e08e413..8f3d3f0 100644
> --- a/include/uapi/linux/nbd.h
> +++ b/include/uapi/linux/nbd.h
> @@ -44,6 +44,7 @@ enum {
>  /* there is a gap here to match userspace */
>  #define NBD_FLAG_SEND_TRIM    (1 << 5) /* send trim/discard */
>  
> +#define NBD_BFLAG_INUSE_BIT	(1) /* bit number for bflags */
>  /* userspace doesn't need the nbd_device structure */
>  
>  /* These are sent over the network in the request/reply magic fields */
> 

-- 
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 |

[toc] | [prev] | [next] | [standalone]

#1398772 — [PATCH v4 11/18] nbd : fix checkpatch structure declaration braces on next line warning.

Signed-off-by: Pranay Kr. Srivastava <pranjas@gmail.com>
---
 drivers/block/nbd.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
index 0bc73dd..a6f11c3 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -870,8 +870,7 @@ static int nbd_ioctl(struct block_device *bdev, fmode_t mode,
 	return error;
 }
 
-static const struct block_device_operations nbd_fops =
-{
+static const struct block_device_operations nbd_fops = {
 	.owner =	THIS_MODULE,
 	.ioctl =	nbd_ioctl,
 	.compat_ioctl =	nbd_ioctl,
-- 
2.6.2

[toc] | [prev] | [next] | [standalone]

#1398775 — [PATCH v4 06/18] nbd: fix checkpatch warning no newline after decleration.

Signed-off-by: Pranay Kr. Srivastava <pranjas@gmail.com>
---
 drivers/block/nbd.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
index 2192c0e..6a4dc3a 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -355,6 +355,7 @@ static inline int sock_recv_bvec(struct nbd_device *nbd, struct bio_vec *bvec)
 {
 	int result;
 	void *kaddr = kmap(bvec->bv_page);
+
 	result = sock_xmit(nbd, 0, kaddr + bvec->bv_offset, bvec->bv_len,
 			MSG_WAITALL);
 	kunmap(bvec->bv_page);
-- 
2.6.2

[toc] | [prev] | [next] | [standalone]

#1398776 — [PATCH v4 17/18] nbd: fix checkpatch printk warning to pr_info

Signed-off-by: Pranay Kr. Srivastava <pranjas@gmail.com>
---
 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 e308f8b..482a3c0 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -1134,7 +1134,7 @@ static void __exit nbd_cleanup(void)
 	}
 	unregister_blkdev(NBD_MAJOR, "nbd");
 	kfree(nbd_dev);
-	printk(KERN_INFO "nbd: unregistered device at major %d\n", NBD_MAJOR);
+	pr_info("nbd: unregistered device at major %d\n", NBD_MAJOR);
 }
 
 module_init(nbd_init);
-- 
2.6.2

[toc] | [prev] | [next] | [standalone]

#1398777 — [PATCH v4 14/18] nbd: fix checkpatch no extra line after decleration warning

Signed-off-by: Pranay Kr. Srivastava <pranjas@gmail.com>
---
 drivers/block/nbd.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
index 690e734..6633ab2 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -1049,6 +1049,7 @@ static int __init nbd_init(void)
 
 	for (i = 0; i < nbds_max; i++) {
 		struct gendisk *disk = alloc_disk(1 << part_shift);
+
 		if (!disk)
 			goto out;
 		nbd_dev[i].disk = disk;
-- 
2.6.2

[toc] | [prev] | [next] | [standalone]

#1398783 — [PATCH v4 13/18] nbd : fix checkpatch printk warning

Signed-off-by: Pranay Kr. Srivastava <pranjas@gmail.com>
---
 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 4fd3016..690e734 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -1018,7 +1018,7 @@ static int __init nbd_init(void)
 	BUILD_BUG_ON(sizeof(struct nbd_request) != 28);
 
 	if (max_part < 0) {
-		printk(KERN_ERR "nbd: max_part must be >= 0\n");
+		pr_err("nbd: max_part must be >= 0\n");
 		return -EINVAL;
 	}
 
-- 
2.6.2

[toc] | [prev] | [next] | [standalone]

#1398785 — [PATCH v4 12/18] nbd : fix checkpatch trailing whitespace warning

Signed-off-by: Pranay Kr. Srivastava <pranjas@gmail.com>
---
 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 a6f11c3..4fd3016 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -1005,7 +1005,7 @@ static void nbd_dbg_close(void)
 #endif
 
 /*
- * And here should be modules and kernel interface 
+ * And here should be modules and kernel interface
  *  (Just smiley confuses emacs :-)
  */
 
-- 
2.6.2

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web