[PATCH net-next] usbnet: fix cyclical race on disconnect with work queue

[PATCH net-next] usbnet: fix cyclical race on disconnect with work queue

[Oliver Neukum]

The work can submit URBs and the URBs can schedule the work.
This cycle needs to be broken, when a device is to be stopped.
Use a flag to do so.

Fixes: f29fc259976e9 ("[PATCH] USB: usbnet (1/9) clean up framing")
Reported-by: syzbot+9665bf55b1c828bbcd8a@syzkaller.appspotmail.com
Signed-off-by: Oliver Neukum <oneukum@suse.com>
---
 drivers/net/usb/usbnet.c   | 37 ++++++++++++++++++++++++++++---------
 include/linux/usb/usbnet.h | 18 ++++++++++++++++++
 2 files changed, 46 insertions(+), 9 deletions(-)

diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index e84efa661589..422d91635045 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -467,10 +467,12 @@ static enum skb_state defer_bh(struct usbnet *dev, struct sk_buff *skb,
 void usbnet_defer_kevent (struct usbnet *dev, int work)
 {
 	set_bit (work, &dev->flags);
-	if (!schedule_work (&dev->kevent))
-		netdev_dbg(dev->net, "kevent %s may have been dropped\n", usbnet_event_names[work]);
-	else
-		netdev_dbg(dev->net, "kevent %s scheduled\n", usbnet_event_names[work]);
+	if (!usbnet_going_away(dev)) {
+		if (!schedule_work (&dev->kevent))
+			netdev_dbg(dev->net, "kevent %s may have been dropped\n", usbnet_event_names[work]);
+		else
+			netdev_dbg(dev->net, "kevent %s scheduled\n", usbnet_event_names[work]);
+	}
 }
 EXPORT_SYMBOL_GPL(usbnet_defer_kevent);
 
@@ -538,7 +540,8 @@ static int rx_submit (struct usbnet *dev, struct urb *urb, gfp_t flags)
 			tasklet_schedule (&dev->bh);
 			break;
 		case 0:
-			__usbnet_queue_skb(&dev->rxq, skb, rx_start);
+			if (!usbnet_going_away(dev))
+				__usbnet_queue_skb(&dev->rxq, skb, rx_start);
 		}
 	} else {
 		netif_dbg(dev, ifdown, dev->net, "rx: stopped\n");
@@ -849,6 +852,16 @@ int usbnet_stop (struct net_device *net)
 	del_timer_sync (&dev->delay);
 	tasklet_kill (&dev->bh);
 	cancel_work_sync(&dev->kevent);
+
+	/*
+	 * we have cyclic dependencies. Those calls are needed
+	 * to break a cycle. We cannot fall into the gaps because
+	 * we have a flag
+	 */
+	tasklet_kill (&dev->bh);
+	del_timer_sync (&dev->delay);
+	cancel_work_sync(&dev->kevent);
+
 	if (!pm)
 		usb_autopm_put_interface(dev->intf);
 
@@ -1174,7 +1187,8 @@ usbnet_deferred_kevent (struct work_struct *work)
 					   status);
 		} else {
 			clear_bit (EVENT_RX_HALT, &dev->flags);
-			tasklet_schedule (&dev->bh);
+			if (!usbnet_going_away(dev))
+				tasklet_schedule (&dev->bh);
 		}
 	}
 
@@ -1196,10 +1210,13 @@ usbnet_deferred_kevent (struct work_struct *work)
 			}
 			if (rx_submit (dev, urb, GFP_KERNEL) == -ENOLINK)
 				resched = 0;
-			usb_autopm_put_interface(dev->intf);
 fail_lowmem:
-			if (resched)
+			usb_autopm_put_interface(dev->intf);
+			if (resched) {
+				set_bit (EVENT_RX_MEMORY, &dev->flags);
+
 				tasklet_schedule (&dev->bh);
+			}
 		}
 	}
 
@@ -1212,13 +1229,13 @@ usbnet_deferred_kevent (struct work_struct *work)
 		if (status < 0)
 			goto skip_reset;
 		if(info->link_reset && (retval = info->link_reset(dev)) < 0) {
-			usb_autopm_put_interface(dev->intf);
 skip_reset:
 			netdev_info(dev->net, "link reset failed (%d) usbnet usb-%s-%s, %s\n",
 				    retval,
 				    dev->udev->bus->bus_name,
 				    dev->udev->devpath,
 				    info->description);
+			usb_autopm_put_interface(dev->intf);
 		} else {
 			usb_autopm_put_interface(dev->intf);
 		}
@@ -1562,6 +1579,7 @@ static void usbnet_bh (struct timer_list *t)
 	} else if (netif_running (dev->net) &&
 		   netif_device_present (dev->net) &&
 		   netif_carrier_ok(dev->net) &&
+		   !usbnet_going_away(dev) &&
 		   !timer_pending(&dev->delay) &&
 		   !test_bit(EVENT_RX_PAUSED, &dev->flags) &&
 		   !test_bit(EVENT_RX_HALT, &dev->flags)) {
@@ -1609,6 +1627,7 @@ void usbnet_disconnect (struct usb_interface *intf)
 	usb_set_intfdata(intf, NULL);
 	if (!dev)
 		return;
+	usbnet_mark_going_away(dev);
 
 	xdev = interface_to_usbdev (intf);
 
diff --git a/include/linux/usb/usbnet.h b/include/linux/usb/usbnet.h
index 9f08a584d707..d26599faab33 100644
--- a/include/linux/usb/usbnet.h
+++ b/include/linux/usb/usbnet.h
@@ -76,8 +76,26 @@ struct usbnet {
 #		define EVENT_LINK_CHANGE	11
 #		define EVENT_SET_RX_MODE	12
 #		define EVENT_NO_IP_ALIGN	13
+/*
+ * this one is special, as it indicates that the device is going away
+ * there are cyclic dependencies between tasklet, timer and bh
+ * that must be broken
+ */
+#		define EVENT_UNPLUG		31
 };
 
+static inline bool usbnet_going_away(struct usbnet *ubn)
+{
+	smp_mb__before_atomic();
+	return test_bit(EVENT_UNPLUG, &ubn->flags);
+}
+
+static inline void usbnet_mark_going_away(struct usbnet *ubn)
+{
+	set_bit(EVENT_UNPLUG, &ubn->flags);
+	smp_mb__after_atomic();
+}
+
 static inline struct usb_driver *driver_of(struct usb_interface *intf)
 {
 	return to_usb_driver(intf->dev.driver);
-- 
2.44.0

Re: [PATCH net-next] usbnet: fix cyclical race on disconnect with work queue

[Greg KH]

On Thu, Mar 21, 2024 at 01:46:41PM +0100, Oliver Neukum wrote:
> The work can submit URBs and the URBs can schedule the work.
> This cycle needs to be broken, when a device is to be stopped.
> Use a flag to do so.
> 
> Fixes: f29fc259976e9 ("[PATCH] USB: usbnet (1/9) clean up framing")
> Reported-by: syzbot+9665bf55b1c828bbcd8a@syzkaller.appspotmail.com
> Signed-off-by: Oliver Neukum <oneukum@suse.com>
> ---
>  drivers/net/usb/usbnet.c   | 37 ++++++++++++++++++++++++++++---------
>  include/linux/usb/usbnet.h | 18 ++++++++++++++++++
>  2 files changed, 46 insertions(+), 9 deletions(-)
> 
Hi,

This is the friendly patch-bot of Greg Kroah-Hartman.  You have sent him
a patch that has triggered this response.  He used to manually respond
to these common problems, but in order to save his sanity (he kept
writing the same thing over and over, yet to different people), I was
created.  Hopefully you will not take offence and will fix the problem
in your patch and resubmit it so that it can be accepted into the Linux
kernel tree.

You are receiving this message because of the following common error(s)
as indicated below:

- You have marked a patch with a "Fixes:" tag for a commit that is in an
  older released kernel, yet you do not have a cc: stable line in the
  signed-off-by area at all, which means that the patch will not be
  applied to any older kernel releases.  To properly fix this, please
  follow the documented rules in the
  Documentation/process/stable-kernel-rules.rst file for how to resolve
  this.

If you wish to discuss this problem further, or you have questions about
how to resolve this issue, please feel free to respond to this email and
Greg will reply once he has dug out from the pending patches received
from other developers.

thanks,

greg k-h's patch email bot

RE: [PATCH net-next] usbnet: fix cyclical race on disconnect with work queue

[Sai Krishna Gajula]

> -----Original Message-----
> From: Oliver Neukum <oneukum@suse.com>
> Sent: Thursday, March 21, 2024 6:17 PM
> To: davem@davemloft.net; edumazet@google.com; kuba@kernel.org;
> pabeni@redhat.com; netdev@vger.kernel.org; linux-usb@vger.kernel.org;
> linux-kernel@vger.kernel.org
> Cc: Oliver Neukum <oneukum@suse.com>;
> syzbot+9665bf55b1c828bbcd8a@syzkaller.appspotmail.com
> Subject: [PATCH net-next] usbnet: fix cyclical race on disconnect
> with work queue

This patch seems to be a fix, in that case the subject need to be with [PATCH net]

> 
> The work can submit URBs and the URBs can schedule the work.
> This cycle needs to be broken, when a device is to be stopped.
> Use a flag to do so.
> 
> Fixes: f29fc259976e9 ("[PATCH] USB: usbnet (1/9) clean up framing")

Please use correct Fixes: style 'Fixes: <12 chars of sha1> ("<title line>")' - ie: 'Fixes: f29fc259976e ("[PATCH] USB: usbnet (1/9) clean up framing")' 

> Reported-by: syzbot+9665bf55b1c828bbcd8a@syzkaller.appspotmail.com
> Signed-off-by: Oliver Neukum <oneukum@suse.com>
> ---
>  drivers/net/usb/usbnet.c   | 37 ++++++++++++++++++++++++++++---------
>  include/linux/usb/usbnet.h | 18 ++++++++++++++++++
>  2 files changed, 46 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c index
> e84efa661589..422d91635045 100644
> --- a/drivers/net/usb/usbnet.c
> +++ b/drivers/net/usb/usbnet.c
> @@ -467,10 +467,12 @@ static enum skb_state defer_bh(struct usbnet *dev,
> struct sk_buff *skb,  void usbnet_defer_kevent (struct usbnet *dev, int work)

space prohibited between function name and open parenthesis '('

> {
>  	set_bit (work, &dev->flags);
> -	if (!schedule_work (&dev->kevent))
> -		netdev_dbg(dev->net, "kevent %s may have been
> dropped\n", usbnet_event_names[work]);
> -	else
> -		netdev_dbg(dev->net, "kevent %s scheduled\n",
> usbnet_event_names[work]);
> +	if (!usbnet_going_away(dev)) {
> +		if (!schedule_work (&dev->kevent))
> +			netdev_dbg(dev->net, "kevent %s may have been
> dropped\n", usbnet_event_names[work]);
> +		else
> +			netdev_dbg(dev->net, "kevent %s scheduled\n",
> usbnet_event_names[work]);
> +	}
>  }
>  EXPORT_SYMBOL_GPL(usbnet_defer_kevent);
> 
> @@ -538,7 +540,8 @@ static int rx_submit (struct usbnet *dev, struct urb
> *urb, gfp_t flags)

space prohibited between function name and open parenthesis '(', where ever applicable.

>  			tasklet_schedule (&dev->bh);
>  			break;
>  		case 0:
> -			__usbnet_queue_skb(&dev->rxq, skb, rx_start);
> +			if (!usbnet_going_away(dev))
> +				__usbnet_queue_skb(&dev->rxq, skb,
> rx_start);
>  		}
>  	} else {
>  		netif_dbg(dev, ifdown, dev->net, "rx: stopped\n"); @@ -849,6
> +852,16 @@ int usbnet_stop (struct net_device *net)
>  	del_timer_sync (&dev->delay);
>  	tasklet_kill (&dev->bh);
>  	cancel_work_sync(&dev->kevent);
> +
> +	/*
> +	 * we have cyclic dependencies. Those calls are needed
> +	 * to break a cycle. We cannot fall into the gaps because
> +	 * we have a flag
> +	 */

Networking block comments don't use an empty /* line, use /* Comment...

> +	tasklet_kill (&dev->bh);
> +	del_timer_sync (&dev->delay);
> +	cancel_work_sync(&dev->kevent);
> +
>  	if (!pm)
>  		usb_autopm_put_interface(dev->intf);
> 
> @@ -1174,7 +1187,8 @@ usbnet_deferred_kevent (struct work_struct *work)
>  					   status);
>  		} else {
>  			clear_bit (EVENT_RX_HALT, &dev->flags);
> -			tasklet_schedule (&dev->bh);
> +			if (!usbnet_going_away(dev))
> +				tasklet_schedule (&dev->bh);
>  		}
>  	}
> 
> @@ -1196,10 +1210,13 @@ usbnet_deferred_kevent (struct work_struct
> *work)
>  			}
>  			if (rx_submit (dev, urb, GFP_KERNEL) == -ENOLINK)
>  				resched = 0;
> -			usb_autopm_put_interface(dev->intf);
>  fail_lowmem:
> -			if (resched)
> +			usb_autopm_put_interface(dev->intf);
> +			if (resched) {
> +				set_bit (EVENT_RX_MEMORY, &dev->flags);
> +
>  				tasklet_schedule (&dev->bh);
> +			}
>  		}
>  	}
> 
> @@ -1212,13 +1229,13 @@ usbnet_deferred_kevent (struct work_struct
> *work)
>  		if (status < 0)
>  			goto skip_reset;
>  		if(info->link_reset && (retval = info->link_reset(dev)) < 0) {
> -			usb_autopm_put_interface(dev->intf);
>  skip_reset:
>  			netdev_info(dev->net, "link reset failed (%d) usbnet
> usb-%s-%s, %s\n",
>  				    retval,
>  				    dev->udev->bus->bus_name,
>  				    dev->udev->devpath,
>  				    info->description);
> +			usb_autopm_put_interface(dev->intf);
>  		} else {
>  			usb_autopm_put_interface(dev->intf);
>  		}
> @@ -1562,6 +1579,7 @@ static void usbnet_bh (struct timer_list *t)
>  	} else if (netif_running (dev->net) &&
>  		   netif_device_present (dev->net) &&
>  		   netif_carrier_ok(dev->net) &&
> +		   !usbnet_going_away(dev) &&
>  		   !timer_pending(&dev->delay) &&
>  		   !test_bit(EVENT_RX_PAUSED, &dev->flags) &&
>  		   !test_bit(EVENT_RX_HALT, &dev->flags)) { @@ -1609,6
> +1627,7 @@ void usbnet_disconnect (struct usb_interface *intf)
>  	usb_set_intfdata(intf, NULL);
>  	if (!dev)
>  		return;
> +	usbnet_mark_going_away(dev);
> 
>  	xdev = interface_to_usbdev (intf);
> 
> diff --git a/include/linux/usb/usbnet.h b/include/linux/usb/usbnet.h index
> 9f08a584d707..d26599faab33 100644
> --- a/include/linux/usb/usbnet.h
> +++ b/include/linux/usb/usbnet.h
> @@ -76,8 +76,26 @@ struct usbnet {
>  #		define EVENT_LINK_CHANGE	11
>  #		define EVENT_SET_RX_MODE	12
>  #		define EVENT_NO_IP_ALIGN	13
> +/*
> + * this one is special, as it indicates that the device is going away
> + * there are cyclic dependencies between tasklet, timer and bh
> + * that must be broken
> + */

Networking block comments don't use an empty /* line, use /* Comment...

> +#		define EVENT_UNPLUG		31
>  };
> 
> +static inline bool usbnet_going_away(struct usbnet *ubn) {
> +	smp_mb__before_atomic();
> +	return test_bit(EVENT_UNPLUG, &ubn->flags); }
> +
> +static inline void usbnet_mark_going_away(struct usbnet *ubn) {
> +	set_bit(EVENT_UNPLUG, &ubn->flags);
> +	smp_mb__after_atomic();
> +}
> +
>  static inline struct usb_driver *driver_of(struct usb_interface *intf)  {
>  	return to_usb_driver(intf->dev.driver);
> --
> 2.44.0
> 

Re: [PATCH net-next] usbnet: fix cyclical race on disconnect with work queue

[Oliver Neukum]

On 3/22/24 18:43, Sai Krishna Gajula wrote:
> 
>> -----Original Message-----
>> From: Oliver Neukum <oneukum@suse.com>
>> Sent: Thursday, March 21, 2024 6:17 PM
>> To: davem@davemloft.net; edumazet@google.com; kuba@kernel.org;
>> pabeni@redhat.com; netdev@vger.kernel.org; linux-usb@vger.kernel.org;
>> linux-kernel@vger.kernel.org
>> Cc: Oliver Neukum <oneukum@suse.com>;
>> syzbot+9665bf55b1c828bbcd8a@syzkaller.appspotmail.com
>> Subject: [PATCH net-next] usbnet: fix cyclical race on disconnect
>> with work queue
> 
> This patch seems to be a fix, in that case the subject need to be with [PATCH net]

OK
> 
>>
>> The work can submit URBs and the URBs can schedule the work.
>> This cycle needs to be broken, when a device is to be stopped.
>> Use a flag to do so.
>>
>> Fixes: f29fc259976e9 ("[PATCH] USB: usbnet (1/9) clean up framing")
> 
> Please use correct Fixes: style 'Fixes: <12 chars of sha1> ("<title line>")' - ie: 'Fixes: f29fc259976e ("[PATCH] USB: usbnet (1/9) clean up framing")'

Ehm, what exactly did I do differently

>> --- a/drivers/net/usb/usbnet.c
>> +++ b/drivers/net/usb/usbnet.c
>> @@ -467,10 +467,12 @@ static enum skb_state defer_bh(struct usbnet *dev,
>> struct sk_buff *skb,  void usbnet_defer_kevent (struct usbnet *dev, int work)
> 
> space prohibited between function name and open parenthesis '('

I am sorry, but this is the context of the diff. You are not suggesting
to mix gratitious format changes into a bug fix, are you?

>> diff --git a/include/linux/usb/usbnet.h b/include/linux/usb/usbnet.h index
>> 9f08a584d707..d26599faab33 100644
>> --- a/include/linux/usb/usbnet.h
>> +++ b/include/linux/usb/usbnet.h
>> @@ -76,8 +76,26 @@ struct usbnet {
>>   #		define EVENT_LINK_CHANGE	11
>>   #		define EVENT_SET_RX_MODE	12
>>   #		define EVENT_NO_IP_ALIGN	13
>> +/*
>> + * this one is special, as it indicates that the device is going away
>> + * there are cyclic dependencies between tasklet, timer and bh
>> + * that must be broken
>> + */
> 
> Networking block comments don't use an empty /* line, use /* Comment...

OK

	Regards
		Oliver

Re: [PATCH net-next] usbnet: fix cyclical race on disconnect with work queue

[Simon Horman]

On Wed, Mar 27, 2024 at 04:10:36PM +0100, Oliver Neukum wrote:
> 
> 
> On 3/22/24 18:43, Sai Krishna Gajula wrote:
> > 
> > > -----Original Message-----
> > > From: Oliver Neukum <oneukum@suse.com>
> > > Sent: Thursday, March 21, 2024 6:17 PM
> > > To: davem@davemloft.net; edumazet@google.com; kuba@kernel.org;
> > > pabeni@redhat.com; netdev@vger.kernel.org; linux-usb@vger.kernel.org;
> > > linux-kernel@vger.kernel.org
> > > Cc: Oliver Neukum <oneukum@suse.com>;
> > > syzbot+9665bf55b1c828bbcd8a@syzkaller.appspotmail.com
> > > Subject: [PATCH net-next] usbnet: fix cyclical race on disconnect
> > > with work queue
> > 
> > This patch seems to be a fix, in that case the subject need to be with [PATCH net]
> 
> OK
> > 
> > > 
> > > The work can submit URBs and the URBs can schedule the work.
> > > This cycle needs to be broken, when a device is to be stopped.
> > > Use a flag to do so.
> > > 
> > > Fixes: f29fc259976e9 ("[PATCH] USB: usbnet (1/9) clean up framing")
> > 
> > Please use correct Fixes: style 'Fixes: <12 chars of sha1> ("<title line>")' - ie: 'Fixes: f29fc259976e ("[PATCH] USB: usbnet (1/9) clean up framing")'
> 
> Ehm, what exactly did I do differently

I think the point being made is that the hash has 13 rather than 12
characters. But, IMHO, that is fine because my understanding is that the
requirement is that the hash is at least, not exactly, 12 characters long.

...

RE: [PATCH net-next] usbnet: fix cyclical race on disconnect with work queue

[Sai Krishna Gajula]

> -----Original Message-----
> From: Oliver Neukum <oneukum@suse.com>
> Sent: Wednesday, March 27, 2024 8:41 PM
> To: Sai Krishna Gajula <saikrishnag@marvell.com>; Oliver Neukum
> <oneukum@suse.com>; davem@davemloft.net; edumazet@google.com;
> kuba@kernel.org; pabeni@redhat.com; netdev@vger.kernel.org; linux-
> usb@vger.kernel.org; linux-kernel@vger.kernel.org
> Cc: syzbot+9665bf55b1c828bbcd8a@syzkaller.appspotmail.com
> Subject: Re: [PATCH net-next] usbnet: fix cyclical race on
> disconnect with work queue
> 
> 
> On 3/22/24 18:43, Sai Krishna Gajula wrote:
> >
> >> -----Original Message-----
> >> From: Oliver Neukum <oneukum@suse.com>
> >> Sent: Thursday, March 21, 2024 6:17 PM
> >> To: davem@davemloft.net; edumazet@google.com; kuba@kernel.org;
> >> pabeni@redhat.com; netdev@vger.kernel.org; linux-usb@vger.kernel.org;
> >> linux-kernel@vger.kernel.org
> >> Cc: Oliver Neukum <oneukum@suse.com>;
> >> syzbot+9665bf55b1c828bbcd8a@syzkaller.appspotmail.com
> >> Subject: [PATCH net-next] usbnet: fix cyclical race on disconnect
> >> with work queue
> >
> > This patch seems to be a fix, in that case the subject need to be with
> > [PATCH net]
> 
> OK
> >
> >>
> >> The work can submit URBs and the URBs can schedule the work.
> >> This cycle needs to be broken, when a device is to be stopped.
> >> Use a flag to do so.
> >>
> >> Fixes: f29fc259976e9 ("[PATCH] USB: usbnet (1/9) clean up framing")
> >
> > Please use correct Fixes: style 'Fixes: <12 chars of sha1> ("<title line>")' - ie:
> 'Fixes: f29fc259976e ("[PATCH] USB: usbnet (1/9) clean up framing")'
> 
> Ehm, what exactly did I do differently

Checkpatch triggers warning if we give Fixes tag(f29fc259976e -9-) more than 12 chars. It is advisable to follow the upstreaming process steps to have uniformity across patches.

> 
> >> --- a/drivers/net/usb/usbnet.c
> >> +++ b/drivers/net/usb/usbnet.c
> >> @@ -467,10 +467,12 @@ static enum skb_state defer_bh(struct usbnet
> >> *dev, struct sk_buff *skb,  void usbnet_defer_kevent (struct usbnet
> >> *dev, int work)
> >
> > space prohibited between function name and open parenthesis '('
> 
> I am sorry, but this is the context of the diff. You are not suggesting to mix
> gratitious format changes into a bug fix, are you?

Checkpatch does a primary check and triggered warning if we use space between fn name and '('.  It is advisable to follow the upstreaming process steps(clean checkpatch output) to have uniformity across patches. Also check comments from Gregkh bot regarding this patch.

> 
> >> diff --git a/include/linux/usb/usbnet.h b/include/linux/usb/usbnet.h
> >> index
> >> 9f08a584d707..d26599faab33 100644
> >> --- a/include/linux/usb/usbnet.h
> >> +++ b/include/linux/usb/usbnet.h
> >> @@ -76,8 +76,26 @@ struct usbnet {
> >>   #		define EVENT_LINK_CHANGE	11
> >>   #		define EVENT_SET_RX_MODE	12
> >>   #		define EVENT_NO_IP_ALIGN	13
> >> +/*
> >> + * this one is special, as it indicates that the device is going
> >> +away
> >> + * there are cyclic dependencies between tasklet, timer and bh
> >> + * that must be broken
> >> + */
> >
> > Networking block comments don't use an empty /* line, use /* Comment...
> 
> OK
> 
> 	Regards
> 		Oliver

Re: [PATCH net-next] usbnet: fix cyclical race on disconnect with work queue

[Oliver Neukum]

On 31.03.24 14:07, Sai Krishna Gajula wrote:
>>>> --- a/drivers/net/usb/usbnet.c
>>>> +++ b/drivers/net/usb/usbnet.c
>>>> @@ -467,10 +467,12 @@ static enum skb_state defer_bh(struct usbnet
>>>> *dev, struct sk_buff *skb,  void usbnet_defer_kevent (struct usbnet
>>>> *dev, int work)
>>>
>>> space prohibited between function name and open parenthesis '('
>>
>> I am sorry, but this is the context of the diff. You are not suggesting to mix
>> gratitious format changes into a bug fix, are you?
> 
> Checkpatch does a primary check and triggered warning if we use space between fn name and '('.  It is advisable to follow the upstreaming process steps(clean checkpatch output) to have uniformity across patches. Also check comments from Gregkh bot regarding this patch.

Hi,

at the risk of repeating myself:

This is a very old driver written long before these conventions.
I did not introduce these spaces. Nevertheless, a format change
has no place in a bug fix.

	Regards
		Oliver