On 08/28/2017 08:13 PM, Nobuo Iwata wrote: > This patch adds recovery from false busy state on concurrent attach > operation. > > The procedure of attach operation is as below. > 1) Find an unused port in /sys/devices/platform/vhci_hcd/status. > (userspace) > 2) Request attach found port to driver through > /sys/devices/platform/vhci_hcd/attach. (userspace) > 3) Lock table, reserve requested port and unlock table. (vhci driver) > > Attaching more than one remote devices concurrently, same unused port > number will be found in step-1. Then one request will succeed and > others will fail even though there are some unused ports. > > With this patch, driver returns EBUSY when requested port has already > been used. In this case, attach command retries from step-1: finding > another unused port. If there's no unused port, the attach operation > will fail in step-1. Otherwise it retries automatically using another > unused port. > > vhci-hcd's interface (only errno) is changed as following. > > Current errno New errno Condition > EINVAL same as left specified port number is in invalid > range > EAGAIN same as left platform_get_drvdata() failed > EINVAL same as left specified socket fd is not valid > EINVAL EBUSY specified port status is not free > > The errno EBUSY was not used in userspace > src/usbip_attach.c:import_device(). It is needed to distinguish the > condition to be able to retry from other unrecoverable errors. > > It is possible to avoid this failure by introducing userspace exclusive > control. But it's exaggerated for this special condition. The locking > itself has done in driver. > As an alternate solution, userspace doesn't specify port number, driver > searches unused port and it returns port number to the userspace. With > this solution, the interface is much different than this patch. > > Signed-off-by: Nobuo Iwata <[email protected]>
Looks good to me. Acked-by: Shuah Khan <[email protected]> thanks, -- Shuah > --- > Version information > > v3) > Updated based on linux-next 2017-08-28. > Removed unnecessary modification to store_detach(). > Added description about difference from alternate solution in change log. > Corrected spelling in change log. > Moved the position of version info in change log. > > v2) > Gathered usbip_vhci_driver_close() for errors under an exit label. > --- > drivers/usb/usbip/vhci_sysfs.c | 6 ++++- > tools/usb/usbip/src/usbip_attach.c | 35 +++++++++++++++++------------- > 2 files changed, 25 insertions(+), 16 deletions(-) > > diff --git a/drivers/usb/usbip/vhci_sysfs.c b/drivers/usb/usbip/vhci_sysfs.c > index 5778b64..1b9f60a 100644 > --- a/drivers/usb/usbip/vhci_sysfs.c > +++ b/drivers/usb/usbip/vhci_sysfs.c > @@ -366,7 +366,11 @@ static ssize_t store_attach(struct device *dev, struct > device_attribute *attr, > sockfd_put(socket); > > dev_err(dev, "port %d already used\n", rhport); > - return -EINVAL; > + /* > + * Will be retried from userspace > + * if there's another free port. > + */ > + return -EBUSY; > } > > dev_info(dev, "pdev(%u) rhport(%u) sockfd(%d)\n", > diff --git a/tools/usb/usbip/src/usbip_attach.c > b/tools/usb/usbip/src/usbip_attach.c > index 6e89768..7f07b2d 100644 > --- a/tools/usb/usbip/src/usbip_attach.c > +++ b/tools/usb/usbip/src/usbip_attach.c > @@ -99,29 +99,34 @@ static int import_device(int sockfd, struct > usbip_usb_device *udev) > rc = usbip_vhci_driver_open(); > if (rc < 0) { > err("open vhci_driver"); > - return -1; > + goto err_out; > } > > - port = usbip_vhci_get_free_port(speed); > - if (port < 0) { > - err("no free port"); > - usbip_vhci_driver_close(); > - return -1; > - } > + do { > + port = usbip_vhci_get_free_port(speed); > + if (port < 0) { > + err("no free port"); > + goto err_driver_close; > + } > > - dbg("got free port %d", port); > + dbg("got free port %d", port); > > - rc = usbip_vhci_attach_device(port, sockfd, udev->busnum, > - udev->devnum, udev->speed); > - if (rc < 0) { > - err("import device"); > - usbip_vhci_driver_close(); > - return -1; > - } > + rc = usbip_vhci_attach_device(port, sockfd, udev->busnum, > + udev->devnum, udev->speed); > + if (rc < 0 && errno != EBUSY) { > + err("import device"); > + goto err_driver_close; > + } > + } while (rc < 0); > > usbip_vhci_driver_close(); > > return port; > + > +err_driver_close: > + usbip_vhci_driver_close(); > +err_out: > + return -1; > } > > static int query_import_device(int sockfd, char *busid) > -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
