> > I'll take care for a graceful handling of the condition as well as support
> > for the new sg driver in 2.2.6 and up.
> Sorry for the delay with my response to Andreas' mail, but I was
> on holidays the last weeks.
No problem. Fortunately I was very busy, so no double work done yet.
> I have already made some work on the support for the new Linux
> SG driver in sanei_scsi.c, in order to use the capabilites of
> this driver to change its buffer size dynamically, and to enable
> low level command queueing.
Great. Very good. Your code below looks good as well, especially, as you
already talked to Douglas to avoid any misconceptions and stuff.
> Two details of the modifications would need an improvement:
> 1. It would be fine to have the buffer size user
> configurable. This could be done either with a configuration
> file, or with an environment variable.
You mean how much space it should try to reserve ?
IMHO we should enhance the sanei_scsi API to be able to negotiate that
at sanei_scsi_open()-time. That would kill off your problem #2 as well.
I would suggest to add something along the lines of a , int *maxsize);
which would be filled with the desired size at call time and with the
guaranteed minimum size when it returns. That does away with that ugly
global variable sanei_scsi_max_request_size.
For compatibility with existing backends, we could keep the global
variable with the minimum value ever obtained and wrap the new call
up like:
sanei_scsi_open (const char *dev, int *fdp,
SANEI_SCSI_Sense_Handler handler, void *handler_arg)
{
return sanei_scsi_open_extended (dev, fdp, handler, handler_arg,
&sanei_scsi_max_request_size);
}
This of course requires, that sanei_scsi_max_request_size is set small
enough at startup to avoid problems with backends that make assumptions
about it even _before_ calling sanei_scsi_open();.
> 2. The new SG driver allocates a separate buffer for each
> file handle. This means, that a situation is possible, where
> the ioctl call to modify the buffer size is successful for
> one file handle, but it fails for another.
Yes. The above modification would cover that - right ?
> this inconsistency for now. It should of course be fixed, if
> these patches will be accepted.
Yep.
A thing I miss with your patch - oh stop - I see it was corrected in
1.0.1. already (I'm talking about the static buffer allocation with
size SG_BIG_BUFF in SANE 1.0.0 that caused very weird problems for me).
Great.
> + ioctl_val = 128 * 1024;
Yep. I'd like to see that configureable. I'd do it inside the backend,
which might e.g. know, the scanner can't handle >64k anyway and thus it can
choose not to waste ressources. Most backends parse options anyway, so
another "buffersize=256k" option won't cause much grief.
> + ioctl(fd, SG_SET_RESERVED_SIZE, &ioctl_val);
> + if (0 == ioctl(fd, SG_GET_RESERVED_SIZE, &ioctl_val))
> + sanei_scsi_max_request_size = ioctl_val;
What exactly happens when the set fails ? Downgrade to maximum possible
value, or keep default ?
We should try to get the former, eventually using a loop.
Other than that, I really like your patch. Looks very promising. Pity I
can't test ist too soon. But definitely at the next servicing interval
of the server that has the scanner.
I'd say we should collect some test data from a few people, and apply it
for now, if it works well.
CU, Andy
-- = Andreas Beck | Email : <andreas.beck@ggi-project.org> =
-- Source code, list archive, and docs: http://www.mostang.com/sane/ To unsubscribe: echo unsubscribe sane-devel | mail majordomo@mostang.com