From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1424027AbcFNFNd (ORCPT ); Tue, 14 Jun 2016 01:13:33 -0400 Received: from mail-wm0-f68.google.com ([74.125.82.68]:33839 "EHLO mail-wm0-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1422783AbcFNFNc (ORCPT ); Tue, 14 Jun 2016 01:13:32 -0400 MIME-Version: 1.0 In-Reply-To: References: <3898019.JRqDjBPssX@adelgunde> <1464863101-16805-1-git-send-email-pranjas@gmail.com> <1464863101-16805-2-git-send-email-pranjas@gmail.com> From: Pranay Srivastava Date: Tue, 14 Jun 2016 10:43:28 +0530 Message-ID: Subject: Re: [PATCH v2 1/5] nbd: fix might_sleep warning on socket shutdown. To: Markus Pargmann , nbd-general@lists.sourceforge.net, linux-kernel@vger.kernel.org Cc: "Pranay Kr. Srivastava" Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hello, On Thu, Jun 9, 2016 at 3:33 PM, Pranay Srivastava wrote: > > Hello > > > On Thu, Jun 2, 2016 at 3:54 PM, Pranay Kr. Srivastava wrote: > > spinlocked ranges should be small and not contain calls into huge > > subfunctions. Fix my mistake and just get the pointer to the socket > > instead of doing everything with spinlock held. > > > > Reported-by: Mikulas Patocka > > Signed-off-by: Markus Pargmann > > > > Changelog: > > Pranay Kr. Srivastava: > > > > 1) Use spin_lock instead of irq version for sock_shutdown. > > > > 2) Use system work queue to actually trigger the shutdown of > > socket. This solves the issue when kernel_sendmsg is currently > > blocked while a timeout occurs. > > > > Signed-off-by: Pranay Kr. Srivastava > > --- > > drivers/block/nbd.c | 65 ++++++++++++++++++++++++++++++++++------------------- > > 1 file changed, 42 insertions(+), 23 deletions(-) > > > > diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c > > index 31e73a7..0339d40 100644 > > --- a/drivers/block/nbd.c > > +++ b/drivers/block/nbd.c > > @@ -39,6 +39,7 @@ > > #include > > > > #include > > +#include > > > > struct nbd_device { > > u32 flags; > > @@ -69,6 +70,10 @@ struct nbd_device { > > #if IS_ENABLED(CONFIG_DEBUG_FS) > > struct dentry *dbg_dir; > > #endif > > + /* > > + *This is specifically for calling sock_shutdown, for now. > > + */ > > + struct work_struct ws_shutdown; > > }; > > > > #if IS_ENABLED(CONFIG_DEBUG_FS) > > @@ -95,6 +100,11 @@ static int max_part; > > */ > > static DEFINE_SPINLOCK(nbd_lock); > > > > +/* > > + * Shutdown function for nbd_dev work struct. > > + */ > > +static void nbd_ws_func_shutdown(struct work_struct *); > > + > > static inline struct device *nbd_to_dev(struct nbd_device *nbd) > > { > > return disk_to_dev(nbd->disk); > > @@ -172,39 +182,35 @@ 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); > > - > > - if (!nbd->sock) { > > - spin_unlock_irq(&nbd->sock_lock); > > - return; > > - } > > + struct socket *sock; > > > > - dev_warn(disk_to_dev(nbd->disk), "shutting down socket\n"); > > - kernel_sock_shutdown(nbd->sock, SHUT_RDWR); > > - sockfd_put(nbd->sock); > > + spin_lock(&nbd->sock_lock); > > + sock = nbd->sock; > > nbd->sock = NULL; > > - spin_unlock_irq(&nbd->sock_lock); > > + spin_unlock(&nbd->sock_lock); > > + > > + if (!sock) > > + return; > > > > del_timer(&nbd->timeout_timer); > > + dev_warn(disk_to_dev(nbd->disk), "shutting down socket\n"); > > + kernel_sock_shutdown(sock, SHUT_RDWR); > > + sockfd_put(sock); > > } > > > > 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); > > - > > + schedule_work(&nbd->ws_shutdown); > > + /* > > + * Make sure sender thread sees nbd->timedout. > > + */ > > + smp_wmb(); > > + wake_up(&nbd->waiting_wq); > > dev_err(nbd_to_dev(nbd), "Connection timed out, shutting down connection\n"); > > } > > > > @@ -592,7 +598,11 @@ static int nbd_thread_send(void *data) > > spin_unlock_irq(&nbd->queue_lock); > > > > /* handle request */ > > - nbd_handle_req(nbd, req); > > + if (nbd->timedout) { > > + req->errors++; > > + nbd_end_request(nbd, req); > > + } else > > + nbd_handle_req(nbd, req); > > } > > > > nbd->task_send = NULL; > > @@ -672,6 +682,7 @@ static void nbd_reset(struct nbd_device *nbd) > > set_capacity(nbd->disk, 0); > > nbd->flags = 0; > > nbd->xmit_timeout = 0; > > + INIT_WORK(&nbd->ws_shutdown, nbd_ws_func_shutdown); > > queue_flag_clear_unlocked(QUEUE_FLAG_DISCARD, nbd->disk->queue); > > del_timer_sync(&nbd->timeout_timer); > > } > > @@ -804,15 +815,15 @@ static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd, > > 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) > > error = -ETIMEDOUT; > > > > @@ -863,6 +874,14 @@ static const struct block_device_operations nbd_fops = > > .compat_ioctl = nbd_ioctl, > > }; > > > > +static void nbd_ws_func_shutdown(struct work_struct *ws_nbd) > > +{ > > + struct nbd_device *nbd_dev = container_of(ws_nbd, struct nbd_device, > > + ws_shutdown); > > + > > + sock_shutdown(nbd_dev); > > +} > > + > > #if IS_ENABLED(CONFIG_DEBUG_FS) > > > > static int nbd_dbg_tasks_show(struct seq_file *s, void *unused) > > -- > > 2.6.2 > > > > > Any update for the above patch series? > -- > ---P.K.S ping?? -- ---P.K.S