> Why spam the kernel log for this? What can a user do with this information? > Why not make it just a debug statement? No need to be 'info' level. It was my mistake. I will change it to 'debug' and send v2. Thank you for reviewing, nobuo.iwata // > -----Original Message----- > From: Greg KH [mailto:gregkh@xxxxxxxxxxxxxxxxxxx] > Sent: Wednesday, April 27, 2016 7:02 AM > To: fx IWATA NOBUO > Cc: valentina.manea.m@xxxxxxxxx; shuah.kh@xxxxxxxxxxx; > linux-usb@xxxxxxxxxxxxxxx; fx MICHIMURA TADAO > Subject: Re: [PATCH v1 1/1] usbip: safe completion against unbind operation > > On Tue, Mar 22, 2016 at 04:33:29PM +0900, Nobuo Iwata wrote: > > This patch adds a code fragment to ignore completing URBs in closing > > connection. > > > > Regarding this patch, 2 execution contexts are related. > > > > 1) stub_tx.c: stub_complete() which is called from USB core > > 1-1) add to unlink list and free URB or > > 1-2) move to tx list > > > > 2) stub_dev.c: stub_shutdown_connection() which is invoked by unbind > > operation through sysfs. > > 2-1) stop TX/RX threads > > 2-2) close TCP connection and set ud.tcp_socket to NULL > > 2-3) cleanup pending URBs by stub_device_cleanup_urbs(sdev) > > 2-4) free unlink list (no lock) > > > > In race condition URBs will be cleared in 2-3) may handled in 1). > > In case 1-1), it will not be transferred bcause tx threads are stooped > > in 2-1). > > In case 1-2), may be freed in 2-4). > > > > With this patch, after 2-2), completing URBs in 1) will not be handled > > and cleared in 2-3). > > > > The kernel log with patch is as below. > > > > kernel: usbip-host 1-3: free sdev f5555680 > > kernel: usbip-host 1-3: free urb efcb8080 > > kernel: usbip-host 1-3: Enter > > kernel: usbip-host 1-3: ignore urb for closed connection efc80c00 (*) > > kernel: usbip-host 1-3: stopped by a call to usb_kill_urb() because of > > cleaning up a virtual connection > > kernel: usbip-host 1-3: free urb efc80c00 (**) > > kernel: usbip-host 1-3: device reset > > kernel: usbip-host 1-3: lock for reset > > kernel: usbip_host: store_match_busid:178: del busid 1-3 > > kernel: uvcvideo: Found UVC 1.00 device Venus USB2.0 Camera > > (056e:700a) > > kernel: input: Venus USB2.0 Camera as > > /devices/pci0000:00/0000:00:1a.7/usb1/1-3/1-3:1.0/input/input18 > > > > (*) skipped with this patch in completion > > (**) released in 2-3 > > > > Signed-off-by: Nobuo Iwata <nobuo.iwata@xxxxxxxxxxxxxxx> > > --- > > drivers/usb/usbip/stub_tx.c | 6 +++++- > > 1 file changed, 5 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/usb/usbip/stub_tx.c b/drivers/usb/usbip/stub_tx.c > > index dbcabc9..a4efc5a 100644 > > --- a/drivers/usb/usbip/stub_tx.c > > +++ b/drivers/usb/usbip/stub_tx.c > > @@ -97,7 +97,11 @@ void stub_complete(struct urb *urb) > > > > /* link a urb to the queue of tx. */ > > spin_lock_irqsave(&sdev->priv_lock, flags); > > - if (priv->unlinking) { > > + if (sdev->ud.tcp_socket == NULL) { > > + dev_info(&urb->dev->dev, > > + "ignore urb for closed connection %p", urb); > > Why spam the kernel log for this? What can a user do with this information? > Why not make it just a debug statement? > > thanks, > > greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
