On 08/08/2017 03:11 AM, fx IWATA NOBUO wrote: > Hello, > >> -----Original Message----- >> From: Shuah Khan [mailto:shuah@xxxxxxxxxx] >> Sent: Thursday, August 03, 2017 2:27 AM >> To: fx IWATA NOBUO <Nobuo.Iwata@xxxxxxxxxxxxxxx>; >> valentina.manea.m@xxxxxxxxx; gregkh@xxxxxxxxxxxxxxxxxxx; >> linux-usb@xxxxxxxxxxxxxxx >> Cc: fx MICHIMURA TADAO <MICHIMURA.Tadao@xxxxxxxxxxxxxxx>; Shuah >> Khan <shuah@xxxxxxxxxx>; Shuah Khan <shuahkh@xxxxxxxxxxxxxxx> >> Subject: Re: [PATCH v2 1/1] usbip: auto retry for concurrent attach >> >> Hi Nobuo Iwata, >> >> Sorry for the delay. This one got lost in my Inbox. >> >> On 05/28/2017 08:31 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. >>> >>> 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. >> >> Please spell check the change log. > > I will. > >>> 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 >>> >>> --- >>> Version information >>> >>> v2) >>> Gathered usbip_vhci_driver_close() for errors under an exit label. >> >> Greg KH already pointed this out. Please fix this. > > OK. > >>> Signed-off-by: Nobuo Iwata <nobuo.iwata@xxxxxxxxxxxxxxx> >>> --- >>> drivers/usb/usbip/vhci_sysfs.c | 8 ++++++-- >>> tools/usb/usbip/src/usbip_attach.c | 33 >> +++++++++++++++++------------- >>> 2 files changed, 25 insertions(+), 16 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; >> >> Why are you changing the return value here? vhci_port_disconnect() >> currently only returns -EINVAL, however of that changes, userspace >> will see a different value. If the correct retunr value in this failure >> path is -EINVAL, let's not change that, unless there are good reasons >> do so. > > Attach operation is done by searching a free port via sysfs and attach > specifying found free port with the port number. > > If 2 processes try to attach concurrently, they will find same port > number and one will fail. In this failed case, it can restart from > searching a free port and it will be successfully attached if there's > another free port. > > But other cases, specified port number is in invalid range (EINVAL), > platform_get_drvdata() failed (EAGAIN) or specified socket fd is not > valid (EINVAL), should not be retried. > > To distinguish the cases, the different errno (EBUSY) is need for the > case if port number is valid range but used. Sounds reasonable. thanks, -- Shuah -- 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
