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

Groups > linux.kernel > #1392728

Re: [RESEND PATCH]nbd: fix might_sleep warning on socket shutdown

Show all headers | View raw

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

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[RESEND PATCH]nbd: Fix might sleep warning. "Pranay Kr. Srivastava" <pranjas@gmail.com> - 2016-04-30 08:20 +0200
  [RESEND PATCH]nbd: fix might_sleep warning on socket shutdown "Pranay Kr. Srivastava" <pranjas@gmail.com> - 2016-04-30 08:30 +0200
    Re: [RESEND PATCH]nbd: fix might_sleep warning on socket shutdown Pranay Srivastava <pranjas@gmail.com> - 2016-05-02 05:30 +0200
      Re: [RESEND PATCH]nbd: fix might_sleep warning on socket shutdown Greg KH <gregkh@linuxfoundation.org> - 2016-05-03 03:00 +0200
        Re: [RESEND PATCH]nbd: fix might_sleep warning on socket shutdown Pranay Srivastava <pranjas@gmail.com> - 2016-05-03 06:00 +0200

csiph-web