Re: "usbip: Implement SG support to vhci-hcd and stub driver" causes a deadlock

[Suwan Kim <suwan.kim027@xxxxxxxxx>]

On Wed, Dec 11, 2019 at 04:20:32AM +0100, Marek Marczykowski-Górecki wrote:
> On Wed, Dec 11, 2019 at 12:07:54PM +0900, Suwan Kim wrote:
> > HCD should giveback URB to driver calling usb_hcd_giveback_urb().
> > But in the case of transaction error, vhci_recv_ret_submit()
> > terminates without giveback URB. So, I think the error path in
> > usbip_recv_xbuff and usbip_recv_iso should unlink URB from ep and
> > insert proper error code in urb->status that indicates error
> > status to driver and handle giveback URB to driver.
> > 
> > Then hub_irq doesn't need to flush error URB.
> > That will be helpful to graceful connection shutdown.
> > 
> > 
> > static void vhci_recv_ret_submit(struct vhci_device *vdev,
> > 				 struct usbip_header *pdu)
> > ...
> > ...
> > 	if (usbip_recv_xbuff(ud, urb) < 0) {
> > 		urb->status = -EPIPE;
> > 		goto error;	// goto error path
> > 	}
> > 
> > 	/* recv iso_packet_descriptor */
> > 	if (usbip_recv_iso(ud, urb) < 0) {
> > 		urb->status = -EPIPE;
> > 		goto error;	// goto error path
> > 	}
> > ...
> > ...
> > error://error path
> > 	spin_lock_irqsave(&vhci->lock, flags);
> > 	usb_hcd_unlink_urb_from_ep(vhci_hcd_to_hcd(vhci_hcd), urb);
> > 	spin_unlock_irqrestore(&vhci->lock, flags);
> > 
> > 	usb_hcd_giveback_urb(vhci_hcd_to_hcd(vhci_hcd), urb, urb->status);
> > 
> > 	usbip_dbg_vhci_rx("Leave\n");
> > }
> 
> Yup, that works! Now the error is handled gracefully instead of hanging.

Marek, Could you test if it works too?

Regards,
Suwan Kim


diff --git a/drivers/usb/usbip/vhci_rx.c b/drivers/usb/usbip/vhci_rx.c
index 33f8972ba842..19a807e398eb 100644
--- a/drivers/usb/usbip/vhci_rx.c
+++ b/drivers/usb/usbip/vhci_rx.c
@@ -78,11 +78,11 @@ static void vhci_recv_ret_submit(struct vhci_device *vdev,

        /* recv transfer buffer */
        if (usbip_recv_xbuff(ud, urb) < 0)
-               return;
+               urb->status = -EPIPE;

        /* recv iso_packet_descriptor */
        if (usbip_recv_iso(ud, urb) < 0)
-               return;
+               urb->status = -EPIPE;

        /* restore the padding in iso packets */
        usbip_pad_iso(ud, urb);