Re: [PATCH -mm 3/3] nbd: don't abuse irqsave/irqrestore while taking nbd->tasks_lock

[Markus Pargmann]

On Sun, Oct 25, 2015 at 04:26:41PM +0100, Oleg Nesterov wrote:
> Both nbd_thread_recv() and nbd_thread_send() are might_sleep() and
> called with irqs enabled(), irqsave/irqrestore make no sense and imo
> look confusing.

Thanks, applied.

Regards,

Markus

> 
> Signed-off-by: Oleg Nesterov <oleg@xxxxxxxxxx>
> ---
>  drivers/block/nbd.c |   22 ++++++++++------------
>  1 files changed, 10 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
> index 0ffd73c..fd79405 100644
> --- a/drivers/block/nbd.c
> +++ b/drivers/block/nbd.c
> @@ -406,23 +406,22 @@ 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);
> +	spin_lock_irq(&nbd->tasks_lock);
>  	nbd->task_recv = current;
> -	spin_unlock_irqrestore(&nbd->tasks_lock, flags);
> +	spin_unlock_irq(&nbd->tasks_lock);
>  
>  	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);
> +		spin_lock_irq(&nbd->tasks_lock);
>  		nbd->task_recv = NULL;
> -		spin_unlock_irqrestore(&nbd->tasks_lock, flags);
> +		spin_unlock_irq(&nbd->tasks_lock);
>  
>  		return ret;
>  	}
> @@ -439,9 +438,9 @@ 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);
> +	spin_lock_irq(&nbd->tasks_lock);
>  	nbd->task_recv = NULL;
> -	spin_unlock_irqrestore(&nbd->tasks_lock, flags);
> +	spin_unlock_irq(&nbd->tasks_lock);
>  
>  	if (signal_pending(current)) {
>  		dev_warn(nbd_to_dev(nbd), "pid %d, %s, got signal",
> @@ -543,11 +542,10 @@ 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);
> +	spin_lock_irq(&nbd->tasks_lock);
>  	nbd->task_send = current;
> -	spin_unlock_irqrestore(&nbd->tasks_lock, flags);
> +	spin_unlock_irq(&nbd->tasks_lock);
>  
>  	set_user_nice(current, MIN_NICE);
>  	while (!kthread_should_stop() || !list_empty(&nbd->waiting_queue)) {
> @@ -581,9 +579,9 @@ static int nbd_thread_send(void *data)
>  		nbd_handle_req(nbd, req);
>  	}
>  
> -	spin_lock_irqsave(&nbd->tasks_lock, flags);
> +	spin_lock_irq(&nbd->tasks_lock);
>  	nbd->task_send = NULL;
> -	spin_unlock_irqrestore(&nbd->tasks_lock, flags);
> +	spin_unlock_irq(&nbd->tasks_lock);
>  
>  	return 0;
>  }
> -- 
> 1.5.5.1
> 
> 

-- 
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: signature.asc
Description: PGP signature