On Tue, May 3, 2016 at 5:45 AM, Greg KH <[email protected]> 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
>> <[email protected]> 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 <[email protected]>
>> > * Parts copyright 2001 Steven Whitehouse <[email protected]>
>> > *
>> > @@ -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
------------------------------------------------------------------------------
Find and fix application performance issues faster with Applications Manager
Applications Manager provides deep performance insights into multiple tiers of
your business applications. It resolves application problems quickly and
reduces your MTTR. Get your free trial!
https://ad.doubleclick.net/ddm/clk/302982198;130105516;z
_______________________________________________
Nbd-general mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/nbd-general
