Hello, On Thu, Jun 9, 2016 at 3:33 PM, Pranay Srivastava <pran...@gmail.com> wrote: > > Hello > > > On Thu, Jun 2, 2016 at 3:54 PM, Pranay Kr. Srivastava <pran...@gmail.com> > 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 <miku...@twibright.com> > > Signed-off-by: Markus Pargmann <m...@pengutronix.de> > > > > Changelog: > > Pranay Kr. Srivastava<pran...@gmail.com>: > > > > 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 <pran...@gmail.com> > > --- > > 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 <asm/types.h> > > > > #include <linux/nbd.h> > > +#include <linux/workqueue.h> > > > > 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