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 <nobuo.iwata@xxxxxxxxxxxxxxx>
Looks good to me.
Acked-by: Shuah Khan <shuahkh@xxxxxxxxxxxxxxx>
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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
