Jon Wikne wrote:
> 
> Stephan Feder <[EMAIL PROTECTED]> wrote:
> 
> > Pieter Nagel wrote:
> > ...
> > > However, I believe for 2.5 the loop should be rewritten as retry loop,
> > > and the function should immediately return whatever it reads, like the
> > > majority of drivers do. As a side effect, the number of retries on
> > > TIMEOUT/NAK would change, so the TIMEOUT values for various scanners
> > > would need to be retuned based on experience.
> >
> > The scanner module does blocking IO. You cannot change it to any kind of
> > nonblocking IO without breaking applications. It would be wise to ask
> > application developers first, e.g. on the SANE developer mailing list
> > ([EMAIL PROTECTED]).

I said "nonblocking" which is not what I meant: read_scanner tries to
read (i.e. requests from hardware) exactly as many bytes as specified by
the caller. It only returns earlier because of an error (short reads are
not considered errors and are handled appropiately) or a timeout.
 
> FYI, FWIW: This is not merely a Sane issue. This is a _kernel_

Pieter said: "...the function should immediately return whatever it
reads...". This would change the semantics of read_scanner and would
break application code, which Sane was just an example for.

> issue. The problem in question causes a kernel Oops, a complete
> hang of the kernel USB scanner module, and is present also with
> _another_ application program, i.e. Vuescan.

I do not see how a kernel oops could be caused by the code in scanner.c.
Could you give a pointer to the offending piece of code?

> > Pieter Nagel wrote:
> > ...
> > Cause: read_scanner() sits in a loop, trying to bulk read all 'count'
> > bytes of data. It erroneously decrements bytes remaining by 'this_read',
> > what it asked for, instead of 'partial', what it got. 

>From scanner.c:

            count -= this_read; /* Compensate for short reads */

I would say that the behaviour you describe is quite on purpose.

> > For large reads (spanning multiple slow stepper-motor steps?) the
> > scanner sometimes returns a short read, which makes 'count' shrink

Is there really a scanner out there that transmits, let's say, only 53
instead of 64 bytes because the stepper-motor has to do some more
work??? Good luck to the application developer for that kind of
hardware.

> > faster than it should, causing an attempt to read less than
> > wMaxPacketSize bytes to be read before the last packet of the bulk
> > transfer, causing EOVERFLOW.  

>From scanner.c:

            this_read = (count >= IBUF_SIZE) ? IBUF_SIZE : count;

As IBUF_SIZE is a multiple of wMaxPacketSize all requested packet sizes
will be  except for the very last packet.

Maybe your application has the following problem:

It wants to read

        max   max short max   max   short
        64    64  30    64    64    30

and requests 64 + 64 + 30 + 64 + 64 + 30 = 316 bytes instead of 6 * 64 =
384 bytes.

Regards,
        Stephan

PS: I have attached a short patch (kludge!) for scanner.c against kernel
2.4.18 that preserves the current semantics.

You should set SCANNER_URB_SIZE to your scanner's wMaxPacketSize, and
maybe you have to replace 
        out->urb.transfer_flags = USB_QUEUE_BULK;
with
        out->urb.transfer_flags = USB_QUEUE_BULK | USB_DISABLE_SPD;

Does it help in any way?

Regards,
        Stephan

Attachment: scanner.c.diff
Description: Binary data

Reply via email to