-----Original Message-----
From: Oliver Neukum <oneukum@suse.com>
Sent: Wednesday, March 27, 2024 8:41 PM
To: Sai Krishna Gajula <redacted>; Oliver Neukum
[off-list ref]; 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:

quoted

quoted

-----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

quoted

quoted

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.

quoted

quoted

--- 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.

quoted

quoted

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