From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S934079AbcECAyp (ORCPT ); Mon, 2 May 2016 20:54:45 -0400 Received: from mail.linuxfoundation.org ([140.211.169.12]:33276 "EHLO mail.linuxfoundation.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933880AbcECAyM (ORCPT ); Mon, 2 May 2016 20:54:12 -0400 Date: Mon, 2 May 2016 17:15:59 -0700 From: Greg KH To: Pranay Srivastava Cc: mpa@pengutronix.de, nbd-general@lists.sourceforge.net, linux-kernel@vger.kernel.org Subject: Re: [RESEND PATCH]nbd: fix might_sleep warning on socket shutdown Message-ID: <20160503001559.GA29942@kroah.com> References: <1461997146-6185-1-git-send-email-pranjas@gmail.com> <1461997146-6185-2-git-send-email-pranjas@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.6.0 (2016-04-01) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 > 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 > > * Parts copyright 2001 Steven Whitehouse > > * > > @@ -35,14 +35,14 @@ > > #include > > #include > > > > -#include > > +#include Why change this? > > #include > > > > #include > > > > 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