linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Aic7xxx and Aic79xx Driver Updates
@ 2003-05-01 22:28 Justin T. Gibbs
  2003-05-02  0:17 ` Willy Tarreau
  2003-05-02 14:30 ` James Bottomley
  0 siblings, 2 replies; 12+ messages in thread
From: Justin T. Gibbs @ 2003-05-01 22:28 UTC (permalink / raw)
  To: linux-scsi; +Cc: linux-kernel, Linus Torvalds, Alan Cox, Marcelo Tosatti

Folks,

I've just uploaded version 1.3.8 of the aic79xx driver and version 
6.2.33 of the aic7xxx driver.  Both are available for 2.4.X and
2.5.X kernels in either bk send format or as a tarball from here:

http://people.FreeBSD.org/~gibbs/linux/SRC/

RPMs and DUDs for various distributions are also available:

http://people.FreeBSD.org/~gibbs/linux/DUD/aic7xxx/
http://people.FreeBSD.org/~gibbs/linux/DUD/aic79xx/
http://people.FreeBSD.org/~gibbs/linux/RPM/aic7xxx/
http://people.FreeBSD.org/~gibbs/linux/RPM/aic79xx/

BK changes relative to top of the 2.5.X tree are listed below.

--
Justin

ChangeSet
  1.1118.33.8 03/05/01 11:06:21 gibbs@overdrive.btc.adaptec.com +3 -0
  Aic7xxx Driver Update (6.2.33)
   o Correct MODULE_INFO string.
   o Bump version number.

ChangeSet
  1.1118.33.7 03/05/01 11:04:13 gibbs@overdrive.btc.adaptec.com +2 -0
  Update Aic79xx and Aic7xxx Documenation

ChangeSet
  1.1118.33.6 03/05/01 11:00:31 gibbs@overdrive.btc.adaptec.com +5 -0
  Aic79xx Driver Update (version 1.3.8)
   o Correct a few BE processor bugs
   o Print an additional diagnostic during recovery processing

ChangeSet
  1.1118.33.5 03/04/24 15:12:48 gibbs@overdrive.btc.adaptec.com +7 -0
  Aic7xxx and Aic79xx Driver Updates
   o Adapt to new IRQ handler declaration/behavior for 2.5.X

ChangeSet
  1.1118.33.4 03/04/24 15:10:16 gibbs@overdrive.btc.adaptec.com +2 -0
  Aic79xx and Aic7xxx driver Update
   o Fix build on 2.5.X

ChangeSet
  1.1118.33.3 03/04/24 13:30:58 gibbs@overdrive.btc.adaptec.com +2 -0
  Merge http://linux.bkbits.net/linux-2.5
  into overdrive.btc.adaptec.com:/usr/home/gibbs/bk/linux-2.5

ChangeSet
  1.971.94.14 03/04/24 13:23:49 gibbs@overdrive.btc.adaptec.com +4 -0
  aic7xxx_osm.h, aic7xxx_osm.c, aic79xx_osm.h, aic79xx_osm.c:
    Remove pre-2.2.X kernel support. 

ChangeSet
  1.971.94.13 03/04/24 12:46:43 gibbs@overdrive.btc.adaptec.com +8 -0
  Aic79xx Driver Upate
   o Switch to handling bad SCSI status as a sequencer interrupt
     instead of having the kernel proccess these failures via
     the completion queue.  This is done because:
  
      - The old scheme required us to pause the sequencer and clear
        critical sections for each SCB.  It seems that these pause
        actions, if coincident with a sequencer FIFO interrupt, would
        result in a FIFO interrupt getting lost or directing to the
        wrong FIFO.  This caused hangs when the driver was stressed
        under high "queue full" loads.
      - The completion code assumed that it was always called with
        the sequencer running.  This may not be the case in timeout
        processing where completions occur manually via
        ahd_pause_and_flushwork().
      - With this scheme, the extra expense of clearing critical
        sections is avoided since the sequencer will only self pause
        once all pending selections have cleared and it is not in
        a critical section.

ChangeSet
  1.971.94.12 03/04/24 12:36:46 gibbs@overdrive.btc.adaptec.com +1 -0
  Aic79xx Driver Update
   o Revert ahd_pause_and_flushwork() behavior so that ENSELO can
     be cleared.  This makes ahd_pause_and_flushwork() more effective
     when the bus is hung.

ChangeSet
  1.971.94.11 03/04/24 12:24:31 gibbs@overdrive.btc.adaptec.com +1 -0
  Aic79xx Driver Update
   o Correct "Unexpected PKT Busfree" error observed under high
     tag loads.

ChangeSet
  1.971.94.10 03/04/24 12:15:47 gibbs@overdrive.btc.adaptec.com +7 -0
  Aic79xx Driver Update
   o Perform a few firmware optimizations
   o Correct the packetized status handler so that
     it can handle CRC errors during status data packets.

ChangeSet
  1.971.94.9 03/04/24 12:10:40 gibbs@overdrive.btc.adaptec.com +2 -0
  Aic7xxx Driver Update
   o Auto disable PCI parity error reporting after 10 parity errors
     are observed.  The user is given a loud warning message telling
     them that eiter a device plugged into their motherboard or their
     motherboard is not very healthy.

ChangeSet
  1.971.94.8 03/04/24 12:07:37 gibbs@overdrive.btc.adaptec.com +4 -0
  Aic7xxx and Aic79xx Driver Updates
   o Correct type safty of option parsing logic
   o Make option toggling work correctly
   o Add "probe_eisa_vlb" as an alias for the "no_probe" option so
     that there is a clearly defined name associated with the command
     line feature that allows eisa_vlb probes to be enabled/disabled
     in the aic7xxx driver.
   o PCI parity error checking defaults to being enabled.

ChangeSet
  1.971.94.7 03/04/24 11:53:55 gibbs@overdrive.btc.adaptec.com +2 -0
  Aic7xxx and Aic79xx driver Update
   o Fix style nits.

ChangeSet
  1.971.94.6 03/04/24 11:49:01 gibbs@overdrive.btc.adaptec.com +2 -0
  Aic7xxx and Aic79xx driver updates
   o Remove extra complexity and code duplication in processing
     the completeq now that the completeq can be run while holding
     both the ah?_lock and the done_lock.

ChangeSet
  1.971.94.5 03/04/24 11:46:55 gibbs@overdrive.btc.adaptec.com +2 -0
  Aic7xxx and Aic79xx driver updates
   o Work around peculiarities in the scan_scsis routines
     that could, due to having duplicate devices on our
     host's device list, cause tagged queing to be disabled
     for devices added via /proc.

ChangeSet
  1.971.94.4 03/04/24 11:42:36 gibbs@overdrive.btc.adaptec.com +2 -0
  Aic7xxx and Aic79xx Driver Update
   o Correct channel information in our /proc output.

ChangeSet
  1.971.94.3 03/04/24 11:24:15 gibbs@overdrive.btc.adaptec.com +6 -0
  Aic7xxx and Aic79xx driver Update
  o Avoid pre-2.5.X mid-layer deadlock due to SCSI malloc fragmentation
  
  For pre-2.5.X kernels, attempt to calculate a safe value
  for our S/G list length.  In these kernels, the midlayer
  allocates an S/G array dynamically when a command is issued
  using SCSI malloc.  This list, which is in an OS dependent
  format that must later be copied to our private S/G list, is
  sized to house just the number of segments needed for the
  current transfer.  Since the code that sizes the SCSI malloc
  pool does not take into consideration fragmentation of the
  pool, executing transactions numbering just a fraction of our
  concurrent transaction limit with list lengths aproaching
  AH?_NSEG in length will quickly depleat the SCSI malloc pool
  of usable space.
  
  Unfortunately, the mid-layer does not properly handle this
  scsi malloc failure.  In kernels prior to 2.4.20, should
  the device that experienced the malloc failure be idle and
  never have any new I/O initiated (block queue is not "kicked"),
  the process will hang indefinitely.  In 2.4.20 and beyond,
  the disk experiencing the failure is marked as a "starved
  device", but this only helps if I/O is initiated to or completes
  on that HBA.  If the failure was induced by another HBA, and
  no other I/O is pending on the HBA and no new transactions are
  queued, we are still succeptible to the hang.  (Also note that
  many 2.4.X kernels do not properly lock the "some_device_starved"
  and "device_starved" fields calling into question their overall
  effectiveness).
  
  By sizing our S/G list to avoid SCSI malloc pool fragmentation,
  we will hopefully avoid this deadlock at least for configurations
  where our own HBAs are the only ones using the SCSI subsystem.

ChangeSet
  1.971.94.2 03/04/09 18:12:31 gibbs@overdrive.btc.adaptec.com +1 -0
  Aic79xx Driver Update
   o Correct failed-wait recovery code so that the controller's registers
     will not be accessed without pausing the controller first.

ChangeSet
  1.971.94.1 03/04/09 13:07:08 gibbs@overdrive.btc.adaptec.com +1 -0
  Merge http://linux.bkbits.net/linux-2.5
  into overdrive.btc.adaptec.com:/usr/home/gibbs/bk/linux-2.5

ChangeSet
  1.971.37.4 03/04/09 13:01:11 gibbs@overdrive.btc.adaptec.com +4 -0
  Aic7xxx Driver Update (version 6.2.32)
   o Perform an audit on use of del_timer() and switch to del_timer_sync()
     where appropriate.
   o Remove the reboot notifier hook which is unused in 2.5.X.
   o Correct some driver unload bugs.

ChangeSet
  1.971.37.3 03/04/09 12:52:07 gibbs@overdrive.btc.adaptec.com +3 -0
  Aic79xx Driver Update (version 1.3.6)
   o Correct bus hang on SE->LVD/LVD->SE tranceiver changes
   o Close a race condition in handling bad scsi status that could
     allow the driver to modify the waiting for selection queue while
     selections were enabled.
   o Perform an audit on use of del_timer() and switch to del_timer_sync()
     where appropriate.
   o Remove the reboot notifier hook which is unused in 2.5.X.
   o Correct some driver unload bugs.

ChangeSet
  1.971.37.2 03/04/09 11:56:15 gibbs@overdrive.btc.adaptec.com +2 -0
  Change the callback argument for aic brace option parsing to u_long
  to avoid casting problems with different architectures.


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: Aic7xxx and Aic79xx Driver Updates
  2003-05-01 22:28 Aic7xxx and Aic79xx Driver Updates Justin T. Gibbs
@ 2003-05-02  0:17 ` Willy Tarreau
  2003-05-02  4:25   ` Justin T. Gibbs
  2003-05-02 14:30 ` James Bottomley
  1 sibling, 1 reply; 12+ messages in thread
From: Willy Tarreau @ 2003-05-02  0:17 UTC (permalink / raw)
  To: Justin T. Gibbs
  Cc: linux-scsi, linux-kernel, Linus Torvalds, Alan Cox, Marcelo Tosatti

On Thu, May 01, 2003 at 04:28:12PM -0600, Justin T. Gibbs wrote:
> Folks,
> 
> I've just uploaded version 1.3.8 of the aic79xx driver and version 
> 6.2.33 of the aic7xxx driver.  Both are available for 2.4.X and
> 2.5.X kernels in either bk send format or as a tarball from here:
> 
> http://people.FreeBSD.org/~gibbs/linux/SRC/

Hi Justin,

I've just tested it and I still have the deadlock on SMP. I also tried with
noapic, but it didn't change. I have reduced the TCQ from 253 to 32, and I
had the impression that it was more difficult to trigger, although I cannot
be certain. With 32, I could boot and go to about half the 'make -j 8 dep',
while it hanged during init script with 253. I may retest by the week-end, but
now I'm going to sleep. Now I'm back to 6.2.28 and everything's OK.

Regards,
Willy


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: Aic7xxx and Aic79xx Driver Updates
  2003-05-02  0:17 ` Willy Tarreau
@ 2003-05-02  4:25   ` Justin T. Gibbs
  2003-05-02  5:56     ` Willy Tarreau
  0 siblings, 1 reply; 12+ messages in thread
From: Justin T. Gibbs @ 2003-05-02  4:25 UTC (permalink / raw)
  To: Willy Tarreau
  Cc: linux-scsi, linux-kernel, Linus Torvalds, Alan Cox, Marcelo Tosatti

> On Thu, May 01, 2003 at 04:28:12PM -0600, Justin T. Gibbs wrote:
>> Folks,
>> 
>> I've just uploaded version 1.3.8 of the aic79xx driver and version 
>> 6.2.33 of the aic7xxx driver.  Both are available for 2.4.X and
>> 2.5.X kernels in either bk send format or as a tarball from here:
>> 
>> http://people.FreeBSD.org/~gibbs/linux/SRC/
> 
> Hi Justin,
> 
> I've just tested it and I still have the deadlock on SMP. I also tried with
> noapic, but it didn't change. I have reduced the TCQ from 253 to 32, and I
> had the impression that it was more difficult to trigger, although I cannot
> be certain. With 32, I could boot and go to about half the 'make -j 8 dep',
> while it hanged during init script with 253. I may retest by the week-end, but
> now I'm going to sleep. Now I'm back to 6.2.28 and everything's OK.

Can you try with this patch?  It seems I forgot to pull part of a change
from the aic79xx driver into the aic7xxx driver.  This could easily cause
a lock order reversal. <sigh>

==== //depot/aic7xxx/linux/drivers/scsi/aic7xxx/aic79xx_osm.c#159 - /home/gibbs/bk/linux-2.4/drivers/scsi/aic7xxx/aic79xx_osm.c ====
--- /tmp/tmp.29873.0	2003-05-01 22:21:54.000000000 -0600
+++ /home/gibbs/bk/linux-2.4/drivers/scsi/aic7xxx/aic79xx_osm.c	2003-05-01 22:04:07.000000000 -0600
@@ -670,7 +670,6 @@
 		TAILQ_REMOVE(&ahd->platform_data->completeq,
 			     acmd, acmd_links.tqe);
 		cmd = &acmd_scsi_cmd(acmd);
-		acmd = TAILQ_NEXT(acmd, acmd_links.tqe);
 		cmd->host_scribble = NULL;
 		if (ahd_cmd_get_transaction_status(cmd) != DID_OK
 		 || (cmd->result & 0xFF) != SCSI_STATUS_OK)
@@ -1756,9 +1755,11 @@
 		TAILQ_REMOVE(&ahd->platform_data->device_runq, dev, links);
 		dev->flags &= ~AHD_DEV_ON_RUN_LIST;
 		ahd_linux_check_device_queue(ahd, dev);
+#if LINUX_VERSION_CODE >= KERNEL_VERSION(2,4,0)
 		/* Yeild to our interrupt handler */
 		ahd_unlock(ahd, &flags);
 		ahd_lock(ahd, &flags);
+#endif
 	}
 #if LINUX_VERSION_CODE >= KERNEL_VERSION(2,4,0)
 	ahd_unlock(ahd, &flags);
==== //depot/aic7xxx/linux/drivers/scsi/aic7xxx/aic7xxx_osm.c#220 - /home/gibbs/bk/linux-2.4/drivers/scsi/aic7xxx/aic7xxx_osm.c ====
--- /tmp/tmp.29873.1	2003-05-01 22:21:54.000000000 -0600
+++ /home/gibbs/bk/linux-2.4/drivers/scsi/aic7xxx/aic7xxx_osm.c	2003-05-01 22:19:46.000000000 -0600
@@ -664,7 +664,6 @@
 		TAILQ_REMOVE(&ahc->platform_data->completeq,
 			     acmd, acmd_links.tqe);
 		cmd = &acmd_scsi_cmd(acmd);
-		acmd = TAILQ_NEXT(acmd, acmd_links.tqe);
 		cmd->host_scribble = NULL;
 		if (ahc_cmd_get_transaction_status(cmd) != DID_OK
 		 || (cmd->result & 0xFF) != SCSI_STATUS_OK)
@@ -1385,9 +1384,11 @@
 		TAILQ_REMOVE(&ahc->platform_data->device_runq, dev, links);
 		dev->flags &= ~AHC_DEV_ON_RUN_LIST;
 		ahc_linux_check_device_queue(ahc, dev);
+#if LINUX_VERSION_CODE >= KERNEL_VERSION(2,4,0)
 		/* Yeild to our interrupt handler */
 		ahc_unlock(ahc, &flags);
 		ahc_lock(ahc, &flags);
+#endif
 	}
 #if LINUX_VERSION_CODE >= KERNEL_VERSION(2,4,0)
 	ahc_unlock(ahc, &flags);
==== //depot/aic7xxx/linux/drivers/scsi/aic7xxx/aic7xxx_osm.h#140 - /home/gibbs/bk/linux-2.4/drivers/scsi/aic7xxx/aic7xxx_osm.h ====
--- /tmp/tmp.29873.2	2003-05-01 22:21:54.000000000 -0600
+++ /home/gibbs/bk/linux-2.4/drivers/scsi/aic7xxx/aic7xxx_osm.h	2003-05-01 22:21:10.000000000 -0600
@@ -737,7 +737,8 @@
 	 * trade the io_request_lock for our per-softc lock.
 	 */
 #if AHC_SCSI_HAS_HOST_LOCK == 0
-	ahc_lock(ahc, flags);
+	spin_unlock(&io_request_lock);
+	spin_lock(&ahc->platform_data->spin_lock);
 #endif
 }
 
@@ -745,7 +746,8 @@
 ahc_midlayer_entrypoint_unlock(struct ahc_softc *ahc, unsigned long *flags)
 {
 #if AHC_SCSI_HAS_HOST_LOCK == 0
-	ahc_unlock(ahc, flags);
+	spin_unlock(&ahd->platform_data->spin_lock);
+	spin_lock(&io_request_lock);
 #endif
 }
 


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: Aic7xxx and Aic79xx Driver Updates
  2003-05-02  4:25   ` Justin T. Gibbs
@ 2003-05-02  5:56     ` Willy Tarreau
  0 siblings, 0 replies; 12+ messages in thread
From: Willy Tarreau @ 2003-05-02  5:56 UTC (permalink / raw)
  To: Justin T. Gibbs
  Cc: Willy Tarreau, linux-scsi, linux-kernel, Linus Torvalds,
	Alan Cox, Marcelo Tosatti

On Thu, May 01, 2003 at 10:25:31PM -0600, Justin T. Gibbs wrote:
 
> Can you try with this patch?  It seems I forgot to pull part of a change
> from the aic79xx driver into the aic7xxx driver.  This could easily cause
> a lock order reversal. <sigh>

Congratulations, Justin, you just spotted it !

I cannot hang it anymore. It supported an fsck and the make -j dep.
I'm happy, I'll be able to reintegrate your updates to my tree !
I just have changed one typo for it to compile :

> --- /tmp/tmp.29873.2	2003-05-01 22:21:54.000000000 -0600
> +++ /home/gibbs/bk/linux-2.4/drivers/scsi/aic7xxx/aic7xxx_osm.h	2003-05-01 22:21:10.000000000 -0600
> @@ -745,7 +746,8 @@
>  ahc_midlayer_entrypoint_unlock(struct ahc_softc *ahc, unsigned long *flags)
>  {
>  #if AHC_SCSI_HAS_HOST_LOCK == 0
> -	ahc_unlock(ahc, flags);
> +	spin_unlock(&ahd->platform_data->spin_lock);

                    ^^^^ this one is in fact &ahc->platform_data...

Thanks ;-)
Willy


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: Aic7xxx and Aic79xx Driver Updates
  2003-05-01 22:28 Aic7xxx and Aic79xx Driver Updates Justin T. Gibbs
  2003-05-02  0:17 ` Willy Tarreau
@ 2003-05-02 14:30 ` James Bottomley
  2003-05-02 17:51   ` Justin T. Gibbs
  1 sibling, 1 reply; 12+ messages in thread
From: James Bottomley @ 2003-05-02 14:30 UTC (permalink / raw)
  To: Justin T. Gibbs
  Cc: SCSI Mailing List, Linux Kernel, Linus Torvalds, Alan Cox,
	Marcelo Tosatti

First off, could you take a look at

http://bugzilla.kernel.org/show_bug.cgi?id=608

I thought it was an sr problem, but it doesn't seem to show up on
anything other than adaptec controllers?  Thanks.

On Thu, 2003-05-01 at 17:28, Justin T. Gibbs wrote:
> ChangeSet
>   1.1118.33.5 03/04/24 15:12:48 gibbs@overdrive.btc.adaptec.com +7 -0
>   Aic7xxx and Aic79xx Driver Updates
>    o Adapt to new IRQ handler declaration/behavior for 2.5.X

The changes for this:

+#if LINUX_VERSION_CODE >= KERNEL_VERSION(2,5,0)
+#define        AIC_LINUX_IRQRETURN_T irqreturn_t
+#define        AIC_LINUX_IRQRETURN(ours) return (IRQ_RETVAL(ours))
+#else
+#define        AIC_LINUX_IRQRETURN_T void
+#define        AIC_LINUX_IRQRETURN(ours)  return
+#endif

Are rather convoluted.  Could you just remove the wrappering for 2.5?


> ChangeSet
>   1.971.94.5 03/04/24 11:46:55 gibbs@overdrive.btc.adaptec.com +2 -0
>   Aic7xxx and Aic79xx driver updates
>    o Work around peculiarities in the scan_scsis routines
>      that could, due to having duplicate devices on our
>      host's device list, cause tagged queing to be disabled
>      for devices added via /proc.

-ahc_linux_select_queue_depth(struct Scsi_Host * host,
-                            Scsi_Device * scsi_devs)
+ahc_linux_select_queue_depth(struct Scsi_Host *host, Scsi_Device
*scsi_devs)

select_queue_depth isn't a 2.5 interface anymore, why do you even still
need it?

> ChangeSet
>   1.971.94.3 03/04/24 11:24:15 gibbs@overdrive.btc.adaptec.com +6 -0
>   Aic7xxx and Aic79xx driver Update
>   o Avoid pre-2.5.X mid-layer deadlock due to SCSI malloc fragmentation
[...]

This is entirely irrelevant to 2.5 as well.

James



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: Aic7xxx and Aic79xx Driver Updates
  2003-05-02 14:30 ` James Bottomley
@ 2003-05-02 17:51   ` Justin T. Gibbs
  2003-05-02 18:07     ` James Bottomley
  2003-05-07  3:22     ` Lukasz Trabinski
  0 siblings, 2 replies; 12+ messages in thread
From: Justin T. Gibbs @ 2003-05-02 17:51 UTC (permalink / raw)
  To: James Bottomley
  Cc: SCSI Mailing List, Linux Kernel, Linus Torvalds, Alan Cox,
	Marcelo Tosatti

> First off, could you take a look at
> 
> http://bugzilla.kernel.org/show_bug.cgi?id=608
> 
> I thought it was an sr problem, but it doesn't seem to show up on
> anything other than adaptec controllers?  Thanks.

I've just updated the bug.

> On Thu, 2003-05-01 at 17:28, Justin T. Gibbs wrote:
>> ChangeSet
>>   1.1118.33.5 03/04/24 15:12:48 gibbs@overdrive.btc.adaptec.com +7 -0
>>   Aic7xxx and Aic79xx Driver Updates
>>    o Adapt to new IRQ handler declaration/behavior for 2.5.X
> 
> The changes for this:
> 
> +#if LINUX_VERSION_CODE >= KERNEL_VERSION(2,5,0)
> +#define        AIC_LINUX_IRQRETURN_T irqreturn_t
> +#define        AIC_LINUX_IRQRETURN(ours) return (IRQ_RETVAL(ours))
> +#else
> +#define        AIC_LINUX_IRQRETURN_T void
> +#define        AIC_LINUX_IRQRETURN(ours)  return
> +#endif
> 
> Are rather convoluted.  Could you just remove the wrappering for 2.5?

The answer to this and your other issues you raise about the driver are
the same.  I do not want to fork the driver.  I still have to maintain
support all the way back to 2.4.7 and branching the driver for every
different supported kernel would be a nightmare to maintain.  As it stands
now, other than the Makefile and kernel config files, there is just one
set of files that supports all of these kernels.  It makes it much
easier for everyone involved including the primary maintainer of the
driver.

Personally, I don't see how this:

AIC_LINUX_IRQRETURN_T
ahd_linux_isr(int irq, void *dev_id, struct pt_regs * regs)
{
        struct  ahd_softc *ahd;
        u_long  flags;
        int     ours;

...

        AIC_LINUX_IRQRETURN(ours);
}

Is any harder to parse than:

irqreturn_t
ahd_linux_isr(int irq, void *dev_id, struct pt_regs * regs)
{
        struct  ahd_softc *ahd;
        u_long  flags;
        int     ours;

...

        return IRQ_RETVAL(ours);
}

I've tried hard to make most of the API differences similarly transparent
within the driver to avoid messy ifdefs.  I haven't succeeded everywhere,
but this is the price you pay when APIs change so often.  All of the code
is also setup so that the backwards compatibility hooks have no impact on
the driver's performance under any support kernel (i.e. each kernel is
supported as best as it can be supported).

Is there some new policy against having drivers that support multiple
kernel versions?

--
Justin


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: Aic7xxx and Aic79xx Driver Updates
  2003-05-02 17:51   ` Justin T. Gibbs
@ 2003-05-02 18:07     ` James Bottomley
  2003-05-02 18:19       ` Linus Torvalds
  2003-05-03 23:01       ` Justin T. Gibbs
  2003-05-07  3:22     ` Lukasz Trabinski
  1 sibling, 2 replies; 12+ messages in thread
From: James Bottomley @ 2003-05-02 18:07 UTC (permalink / raw)
  To: Justin T. Gibbs
  Cc: SCSI Mailing List, Linux Kernel, Linus Torvalds, Alan Cox,
	Marcelo Tosatti

On Fri, 2003-05-02 at 12:51, Justin T. Gibbs wrote:
> > First off, could you take a look at
> > 
> > http://bugzilla.kernel.org/show_bug.cgi?id=608
> > 
> > I thought it was an sr problem, but it doesn't seem to show up on
> > anything other than adaptec controllers?  Thanks.
> 
> I've just updated the bug.


Thanks.

> > On Thu, 2003-05-01 at 17:28, Justin T. Gibbs wrote:
> >> ChangeSet
> >>   1.1118.33.5 03/04/24 15:12:48 gibbs@overdrive.btc.adaptec.com +7 -0
> >>   Aic7xxx and Aic79xx Driver Updates
> >>    o Adapt to new IRQ handler declaration/behavior for 2.5.X
> > 
> > The changes for this:
> > 
> > +#if LINUX_VERSION_CODE >= KERNEL_VERSION(2,5,0)
> > +#define        AIC_LINUX_IRQRETURN_T irqreturn_t
> > +#define        AIC_LINUX_IRQRETURN(ours) return (IRQ_RETVAL(ours))
> > +#else
> > +#define        AIC_LINUX_IRQRETURN_T void
> > +#define        AIC_LINUX_IRQRETURN(ours)  return
> > +#endif
> > 
> > Are rather convoluted.  Could you just remove the wrappering for 2.5?
> 
> The answer to this and your other issues you raise about the driver are
> the same.  I do not want to fork the driver.  I still have to maintain
> support all the way back to 2.4.7 and branching the driver for every
> different supported kernel would be a nightmare to maintain.  As it stands
> now, other than the Makefile and kernel config files, there is just one
> set of files that supports all of these kernels.  It makes it much
> easier for everyone involved including the primary maintainer of the
> driver.

Well, you do keep a 2.4 and a 2.5 tree, so you still have to apply your
patches twice.  You also seem to have two drivers: aic7xxx and aic79xx
which seem to duplicate about 80% of the code between them, so you have
to further patch each of these drivers for each of your fixes.

I'm not asking for any changes to the way you do 2.4, just for 2.5 where
we have no vendor versions to support and there should only be a single
tree.

> Personally, I don't see how this:
> 
> AIC_LINUX_IRQRETURN_T
> ahd_linux_isr(int irq, void *dev_id, struct pt_regs * regs)
> {
>         struct  ahd_softc *ahd;
>         u_long  flags;
>         int     ours;
> 
> ...
> 
>         AIC_LINUX_IRQRETURN(ours);
> }
> 
> Is any harder to parse than:
> 
> irqreturn_t
> ahd_linux_isr(int irq, void *dev_id, struct pt_regs * regs)
> {
>         struct  ahd_softc *ahd;
>         u_long  flags;
>         int     ours;
> 
> ...
> 
>         return IRQ_RETVAL(ours);
> }

The latter requires fewer levels of indirection to understand what's
going on.

> I've tried hard to make most of the API differences similarly transparent
> within the driver to avoid messy ifdefs.  I haven't succeeded everywhere,
> but this is the price you pay when APIs change so often.  All of the code
> is also setup so that the backwards compatibility hooks have no impact on
> the driver's performance under any support kernel (i.e. each kernel is
> supported as best as it can be supported).

Well, for 2.4, where there are multiple vendor versions, I'm OK with
this.  For 2.5 it's not necessary (yet).

> Is there some new policy against having drivers that support multiple
> kernel versions?

Not as such.  There is a preference for clean additions, though.  Adding
code to drivers that is never used and will never be used does tend to
make them more obscure to the reader.

James



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: Aic7xxx and Aic79xx Driver Updates
  2003-05-02 18:07     ` James Bottomley
@ 2003-05-02 18:19       ` Linus Torvalds
  2003-05-03 23:03         ` Justin T. Gibbs
  2003-05-03 23:01       ` Justin T. Gibbs
  1 sibling, 1 reply; 12+ messages in thread
From: Linus Torvalds @ 2003-05-02 18:19 UTC (permalink / raw)
  To: James Bottomley
  Cc: Justin T. Gibbs, SCSI Mailing List, Linux Kernel, Alan Cox,
	Marcelo Tosatti


On 2 May 2003, James Bottomley wrote:
> 
> I'm not asking for any changes to the way you do 2.4, just for 2.5 where
> we have no vendor versions to support and there should only be a single
> tree.

The way the backwards-compatibility is _meant_ to work is that a driver 
can just do this:

	#ifndef IRQ_RETVAL
	  typedef void irqreturn_t;
	  #define IRQ_NONE
	  #define IRQ_HANDLED
	  #define IRQ_RETVAL(x)
	#endif

and after that you can just use the 2.5.x semantics even with a 2.4.x 
kernel.

Which is nice and clean, and allows you to support old kernels _without_ 
having any translation layer.

		Linus


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: Aic7xxx and Aic79xx Driver Updates
  2003-05-02 18:07     ` James Bottomley
  2003-05-02 18:19       ` Linus Torvalds
@ 2003-05-03 23:01       ` Justin T. Gibbs
  1 sibling, 0 replies; 12+ messages in thread
From: Justin T. Gibbs @ 2003-05-03 23:01 UTC (permalink / raw)
  To: James Bottomley
  Cc: SCSI Mailing List, Linux Kernel, Linus Torvalds, Alan Cox,
	Marcelo Tosatti

>> The answer to this and your other issues you raise about the driver are
>> the same.  I do not want to fork the driver.  I still have to maintain
>> support all the way back to 2.4.7 and branching the driver for every
>> different supported kernel would be a nightmare to maintain.  As it stands
>> now, other than the Makefile and kernel config files, there is just one
>> set of files that supports all of these kernels.  It makes it much
>> easier for everyone involved including the primary maintainer of the
>> driver.
> 
> Well, you do keep a 2.4 and a 2.5 tree, so you still have to apply your
> patches twice.

Linux has finally started to use revision control, so I try to take
advantage of that by breaking up my changes and providing complete
changelogs.  Yes, it is a pain since I have to do three commits (Perforce,
BK linux-2.4, BK linux-2.5), but there is only one "version" of the driver
that I have to keep in my head, rather than two or twenty.  The 2.4.X code
also encourages those "upgrading" the drivers for 2.5.X features to do so in
a 2.4.X compatible fashion.  It's usually not that hard.

> You also seem to have two drivers: aic7xxx and aic79xx
> which seem to duplicate about 80% of the code between them, so you have
> to further patch each of these drivers for each of your fixes.

80% is a bit of an overstatement.  Yes, the OSMs are similar because
the author is the same.  Where code can be factored it is put into the
aiclib files - a process that is not fully complete.  The concentration
has been on keeping the drivers up to date with 2.5 and fixing bugs, but
some bit of code moves to common files in almost every version bump.

> I'm not asking for any changes to the way you do 2.4, just for 2.5 where
> we have no vendor versions to support and there should only be a single
> tree.

And as the maintainer, I know that maintaining a single driver is
far easier than splitting it up.  Other maintainers may feel differently,
but they are not maintaining these drivers.

>> I've tried hard to make most of the API differences similarly transparent
>> within the driver to avoid messy ifdefs.  I haven't succeeded everywhere,
>> but this is the price you pay when APIs change so often.  All of the code
>> is also setup so that the backwards compatibility hooks have no impact on
>> the driver's performance under any support kernel (i.e. each kernel is
>> supported as best as it can be supported).
> 
> Well, for 2.4, where there are multiple vendor versions, I'm OK with
> this.  For 2.5 it's not necessary (yet).

If at some point there becomes an official policy that all backward
compatibility code must be removed, I will do so.  I'm sure I'm not
the only driver maintainer that will complain if/when this happens.  Until
then, I hope that the level of "obscurity" this adds to the code doesn't
prevent you or others from providing substantive reviews of my drivers.

--
Justin


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: Aic7xxx and Aic79xx Driver Updates
  2003-05-02 18:19       ` Linus Torvalds
@ 2003-05-03 23:03         ` Justin T. Gibbs
  0 siblings, 0 replies; 12+ messages in thread
From: Justin T. Gibbs @ 2003-05-03 23:03 UTC (permalink / raw)
  To: Linus Torvalds, James Bottomley
  Cc: SCSI Mailing List, Linux Kernel, Alan Cox, Marcelo Tosatti

> On 2 May 2003, James Bottomley wrote:
>> 
>> I'm not asking for any changes to the way you do 2.4, just for 2.5 where
>> we have no vendor versions to support and there should only be a single
>> tree.
> 
> The way the backwards-compatibility is _meant_ to work is that a driver 
> can just do this:
> 
> 	#ifndef IRQ_RETVAL
> 	  typedef void irqreturn_t;
> 	  #define IRQ_NONE
> 	  #define IRQ_HANDLED
> 	  #define IRQ_RETVAL(x)
> 	#endif

I switched the drivers to using this yesterday.

	#ifndef IRQ_RETVAL
	  typedef void irqreturn_t;
	  #define IRQ_RETVAL(x)
	#endif

Updated BK send and tar files are here:

http://people.FreeBSD.org/~gibbs/linux/SRC/

--
Justin


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: Aic7xxx and Aic79xx Driver Updates
  2003-05-02 17:51   ` Justin T. Gibbs
  2003-05-02 18:07     ` James Bottomley
@ 2003-05-07  3:22     ` Lukasz Trabinski
  2003-05-07  8:31       ` Justin T. Gibbs
  1 sibling, 1 reply; 12+ messages in thread
From: Lukasz Trabinski @ 2003-05-07  3:22 UTC (permalink / raw)
  To: linux-kernel

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 693 bytes --]

In article <2274070000.1051897888@aslan.btc.adaptec.com> you wrote:
>> I thought it was an sr problem, but it doesn't seem to show up on
>> anything other than adaptec controllers?  Thanks.
> 
> I've just updated the bug.

Have You updated it on page too?
Drivers taken from http://people.freebsd.org/~gibbs/linux/SRC/

Adaptec AIC79xx driver version: 1.3.8
Adaptec AIC7902 Ultra320 SCSI adapter
aic7902: Ultra320 Wide Channel A, SCSI Id=7, PCI-X 67-100Mhz, 512 SCBs

During running slocate/updatedb:

bash-2.05b$ uptime
05:07:28  up 1 day,  8:09,  4 users,  load average: 67.07, 30.93, 12.51
                                                    ^^^^^^^^^^^^^^^^^^^

-- 
*[ £ukasz Tr±biñski ]*

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: Aic7xxx and Aic79xx Driver Updates
  2003-05-07  3:22     ` Lukasz Trabinski
@ 2003-05-07  8:31       ` Justin T. Gibbs
  0 siblings, 0 replies; 12+ messages in thread
From: Justin T. Gibbs @ 2003-05-07  8:31 UTC (permalink / raw)
  To: Lukasz Trabinski, linux-kernel

> In article <2274070000.1051897888@aslan.btc.adaptec.com> you wrote:
>>> I thought it was an sr problem, but it doesn't seem to show up on
>>> anything other than adaptec controllers?  Thanks.
>>
>> I've just updated the bug.
>
> Have You updated it on page too?

I can't parse that.  I added some text to the bug tracker.  This
bug doesn't appear to be a driver issue, so no source changes were made.

> During running slocate/updatedb:
>
> bash-2.05b$ uptime
> 05:07:28  up 1 day,  8:09,  4 users,  load average: 67.07, 30.93, 12.51
>                                                     ^^^^^^^^^^^^^^^^^^^

Which doesn't really tell me much.  What processes are chewing CPU time.
Is it system time?  What kernel version are you using and do you see this
without even using the latest driver?

--
Justin


^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2003-05-07 14:19 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2003-05-01 22:28 Aic7xxx and Aic79xx Driver Updates Justin T. Gibbs
2003-05-02  0:17 ` Willy Tarreau
2003-05-02  4:25   ` Justin T. Gibbs
2003-05-02  5:56     ` Willy Tarreau
2003-05-02 14:30 ` James Bottomley
2003-05-02 17:51   ` Justin T. Gibbs
2003-05-02 18:07     ` James Bottomley
2003-05-02 18:19       ` Linus Torvalds
2003-05-03 23:03         ` Justin T. Gibbs
2003-05-03 23:01       ` Justin T. Gibbs
2003-05-07  3:22     ` Lukasz Trabinski
2003-05-07  8:31       ` Justin T. Gibbs

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).