On 08/08/2017 03:11 AM, fx IWATA NOBUO wrote:
>> -----Original Message-----
>> From: Shuah Khan [mailto:sh...@kernel.org]
>> Sent: Thursday, August 03, 2017 2:27 AM
>> To: fx IWATA NOBUO <nobuo.iw...@fujixerox.co.jp>;
>> valentina.mane...@gmail.com; gre...@linuxfoundation.org;
>> Cc: fx MICHIMURA TADAO <michimura.ta...@fujixerox.co.jp>; Shuah
>> Khan <sh...@kernel.org>; Shuah Khan <shua...@osg.samsung.com>
>> 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
>>> The procedure of attach operation is as below.
>>> 1) Find an unused port in /sys/devices/platform/vhci_hcd/status.
>>> 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
>>> vhci-hcd's interface (only errno) is changed as following.
>>> Current errno New errno Condition
>>> EINVAL same as left specified port numbre is in
>>> 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
>>> Gathered usbip_vhci_driver_close() for errors under an exit label.
>> Greg KH already pointed this out. Please fix this.
>>> Signed-off-by: Nobuo Iwata <nobuo.iw...@fujixerox.co.jp>
>>> 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
>>> 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.
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html