sane-project-website/old-archive/1999-07/0036.html

<!-- received="Mon Jul  5 14:39:34 1999 PDT" -->
<!-- sent="Mon, 05 Jul 1999 23:22:52 +0200" -->
<!-- name="abel deuring" -->
<!-- email="a.deuring@satzbau-gmbh.de" -->
<!-- subject="two patches for sanei_scsi.c" -->
<!-- id="" -->
<!-- inreplyto="" -->
<title>sane-devel: two patches for sanei_scsi.c</title>
<h1>two patches for sanei_scsi.c</h1>
<b>abel deuring</b> (<a href="mailto:a.deuring@satzbau-gmbh.de"><i>a.deuring@satzbau-gmbh.de</i></a>)<br>
<i>Mon, 05 Jul 1999 23:22:52 +0200</i>
<p>
<ul>
<li> <b>Messages sorted by:</b> <a href="date.html#36">[ date ]</a><a href="index.html#36">[ thread ]</a><a href="subject.html#36">[ subject ]</a><a href="author.html#36">[ author ]</a>
<!-- next="start" -->
<li> <b>Next message:</b> <a href="0037.html">Juergen Scherer: "Microtek V600"</a>
<li> <b>Previous message:</b> <a href="0035.html">Jonathan A. Buzzard: "Re: UMAX parallel"</a>
<!-- nextthread="start" -->
<!-- reply="end" -->
</ul>
<!-- body="start" -->
Hi all!<br>
<p>
After Doug Gilbert wrote on this list that his SG driver already <br>
is able to handle queued SCSI commands, I decided to look closer <br>
into sanei_scsi.c and into Doug's documentation of the SG driver.<br>
<p>
The results are two patches; both should compile cleanly with <br>
the old and the new SG driver.<br>
<p>
The first patch listed below adds some code to sanei_scsi_open<br>
in order to detect if the new or the old SG driver is being used,<br>
and it tries to enable command queueing. If the new driver is <br>
found, the patch tries to increase the size of the buffer <br>
reserved by the SG driver. sanei_scsi_max_request_size (the <br>
variable which formerly held the value of SG_BIG_BUFF) is set<br>
to the buffer size returned by the sg driver as the actual <br>
reserved buffer size. This should resolve the problem that <br>
Sane uses a smaller buffer with the new SG driver than <br>
neccessary. Unfortunately, this method to set <br>
sanei_scsi_max_request_size did not work with the SG driver<br>
version 2.1.31, only with 2.1.34. (To be precise: I did the<br>
work on a Suse Linux 6.1 installation; kernel version 2.2.7)<br>
<p>
The second patch is both more interesting and more questionable:<br>
<p>
With this patch, sanei_scsi_req_enter immediately calls issue(),<br>
which means that the SCSI command is immediately sent to the <br>
scanner. Under certain circumstances, this can result in a <br>
considerable performance gain:<br>
<p>
With my P166 box, which has a SCSI disc with a Linux installation <br>
and a Sharp JX250 connected to an Adaptec 2940, an A4-size gray <br>
scale scan with 400 dpi needs approximately 20 seconds with the old<br>
sanei_scsi_req_enter implementation; with the patched version<br>
the scan time decreases to 12..13 seconds. (A note for people<br>
looking for over-all speed figures: This _not_ the "cycle time"<br>
for consecutive scans; it is the output of a command like<br>
"time scanimage [options] &gt; /tmp/testfile". For the "cycle time",<br>
one must add the time needed by the scanner to move the carriage<br>
back.) This shows the impressing capabilities of Doug's work --<br>
and it shows, considering the surprisingly small modifications<br>
reqired, that David prepared already everything in sanei_scsi.c<br>
for command queueing.<br>
<p>
Now for the questionable aspect of this patch: Since the commands<br>
are sent immediately to the scanner by sanei_scsi_req_enter, the <br>
scanner must be able to buffer these commands. If this fails, <br>
both the scanner and the SG driver (or something else inside the<br>
kernel) may become completely confused. This is the case for the<br>
JX250, if more than two commands are queued. (A hint for JX250 <br>
users: make sure that in the file "sharp.conf", "option readqueue" <br>
is set to 2, if you want to try this patch. If the patch should <br>
find its way into the Sane package despite my own doubts about it, <br>
I will remove this option, of course.)<br>
<p>
In other words: The patch makes assumptions about the scanners<br>
and the backends which are not neccessarily fulfilled. At present,<br>
the patch should affect Mustek-, HP- and Sharp-scanners; the other<br>
backends don't use sanei_scsi_req_enter, and should therefore not<br>
be affected.<br>
<p>
So, dear readers and co-developers, what do you think about <br>
this patch? Personally, I would prefer an implementation of <br>
command queueing, where the SG driver buffers the commands, <br>
and sends the first command in the queue from within a call to <br>
sg_read for the previous SCSI command -- but, as the Rolling <br>
Stones pointed out some years ago: You can't always get what you <br>
want. To avoid misunderstandings: This is not a critisism on <br>
Doug's work -- he has done a great job with the SG driver. But<br>
its capabilities put up a difficult question how to deal with <br>
them in sanei_scsi.c.<br>
<p>
Abel<br>
<p>
PS: Doug, thanks for your quick answer to my questions about<br>
command queueing with the SG driver!<br>
<p>
-------------- Patch 1: ------------------<br>
<p>
--- sanei_scsi.c	Mon Jul  5 22:15:22 1999<br>
+++ sanei_scsi.c.patch1	Mon Jul  5 22:24:41 1999<br>
@@ -194,6 +194,22 @@<br>
 #endif<br>
 <br>
 int sanei_scsi_max_request_size = MAX_DATA;<br>
+#if USE == LINUX_INTERFACE<br>
+/* to following #defines follow Doug Gilbert's sample code<br>
+   to achieve run time compatibility with the old and the<br>
+   new SG driver for Linux<br>
+*/<br>
+#ifndef SG_SET_COMMAND_Q<br>
+#define SG_SET_COMMAND_Q 0x2271<br>
+#endif<br>
+#ifndef SG_SET_RESERVED_SIZE<br>
+#define SG_SET_RESERVED_SIZE 0x2275<br>
+#endif <br>
+#ifndef SG_GET_RESERVED_SIZE<br>
+#define SG_GET_RESERVED_SIZE 0x2272<br>
+#endif <br>
+int have_new_sg = 0;<br>
+#endif<br>
 <br>
 #if USE == FREEBSD_CAM_INTERFACE<br>
 # define CAM_MAXDEVS	128<br>
@@ -913,6 +929,33 @@<br>
       }<br>
   }<br>
 #endif /* SGIOCSTL */<br>
+#if USE == LINUX_INTERFACE<br>
+  {<br>
+    /* check for the SG version<br>
+    */<br>
+    <br>
+    int ioctl_val = 1;<br>
+    /* try to enable command queueing. If this is successful, assume<br>
+       that the new SG driver is installed, else assume that we<br>
+       have the old driver<br>
+    */<br>
+    if(0 == ioctl(fd, SG_SET_COMMAND_Q, &amp;ioctl_val)) <br>
+      {<br>
+        have_new_sg = 1;<br>
+        DBG(1, "sanei_scsi_open: command queueing enabled\n");<br>
+<br>
+        /* try to reserve a bigger SG buffer */<br>
+        ioctl_val = 32 * 4096;<br>
+        ioctl(fd, SG_SET_RESERVED_SIZE, &amp;ioctl_val);<br>
+<br>
+        if (0 == ioctl(fd, SG_GET_RESERVED_SIZE, &amp;ioctl_val)) <br>
+          sanei_scsi_max_request_size = ioctl_val;<br>
+        else<br>
+          DBG(1, "sanei_scsi_open: cannot read SG buffer size - %s\n",<br>
strerror(errno));<br>
+        DBG(1, "sanei_scsi_open: using %i bytes as SCSI buffer\n",<br>
sanei_scsi_max_request_size);<br>
+      }<br>
+  }<br>
+#endif<br>
 #endif /* !DECUNIX_INTERFACE */<br>
 <br>
   if (fd &gt;= num_alloced)<br>
<p>
<p>
---------- Patch 2 (to be applied to sanei_scsi.c ------------<br>
---------- containing the changes from patch 1): -------------<br>
<p>
--- sanei_scsi.c	Mon Jul  5 22:24:41 1999<br>
+++ sanei_scsi.c.patch2	Mon Jul  5 22:15:22 1999<br>
@@ -1290,16 +1290,39 @@<br>
 static void<br>
 issue (struct req *req)<br>
 {<br>
-  ssize_t nwritten;<br>
+  ssize_t nwritten, count = 0;<br>
 <br>
   if (!req || req-&gt;running)<br>
     return;<br>
 <br>
   DBG (4, "sanei_scsi.issue: %p\n", req);<br>
 <br>
-  ATOMIC (req-&gt;running = 1;<br>
-	  nwritten = write (req-&gt;fd, &amp;req-&gt;cdb, req-&gt;cdb.hdr.pack_len));<br>
-<br>
+  if (!have_new_sg)<br>
+    {<br>
+      ATOMIC (req-&gt;running = 1;<br>
+              nwritten = write (req-&gt;fd, &amp;req-&gt;cdb,<br>
req-&gt;cdb.hdr.pack_len));<br>
+    }<br>
+  else <br>
+    {<br>
+      while (count &lt; 100) <br>
+        {<br>
+          ATOMIC (nwritten = write (req-&gt;fd, &amp;req-&gt;cdb,<br>
req-&gt;cdb.hdr.pack_len);<br>
+                  if (nwritten !=req-&gt;cdb.hdr.pack_len <br>
+                      &amp;&amp; errno == EAGAIN<br>
+                      &amp;&amp; count &lt; 100)<br>
+                    count++;<br>
+                  else <br>
+                    req-&gt;running = 1;<br>
+                 );<br>
+          if (!req-&gt;running) <br>
+            {<br>
+              usleep(10000);<br>
+              DBG(2, "sanei_scsi.issue: write call returned EAGAIN\n");<br>
+            }<br>
+          else<br>
+            break;<br>
+        }<br>
+    }<br>
   if (nwritten != req-&gt;cdb.hdr.pack_len)<br>
     {<br>
       DBG (1, "sanei_scsi.issue: bad write (errno=%s)\n",<br>
@@ -1382,6 +1405,8 @@<br>
   DBG (4, "scsi_req_enter: entered %p\n", req);<br>
 <br>
   *idp = req;<br>
+  if (have_new_sg)<br>
+    issue(req);<br>
   return SANE_STATUS_GOOD;<br>
 }<br>
<p>
<pre>
--
Source code, list archive, and docs: <a href="http://www.mostang.com/sane/">http://www.mostang.com/sane/</a>
To unsubscribe: echo unsubscribe sane-devel | mail <a href="mailto:majordomo@mostang.com">majordomo@mostang.com</a>
</pre>
<!-- body="end" -->
<p>
<ul>
<!-- next="start" -->
<li> <b>Next message:</b> <a href="0037.html">Juergen Scherer: "Microtek V600"</a>
<li> <b>Previous message:</b> <a href="0035.html">Jonathan A. Buzzard: "Re: UMAX parallel"</a>
<!-- nextthread="start" -->
<!-- reply="end" -->
</ul>