[9fans] new usb stack and implicit timeouts

[9fans] new usb stack and implicit timeouts

[cinap_lenrek]

from the manpage:

          For control, bulk, and isochronous transfers, there is an
          implicit timeout performed by the kernel and it is not nec-
          essary for applications to place their own timers.  For
          interrupt transfers, the kernel will not time out any opera-
          tion.

souldnt the application / userspace driver know better than some
random choosen timeout in the kernel driver?

also, this has not been taken into account for the new usb/ether.

for now i'll just compare the errstr and try again, but this implicit
timeout stuff just smells "too smart" for me.

--
cinap

Re: [9fans] new usb stack and implicit timeouts

[Bruce Ellis]

The only justification I can see is to disconnect to stuff that's been
unplugged or misbehaves.

In your case that's not true.

brucee

On Sun, Jul 19, 2009 at 5:16 PM, <cinap_lenrek@gmx.de> wrote:
> from the manpage:
>
>          For control, bulk, and isochronous transfers, there is an
>          implicit timeout performed by the kernel and it is not nec-
>          essary for applications to place their own timers.  For
>          interrupt transfers, the kernel will not time out any opera-
>          tion.
>
> souldnt the application / userspace driver know better than some
> random choosen timeout in the kernel driver?
>
> also, this has not been taken into account for the new usb/ether.
>
> for now i'll just compare the errstr and try again, but this implicit
> timeout stuff just smells "too smart" for me.
>
> --
> cinap
>
>
>

Re: [9fans] new usb stack and implicit timeouts

[cinap_lenrek]

[-- Attachment #1: Type: text/plain, Size: 379 bytes --]

pulling the device gets me a "crc/timeout error", not a
"request timed out".

but i'm not sure if this is always the case though.

the driver should not artificially generate errors in
my opinion even if it would be convinient for some
userspace drivers to have it. those who need a timeout
should choose ther own value depending on what
they are doing.

--
cinap


[-- Attachment #2: Type: message/rfc822, Size: 4032 bytes --]

From: Bruce Ellis <bruce.ellis@gmail.com>
To: Fans of the OS Plan 9 from Bell Labs <9fans@9fans.net>
Subject: Re: [9fans] new usb stack and implicit timeouts
Date: Sun, 19 Jul 2009 17:21:16 +1000
Message-ID: <775b8d190907190021hadae3ffyea534970379e313d@mail.gmail.com>

The only justification I can see is to disconnect to stuff that's been
unplugged or misbehaves.

In your case that's not true.

brucee

On Sun, Jul 19, 2009 at 5:16 PM, <cinap_lenrek@gmx.de> wrote:
> from the manpage:
>
>          For control, bulk, and isochronous transfers, there is an
>          implicit timeout performed by the kernel and it is not nec-
>          essary for applications to place their own timers.  For
>          interrupt transfers, the kernel will not time out any opera-
>          tion.
>
> souldnt the application / userspace driver know better than some
> random choosen timeout in the kernel driver?
>
> also, this has not been taken into account for the new usb/ether.
>
> for now i'll just compare the errstr and try again, but this implicit
> timeout stuff just smells "too smart" for me.
>
> --
> cinap
>
>
>

Re: [9fans] new usb stack and implicit timeouts

[Francisco J Ballesteros]

THere are some disks that do not respond
to the controller after they crash. Also, RPCs carrying ctl requests
to the devices
may not respond either in some devices. I thought it was for sure
an error when control and bulk requests took more than a while.

Right now I´m not so sure regarding bulk transfers, but I still think
it´s a good idea to timeout in the kernel control requests.

pulling out is a different thing, as you say.

ether requires more work, agree.

On Sun, Jul 19, 2009 at 9:54 AM, <cinap_lenrek@gmx.de> wrote:
> pulling the device gets me a "crc/timeout error", not a
> "request timed out".
>
> but i'm not sure if this is always the case though.
>
> the driver should not artificially generate errors in
> my opinion even if it would be convinient for some
> userspace drivers to have it. those who need a timeout
> should choose ther own value depending on what
> they are doing.
>
> --
> cinap
>
>
>
> ---------- Forwarded message ----------
> From: Bruce Ellis <bruce.ellis@gmail.com>
> To: Fans of the OS Plan 9 from Bell Labs <9fans@9fans.net>
> Date: Sun, 19 Jul 2009 17:21:16 +1000
> Subject: Re: [9fans] new usb stack and implicit timeouts
> The only justification I can see is to disconnect to stuff that's been
> unplugged or misbehaves.
>
> In your case that's not true.
>
> brucee
>
> On Sun, Jul 19, 2009 at 5:16 PM, <cinap_lenrek@gmx.de> wrote:
>> from the manpage:
>>
>>          For control, bulk, and isochronous transfers, there is an
>>          implicit timeout performed by the kernel and it is not nec-
>>          essary for applications to place their own timers.  For
>>          interrupt transfers, the kernel will not time out any opera-
>>          tion.
>>
>> souldnt the application / userspace driver know better than some
>> random choosen timeout in the kernel driver?
>>
>> also, this has not been taken into account for the new usb/ether.
>>
>> for now i'll just compare the errstr and try again, but this implicit
>> timeout stuff just smells "too smart" for me.
>>
>> --
>> cinap
>>
>>
>>
>

Re: [9fans] new usb stack and implicit timeouts

[Richard Miller]

> THere are some disks that do not respond
> to the controller after they crash. Also, RPCs carrying ctl requests
> to the devices
> may not respond either in some devices. I thought it was for sure
> an error when control and bulk requests took more than a while.

Bulk pipes are not always used in a request-response manner -- they
are just pipes.  For many device classes (like ether or serial),
a bulk read may need to wait an arbitrary amount of time until the
other end has something to send.  Have you seen anything in the USB
spec which indicates a timeout for reading from a bulk pipe is
appropriate?

Re: [9fans] new usb stack and implicit timeouts

[Francisco J Ballesteros]

> other end has something to send.  Have you seen anything in the USB
> spec which indicates a timeout for reading from a bulk pipe is
> appropriate?
>

No. All devices I had tested at that time required a time out.
Ethernet came later.

I think it's better to remove the timeout from bulk endpoints (perhaps by
making it optional) and leave
it there in the case of control requests. I was just waiting to september,
to give it time to the current code to show wichever bugs it may have.
Perhaps is better not to wait.

I strongly argue in favor of in-kernel ctl timeouts. Otherwise programs like
usbd (and all drivers) will have a bad time. Moreover, I've seen
devices that block on particular requests but crash if you interrupt
them at bad times. The kernel is careful to interrupt the control
transfers without bothering the device much.

Re: [9fans] new usb stack and implicit timeouts

[Charles Forsyth]

disconnecting a usb device should result (eventually)
in a suitable status on the relevant hub, and thus
shouldn't require a timeout to get an error back to the user.

devices that don't respond because they are in a bad
state can be unplugged (if removeable).

built-in devices on built-in hubs that get into a
bad state probably need a usb or system reset,
but some higher-level application (or human) timeout
can interrupt a too-long delayed request.

so far, i'd argue that a per-request timeout wasn't needed.

Re: [9fans] new usb stack and implicit timeouts

[Richard Miller]

> I think it's better to remove the timeout from bulk endpoints (perhaps by
> making it optional)

There's already a general way to time out any read/write operation
alarm() and notify().  Why add a special case option for one particular
type of file?  I would say just remove it.

>
> I strongly argue in favor of in-kernel ctl timeouts.

Yes, the control endpoint has a well-defined RPC protocol and
lack of response is always an error.

Re: [9fans] new usb stack and implicit timeouts

[Francisco J Ballesteros]

> There's already a general way to time out any read/write operation
> alarm() and notify().  Why add a special case option for one particular
> type of file?  I would say just remove it.
>

You're right.
I'll do so.

Re: [9fans] new usb stack and implicit timeouts

[erik quanstrom]

On Sun Jul 19 07:53:33 EDT 2009, 9fans@hamnavoe.com wrote:
> > I think it's better to remove the timeout from bulk endpoints (perhaps by
> > making it optional)
>
> There's already a general way to time out any read/write operation
> alarm() and notify().  Why add a special case option for one particular
> type of file?  I would say just remove it.

i'm confliced on this one.  i want to agree with you, but have
recient memory of uncooperative devices.  and i would hate
to limit the functionality of usb because we're trying to
make things too simple.

i'm not up-to-speed on usb.  aren't there device types
that take timeouts in their requests?  isn't it easier to set
up time timeout at the beginning?  are there devices that
if given a timeout will give their best available data when
the timeout expires?

sd doesn't deal with this problem.  for example, currently most
of the sd devices (orion is an exception) are uninterruptable.
and you get whatever timeout you get.

the sticky bit is that scsi and ata devices implement timeouts
on the devices.  these might not always be appropriate and
you can't depend on anything.  i have drives that vary from
53000s default timeouts to 2s default r/w timeouts.  if you need
a longer timeout than the disk or driver wants, you can't
specify that with an alarm(2).  and if you want a shorter one,
aborting commands can take several seconds with sata devices.

- erik

Re: [9fans] new usb stack and implicit timeouts

[erik quanstrom]

> sd doesn't deal with this problem.  for example, currently most
> of the sd devices (orion is an exception) are uninterruptable.
> and you get whatever timeout you get.
>
> the sticky bit is that scsi and ata devices implement timeouts
> on the devices.  these might not always be appropriate and
> you can't depend on anything.  i have drives that vary from
> 53000s default timeouts to 2s default r/w timeouts.  if you need
> a longer timeout than the disk or driver wants, you can't
> specify that with an alarm(2).  and if you want a shorter one,
> aborting commands can take several seconds with sata devices.

sorry.  that wasn't clear.  i was talking about the raw file.

- erik

Re: [9fans] new usb stack and implicit timeouts

[Richard Miller]

> aren't there device types
> that take timeouts in their requests?

There might be, but if so, that's the business of the device's
own driver, not the usb driver.

>  isn't it easier to set
> up time timeout at the beginning?

Not if you use normal read/write to talk to usb endpoints (which
seems to me a Good Thing).  Normal read/write system call doesn't
have a timeout argument.

>  are there devices that
> if given a timeout will give their best available data when
> the timeout expires?

Again, if so, that's for the device's driver to set up and deal with,
not the usb driver.

Re: [9fans] new usb stack and implicit timeouts

[erik quanstrom]

> >  isn't it easier to set
> > up time timeout at the beginning?
>
> Not if you use normal read/write to talk to usb endpoints (which
> seems to me a Good Thing).  Normal read/write system call doesn't
> have a timeout argument.

do you mean "normal read/write" vs. an rpc protocol, say, like
/dev/sdXX/raw?

- erik

Re: [9fans] new usb stack and implicit timeouts

[Francisco J Ballesteros]

that's what I understood.
In any case I'll run the code through all devices I have before
sending any usb patch. I'm still not sure that some disks currently
working won't cease working if they do their own timeouts. I just
want to be sure.


I placed timeouts there only when I found uncooperative devices, in practice.
In theory, not even ctl timeouts are needed. (I should get
crc/timeout errors even in those cases according to the std).

but I have learned the hard way not to trust any usb std.

On Sun, Jul 19, 2009 at 4:32 PM, erik quanstrom<quanstro@quanstro.net> wrote:
>> >  isn't it easier to set
>> > up time timeout at the beginning?
>>
>> Not if you use normal read/write to talk to usb endpoints (which
>> seems to me a Good Thing).  Normal read/write system call doesn't
>> have a timeout argument.
>
> do you mean "normal read/write" vs. an rpc protocol, say, like
> /dev/sdXX/raw?
>
> - erik
>
>

Re: [9fans] new usb stack and implicit timeouts

[Richard Miller]

> Normal read/write system call doesn't
>> have a timeout argument.
>
> do you mean "normal read/write" vs. an rpc protocol, say, like
> /dev/sdXX/raw?

No, I meant "normal read/write" vs more complicated things like select()
in other operating systems.

Re: [9fans] new usb stack and implicit timeouts

[cinap_lenrek]

[-- Attachment #1: Type: text/plain, Size: 74 bytes --]

http://www.beyondlogic.org/usbnutshell/usb6.htm#SetupPacket

--
cinap

[-- Attachment #2: Type: message/rfc822, Size: 4208 bytes --]

From: Francisco J Ballesteros <nemo@lsub.org>
To: Fans of the OS Plan 9 from Bell Labs <9fans@9fans.net>
Subject: Re: [9fans] new usb stack and implicit timeouts
Date: Sun, 19 Jul 2009 17:14:10 +0200
Message-ID: <8ccc8ba40907190814g652f88f6u817a3085b563fdf7@mail.gmail.com>

that's what I understood.
In any case I'll run the code through all devices I have before
sending any usb patch. I'm still not sure that some disks currently
working won't cease working if they do their own timeouts. I just
want to be sure.


I placed timeouts there only when I found uncooperative devices, in practice.
In theory, not even ctl timeouts are needed. (I should get
crc/timeout errors even in those cases according to the std).

but I have learned the hard way not to trust any usb std.

On Sun, Jul 19, 2009 at 4:32 PM, erik quanstrom<quanstro@quanstro.net> wrote:
>> >  isn't it easier to set
>> > up time timeout at the beginning?
>>
>> Not if you use normal read/write to talk to usb endpoints (which
>> seems to me a Good Thing).  Normal read/write system call doesn't
>> have a timeout argument.
>
> do you mean "normal read/write" vs. an rpc protocol, say, like
> /dev/sdXX/raw?
>
> - erik
>
>

Re: [9fans] new usb stack and implicit timeouts

[Francisco J Ballesteros]

On Sun, Jul 19, 2009 at 5:46 PM, <cinap_lenrek@gmx.de> wrote:
> http://www.beyondlogic.org/usbnutshell/usb6.htm#SetupPacket
>

IIRC, I think the host controller is responsible for timing out
requests sent to the device (I refer to setup packets), but my uchi
does not. In any case, I don't think anyone wants to remove timeouts
from ctl requests.

Re: [9fans] new usb stack and implicit timeouts

[Gorka Guardiola]

On Sun, Jul 19, 2009 at 5:58 PM, Francisco J Ballesteros<nemo@lsub.org> wrote:
> On Sun, Jul 19, 2009 at 5:46 PM, <cinap_lenrek@gmx.de> wrote:
>> http://www.beyondlogic.org/usbnutshell/usb6.htm#SetupPacket
>>
>
> IIRC, I think the host controller is responsible for timing out
> requests sent to the device (I refer to setup packets), but my uchi
> does not. In any case, I don't think anyone wants to remove timeouts
> from ctl requests.
>
>

I am unsure I would remove timeouts even from bulk endpoints.
It is true that some devices (the usb/serial for example) need to
read for an undefined time waiting for data, but I don't think that is
an issue as long
as the timeouts are long enough, doing polling is quite easy. There is
polling in the
lower levels anyway.

On the other hand, I think smart card readers go for
lunch on a read and may never come
back if there is no timeout. Of course alarm() can be used, but
a timeout makes it simpler. I prefer having to poll on some
cases than having to use signals on others.

--
- curiosity sKilled the cat

Re: [9fans] new usb stack and implicit timeouts

[Dan Cross]

Pardon me if this is totally ignorant, but can't we just have a ctl
message to control a timeout, which applications may then set on their
own?

On Mon, Jul 20, 2009 at 3:12 AM, Gorka Guardiola<paurea@gmail.com> wrote:
> On Sun, Jul 19, 2009 at 5:58 PM, Francisco J Ballesteros<nemo@lsub.org> wrote:
>> On Sun, Jul 19, 2009 at 5:46 PM, <cinap_lenrek@gmx.de> wrote:
>>> http://www.beyondlogic.org/usbnutshell/usb6.htm#SetupPacket
>>>
>>
>> IIRC, I think the host controller is responsible for timing out
>> requests sent to the device (I refer to setup packets), but my uchi
>> does not. In any case, I don't think anyone wants to remove timeouts
>> from ctl requests.
>>
>>
>
> I am unsure I would remove timeouts even from bulk endpoints.
> It is true that some devices (the usb/serial for example) need to
> read for an undefined time waiting for data, but I don't think that is
> an issue as long
> as the timeouts are long enough, doing polling is quite easy. There is
> polling in the
> lower levels anyway.
>
> On the other hand, I think smart card readers go for
> lunch on a read and may never come
> back if there is no timeout. Of course alarm() can be used, but
> a timeout makes it simpler. I prefer having to poll on some
> cases than having to use signals on others.
>
> --
> - curiosity sKilled the cat
>
>

Re: [9fans] new usb stack and implicit timeouts

[Francisco J Ballesteros]

That is what I was considering instead of alarm-threadnotify
(provided that the time out code in the kernel has to be kept
there, for ctl transfers).

But I'd like to provide the interface as it should be (otherewise there
will be surprises).

On Mon, Jul 20, 2009 at 5:07 PM, Dan Cross<crossd@gmail.com> wrote:
> Pardon me if this is totally ignorant, but can't we just have a ctl
> message to control a timeout, which applications may then set on their
> own?
>
> On Mon, Jul 20, 2009 at 3:12 AM, Gorka Guardiola<paurea@gmail.com> wrote:
>> On Sun, Jul 19, 2009 at 5:58 PM, Francisco J Ballesteros<nemo@lsub.org> wrote:
>>> On Sun, Jul 19, 2009 at 5:46 PM, <cinap_lenrek@gmx.de> wrote:
>>>> http://www.beyondlogic.org/usbnutshell/usb6.htm#SetupPacket
>>>>
>>>
>>> IIRC, I think the host controller is responsible for timing out
>>> requests sent to the device (I refer to setup packets), but my uchi
>>> does not. In any case, I don't think anyone wants to remove timeouts
>>> from ctl requests.
>>>
>>>
>>
>> I am unsure I would remove timeouts even from bulk endpoints.
>> It is true that some devices (the usb/serial for example) need to
>> read for an undefined time waiting for data, but I don't think that is
>> an issue as long
>> as the timeouts are long enough, doing polling is quite easy. There is
>> polling in the
>> lower levels anyway.
>>
>> On the other hand, I think smart card readers go for
>> lunch on a read and may never come
>> back if there is no timeout. Of course alarm() can be used, but
>> a timeout makes it simpler. I prefer having to poll on some
>> cases than having to use signals on others.
>>
>> --
>> - curiosity sKilled the cat
>>
>>
>
>

Re: [9fans] new usb stack and implicit timeouts

[Lyndon Nerenberg]

> I am unsure I would remove timeouts even from bulk endpoints.
> It is true that some devices (the usb/serial for example) need to
> read for an undefined time waiting for data, but I don't think that is
> an issue as long
> as the timeouts are long enough,

Please show us the algorithm that *correctly* determines 'long enough' for
every application talking to the devices.

Re: [9fans] new usb stack and implicit timeouts

[Dan Cross]

fd = open("/some/ctl", OWRITE);
write(fd, "timeout LONG_ENOUGH", length);
close(fd);

On Mon, Jul 20, 2009 at 2:10 PM, Lyndon Nerenberg<lyndon@orthanc.ca> wrote:
>> I am unsure I would remove timeouts even from bulk endpoints.
>> It is true that some devices (the usb/serial for example) need to
>> read for an undefined time waiting for data, but I don't think that is
>> an issue as long
>> as the timeouts are long enough,
>
> Please show us the algorithm that *correctly* determines 'long enough' for
> every application talking to the devices.
>
>

Re: [9fans] new usb stack and implicit timeouts

[Francisco J Ballesteros]

In the end, I'm going to let the user driver issue a
timeout control request to devusb to activate timeouts
if desired. Instead of forcing it to rely on alarm/threadnotify.

The main reason is that the FS machinery used by some drivers
may use different processes for different requests. Using
notify would require installing handlers on them, but they
are not `owned' by the driver. It's just that they call driver
routines to implement read/write when the file belongs
to the driver.

Considering also that the tmout code in the kernel has
to be there in any case to abort control requests cleanly,
I think the easiest thing is to let the user activate this
code for other types of endpoint (besides control ones).

Perhaps I'm missing something. If anyone can see I'm
making a mistake, I'd like to learn that. This is part of
the interface for drivers and once we are happy with it
it's likely to stay that way for a while.