Skip to contentSkip to conversation

drivers/scsi/advansys.c check_region() fix

PATCH In review Omkhar Arasaratnam 3 participants 3 messages Dec 30, 2003 – Jan 8, 2004

Subsystems (2)
SCSI Advansys SCSI driver
lore
Share & tools
lore

Omkhar Arasaratnam

author
#0 lore raw

PATCH 1/1 drivers/scsi/advansys.c check_region() fix

Another trivial check_region() fix verified by Gene

drivers/scsi/advansys.c renamed from drivers/scsi/advansys.c.org +6 -3
@@ -4619,7 +4619,7 @@
                         ASC_DBG1(1,
                                 "advansys_detect: probing I/O port 0x%x...\n",
                             iop);
-                        if (check_region(iop, ASC_IOADR_GAP) != 0) {
+                        if (!request_region(iop, ASC_IOADR_GAP, "advansys")) {
                             printk(
 "AdvanSys SCSI: specified I/O Port 0x%X is busy\n", iop);
                             /* Don't try this I/O port twice. */

@@ -4630,6 +4630,7 @@
 "AdvanSys SCSI: specified I/O Port 0x%X has no adapter\n", iop);
                             /* Don't try this I/O port twice. */
                             asc_ioport[ioport] = 0;
+			    release_region(iop, ASC_IOADR_GAP);
                             goto ioport_try_again;
                         } else {
                             /*

@@ -4647,6 +4648,7 @@
                                   * 'ioport' past this board.
                                   */
                                  ioport++;
+				 release_region(iop, ASC_IOADR_GAP);
                                  goto ioport_try_again;
                             }
                         }

@@ -10003,10 +10005,11 @@
     }
     for (; i < ASC_IOADR_TABLE_MAX_IX; i++) {
         iop_base = _asc_def_iop_base[i];
-        if (check_region(iop_base, ASC_IOADR_GAP) != 0) {
+        if (!request_region(iop_base, ASC_IOADR_GAP, "advansys")) {
             ASC_DBG1(1,
-               "AscSearchIOPortAddr11: check_region() failed I/O port 0x%x\n",
+               "AscSearchIOPortAddr11: request_region() failed I/O port 0x%x\n",
                      iop_base);
+	    release_region(iop_base, ASC_IOADR_GAP);
             continue;
         }
         ASC_DBG1(1, "AscSearchIOPortAddr11: probing I/O port 0x%x\n", iop_base);

O

#1 lore raw

In message <[email protected]> you write:

Quoted 1 line

Another trivial check_region() fix verified by Gene

And almost certainly wrong.

The *point* of request_region() is that you do it before any I/O to the region.

So you can't release it before calling AscGetChipVersion().

Converting this driver is quite a bit of work, since you have to trace down every path which uses the region and make sure it's covered. The fact that it's formatted like an angry haiku doesn't help.

Rusty.
--
Anyone who quotes me in their sig is an idiot. -- Rusty Russell.

#2 lore raw
Quoted 4 lines · Rusty Russell

In message <[email protected]> you write:

Quoted 1 line

Another trivial check_region() fix verified by Gene

And almost certainly wrong.

I'm the "Gene", Rusty. What symptoms would I see if its wrong? Memory leak? Instability? My dog falls over before his time? My pickup throws a rod?

In my case, its only running a small 4 tape Seagate changer, so it doesn't get a lot of exersize other than by amanda's nightly run.

Quoted 9 lines

The *point* of request_region() is that you do it before any I/O to the region.

So you can't release it before calling AscGetChipVersion().

Converting this driver is quite a bit of work, since you have to trace down every path which uses the region and make sure it's covered. The fact that it's formatted like an angry haiku doesn't help.

I noticed, gawd, that looks like a bowl of spagetti and I'm nowhere near the coder you fellows are. But it did get rid of the compiler warnings about check_region().

Signature · 7 lines
Cheers, Gene
AMD K6-III@500mhz 320M
Athlon1600XP@1400mhz  512M
99.22% setiathome rank, not too shabby for a WV hillbilly
Yahoo.com attornies please note, additions to this message
by Gene Heskett are:
Copyright 2003 by Maurice Eugene Heskett, all rights reserved.