* [RESEND PATCH]nbd: Fix might sleep warning. @ 2016-04-30 6:19 Pranay Kr. Srivastava 2016-04-30 6:19 ` [RESEND PATCH]nbd: fix might_sleep warning on socket shutdown Pranay Kr. Srivastava 0 siblings, 1 reply; 5+ messages in thread From: Pranay Kr. Srivastava @ 2016-04-30 6:19 UTC (permalink / raw) To: mpa, nbd-general, linux-kernel, gregkh; +Cc: Pranay Kr. Srivastava When an nbd request times out then the nbd_xmit_timeout tries to close the socket by taking a spin_lock over the socket. This however generates a warning on kernel_sock_shutdown. This patch fixes this issue. Pranay Kr. Srivastava (1): fix might_sleep warning on socket shutdown drivers/block/nbd.c | 85 +++++++++++++++++++++++++++++++---------------------- 1 file changed, 50 insertions(+), 35 deletions(-) -- 2.6.2 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [RESEND PATCH]nbd: fix might_sleep warning on socket shutdown 2016-04-30 6:19 [RESEND PATCH]nbd: Fix might sleep warning Pranay Kr. Srivastava @ 2016-04-30 6:19 ` Pranay Kr. Srivastava 2016-05-02 3:28 ` Pranay Srivastava 0 siblings, 1 reply; 5+ messages in thread From: Pranay Kr. Srivastava @ 2016-04-30 6:19 UTC (permalink / raw) To: mpa, nbd-general, linux-kernel, gregkh; +Cc: Pranay Kr. Srivastava 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. 3. Move sock_shutdown outside the tx_lock in NBD_DO_IT. --- drivers/block/nbd.c | 85 +++++++++++++++++++++++++++++++---------------------- 1 file changed, 50 insertions(+), 35 deletions(-) diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c index 31e73a7..a52cc16 100644 --- a/drivers/block/nbd.c +++ b/drivers/block/nbd.c @@ -3,7 +3,7 @@ * * Note that you can not swap over this thing, yet. Seems to work but * deadlocks sometimes - you can not swap over TCP in general. - * + * * Copyright 1997-2000, 2008 Pavel Machek <pavel@ucw.cz> * Parts copyright 2001 Steven Whitehouse <steve@chygwyn.com> * @@ -35,14 +35,14 @@ #include <linux/types.h> #include <linux/debugfs.h> -#include <asm/uaccess.h> +#include <linux/uaccess.h> #include <asm/types.h> #include <linux/nbd.h> struct nbd_device { u32 flags; - struct socket * sock; /* If == NULL, device is not ready, yet */ + struct socket *sock; /* If == NULL, device is not ready, yet */ int magic; spinlock_t queue_lock; @@ -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"); } @@ -266,6 +257,7 @@ static inline int sock_send_bvec(struct nbd_device *nbd, struct bio_vec *bvec, { int result; void *kaddr = kmap(bvec->bv_page); + result = sock_xmit(nbd, 1, kaddr + bvec->bv_offset, bvec->bv_len, flags); kunmap(bvec->bv_page); @@ -278,6 +270,7 @@ static int nbd_send_req(struct nbd_device *nbd, struct request *req) int result, flags; struct nbd_request request; unsigned long size = blk_rq_bytes(req); + u32 type; if (req->cmd_type == REQ_TYPE_DRV_PRIV) @@ -363,6 +356,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); @@ -579,7 +573,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 +606,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; @@ -601,8 +619,8 @@ static int nbd_thread_send(void *data) } /* - * We always wait for result of write, for now. It would be nice to make it optional - * in future + * We always wait for result of write, for now. + * It would be nice to make it optional in future * if ((rq_data_dir(req) == WRITE) && (nbd->flags & NBD_WRITE_NOCHK)) * { printk( "Warning: Ignoring result!\n"); nbd_end_request( req ); } */ @@ -611,7 +629,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; @@ -647,7 +665,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 +675,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 +684,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); @@ -729,7 +747,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); @@ -803,17 +821,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); @@ -856,8 +872,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, @@ -1075,7 +1090,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 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [RESEND PATCH]nbd: fix might_sleep warning on socket shutdown 2016-04-30 6:19 ` [RESEND PATCH]nbd: fix might_sleep warning on socket shutdown Pranay Kr. Srivastava @ 2016-05-02 3:28 ` Pranay Srivastava 2016-05-03 0:15 ` Greg KH 0 siblings, 1 reply; 5+ messages in thread From: Pranay Srivastava @ 2016-05-02 3:28 UTC (permalink / raw) To: mpa, nbd-general, linux-kernel, gregkh; +Cc: Pranay Kr. Srivastava Hi, Can the following patch be reviewed? I'm working on some more changes on top of this change, so it'll be really helpful if someone can review this patch and let me know of shortcomings/issues with this. On Sat, Apr 30, 2016 at 11:49 AM, Pranay Kr. Srivastava <pranjas@gmail.com> 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. > > 3. Move sock_shutdown outside the tx_lock in NBD_DO_IT. > --- > drivers/block/nbd.c | 85 +++++++++++++++++++++++++++++++---------------------- > 1 file changed, 50 insertions(+), 35 deletions(-) > > diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c > index 31e73a7..a52cc16 100644 > --- a/drivers/block/nbd.c > +++ b/drivers/block/nbd.c > @@ -3,7 +3,7 @@ > * > * Note that you can not swap over this thing, yet. Seems to work but > * deadlocks sometimes - you can not swap over TCP in general. > - * > + * > * Copyright 1997-2000, 2008 Pavel Machek <pavel@ucw.cz> > * Parts copyright 2001 Steven Whitehouse <steve@chygwyn.com> > * > @@ -35,14 +35,14 @@ > #include <linux/types.h> > #include <linux/debugfs.h> > > -#include <asm/uaccess.h> > +#include <linux/uaccess.h> > #include <asm/types.h> > > #include <linux/nbd.h> > > struct nbd_device { > u32 flags; > - struct socket * sock; /* If == NULL, device is not ready, yet */ > + struct socket *sock; /* If == NULL, device is not ready, yet */ > int magic; > > spinlock_t queue_lock; > @@ -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"); > } > @@ -266,6 +257,7 @@ static inline int sock_send_bvec(struct nbd_device *nbd, struct bio_vec *bvec, > { > int result; > void *kaddr = kmap(bvec->bv_page); > + > result = sock_xmit(nbd, 1, kaddr + bvec->bv_offset, > bvec->bv_len, flags); > kunmap(bvec->bv_page); > @@ -278,6 +270,7 @@ static int nbd_send_req(struct nbd_device *nbd, struct request *req) > int result, flags; > struct nbd_request request; > unsigned long size = blk_rq_bytes(req); > + > u32 type; > > if (req->cmd_type == REQ_TYPE_DRV_PRIV) > @@ -363,6 +356,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); > @@ -579,7 +573,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 +606,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; > @@ -601,8 +619,8 @@ static int nbd_thread_send(void *data) > } > > /* > - * We always wait for result of write, for now. It would be nice to make it optional > - * in future > + * We always wait for result of write, for now. > + * It would be nice to make it optional in future > * if ((rq_data_dir(req) == WRITE) && (nbd->flags & NBD_WRITE_NOCHK)) > * { printk( "Warning: Ignoring result!\n"); nbd_end_request( req ); } > */ > @@ -611,7 +629,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; > > @@ -647,7 +665,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 +675,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 +684,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); > @@ -729,7 +747,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); > @@ -803,17 +821,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); > @@ -856,8 +872,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, > @@ -1075,7 +1090,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 > -- ---P.K.S ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [RESEND PATCH]nbd: fix might_sleep warning on socket shutdown 2016-05-02 3:28 ` Pranay Srivastava @ 2016-05-03 0:15 ` Greg KH 2016-05-03 3:55 ` Pranay Srivastava 0 siblings, 1 reply; 5+ messages in thread From: Greg KH @ 2016-05-03 0:15 UTC (permalink / raw) To: Pranay Srivastava; +Cc: mpa, nbd-general, linux-kernel On Mon, May 02, 2016 at 08:58:34AM +0530, Pranay Srivastava wrote: > Hi, > > Can the following patch be reviewed? I'm working on some more changes > on top of this change, > so it'll be really helpful if someone can review this patch and let me > know of shortcomings/issues > with this. > > > On Sat, Apr 30, 2016 at 11:49 AM, Pranay Kr. Srivastava > <pranjas@gmail.com> 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. > > > > 3. Move sock_shutdown outside the tx_lock in NBD_DO_IT. Why are you doing three things in one patch? > > --- > > drivers/block/nbd.c | 85 +++++++++++++++++++++++++++++++---------------------- > > 1 file changed, 50 insertions(+), 35 deletions(-) > > > > diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c > > index 31e73a7..a52cc16 100644 > > --- a/drivers/block/nbd.c > > +++ b/drivers/block/nbd.c > > @@ -3,7 +3,7 @@ > > * > > * Note that you can not swap over this thing, yet. Seems to work but > > * deadlocks sometimes - you can not swap over TCP in general. > > - * > > + * What did you change here? > > * Copyright 1997-2000, 2008 Pavel Machek <pavel@ucw.cz> > > * Parts copyright 2001 Steven Whitehouse <steve@chygwyn.com> > > * > > @@ -35,14 +35,14 @@ > > #include <linux/types.h> > > #include <linux/debugfs.h> > > > > -#include <asm/uaccess.h> > > +#include <linux/uaccess.h> Why change this? > > #include <asm/types.h> > > > > #include <linux/nbd.h> > > > > struct nbd_device { > > u32 flags; > > - struct socket * sock; /* If == NULL, device is not ready, yet */ > > + struct socket *sock; /* If == NULL, device is not ready, yet */ You are mixing "code cleanup" changes with "change the logic" changes, please break this up into a series of patches, doing the code cleanup _last_ as you want to fix bugs first. thanks, greg k-h ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [RESEND PATCH]nbd: fix might_sleep warning on socket shutdown 2016-05-03 0:15 ` Greg KH @ 2016-05-03 3:55 ` Pranay Srivastava 0 siblings, 0 replies; 5+ messages in thread From: Pranay Srivastava @ 2016-05-03 3:55 UTC (permalink / raw) To: Greg KH; +Cc: mpa, nbd-general, linux-kernel On Tue, May 3, 2016 at 5:45 AM, Greg KH <gregkh@linuxfoundation.org> wrote: > On Mon, May 02, 2016 at 08:58:34AM +0530, Pranay Srivastava wrote: >> Hi, >> >> Can the following patch be reviewed? I'm working on some more changes >> on top of this change, >> so it'll be really helpful if someone can review this patch and let me >> know of shortcomings/issues >> with this. >> >> >> On Sat, Apr 30, 2016 at 11:49 AM, Pranay Kr. Srivastava >> <pranjas@gmail.com> 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. >> > >> > 3. Move sock_shutdown outside the tx_lock in NBD_DO_IT. > > Why are you doing three things in one patch? > >> > --- >> > drivers/block/nbd.c | 85 +++++++++++++++++++++++++++++++---------------------- >> > 1 file changed, 50 insertions(+), 35 deletions(-) >> > >> > diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c >> > index 31e73a7..a52cc16 100644 >> > --- a/drivers/block/nbd.c >> > +++ b/drivers/block/nbd.c >> > @@ -3,7 +3,7 @@ >> > * >> > * Note that you can not swap over this thing, yet. Seems to work but >> > * deadlocks sometimes - you can not swap over TCP in general. >> > - * >> > + * > > What did you change here? > >> > * Copyright 1997-2000, 2008 Pavel Machek <pavel@ucw.cz> >> > * Parts copyright 2001 Steven Whitehouse <steve@chygwyn.com> >> > * >> > @@ -35,14 +35,14 @@ >> > #include <linux/types.h> >> > #include <linux/debugfs.h> >> > >> > -#include <asm/uaccess.h> >> > +#include <linux/uaccess.h> > > Why change this? > >> > #include <asm/types.h> >> > >> > #include <linux/nbd.h> >> > >> > struct nbd_device { >> > u32 flags; >> > - struct socket * sock; /* If == NULL, device is not ready, yet */ >> > + struct socket *sock; /* If == NULL, device is not ready, yet */ > > You are mixing "code cleanup" changes with "change the logic" changes, > please break this up into a series of patches, doing the code cleanup > _last_ as you want to fix bugs first. > Ok. Will do that. > thanks, > > greg k-h Thanks alot Greg for review. -- ---P.K.S ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-05-03 3:55 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-04-30 6:19 [RESEND PATCH]nbd: Fix might sleep warning Pranay Kr. Srivastava 2016-04-30 6:19 ` [RESEND PATCH]nbd: fix might_sleep warning on socket shutdown Pranay Kr. Srivastava 2016-05-02 3:28 ` Pranay Srivastava 2016-05-03 0:15 ` Greg KH 2016-05-03 3:55 ` Pranay Srivastava
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox