Hi,
On 12/26/2016 05:53 AM, 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.
>
> It is possible to avoid this fail by introdocing userspace exclusive
> control. But it's exaggerated for this special condition. The locking
> itself is done in driver.
>
> 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. Othrwise it retries automatically using new free port.
>
> Currunt userspace src/usbip_attach.c:import_device() doesn't use the
> errno.
>
> vhci-hcd's interface (only errno) is changed as following.
>
> Current errno New errno Condition
> EINVAL same as left specified port numbre 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
>
> Signed-off-by: Nobuo Iwata <nobuo.iwata@xxxxxxxxxxxxxxx>
> ---
> drivers/usb/usbip/vhci_sysfs.c | 8 ++++++--
> tools/usb/usbip/src/usbip_attach.c | 28 +++++++++++++++-------------
> 2 files changed, 21 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/usb/usbip/vhci_sysfs.c b/drivers/usb/usbip/vhci_sysfs.c
> index b96e5b1..5d4be4b 100644
> --- a/drivers/usb/usbip/vhci_sysfs.c
> +++ b/drivers/usb/usbip/vhci_sysfs.c
> @@ -214,7 +214,7 @@ static ssize_t store_detach(struct device *dev, struct device_attribute *attr,
>
> ret = vhci_port_disconnect(hcd_to_vhci(hcd), rhport);
> if (ret < 0)
> - return -EINVAL;
> + return ret;
>
> usbip_dbg_vhci_sysfs("Leave\n");
>
> @@ -314,7 +314,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 70a6b50..695b2e5 100644
> --- a/tools/usb/usbip/src/usbip_attach.c
> +++ b/tools/usb/usbip/src/usbip_attach.c
> @@ -101,20 +101,22 @@ static int import_device(int sockfd, struct usbip_usb_device *udev)
> return -1;
> }
>
> - port = usbip_vhci_get_free_port();
> - if (port < 0) {
> - err("no free port");
> - usbip_vhci_driver_close();
> - return -1;
> - }
> + do {
> + port = usbip_vhci_get_free_port();
> + if (port < 0) {
> + err("no free port");
> + usbip_vhci_driver_close();
> + return -1;
> + }
>
> - 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");
> + usbip_vhci_driver_close();
> + return -1;
> + }
copy-paste in error path. Could you please fix this to use goto as only
for this single function usbip_vhci_driver_close() is called in 3
different places.
> + } while (rc < 0);
>
> usbip_vhci_driver_close();
>
>
Best regards,
--
Krzysztof Opasiak
Samsung R&D Institute Poland
Samsung Electronics
--
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
