2003-05-01 22:16:13

by Justin T. Gibbs

[permalink] [raw]
Subject: Aic7xxx and Aic79xx Driver Updates

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 [email protected] +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 [email protected] +2 -0
Update Aic79xx and Aic7xxx Documenation

ChangeSet
1.1118.33.6 03/05/01 11:00:31 [email protected] +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 [email protected] +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 [email protected] +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 [email protected] +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 [email protected] +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 [email protected] +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 [email protected] +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 [email protected] +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 [email protected] +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 [email protected] +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 [email protected] +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 [email protected] +2 -0
Aic7xxx and Aic79xx driver Update
o Fix style nits.

ChangeSet
1.971.94.6 03/04/24 11:49:01 [email protected] +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 [email protected] +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 [email protected] +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 [email protected] +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 [email protected] +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 [email protected] +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 [email protected] +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 [email protected] +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 [email protected] +2 -0
Change the callback argument for aic brace option parsing to u_long
to avoid casting problems with different architectures.


2003-05-02 00:06:00

by Willy Tarreau

[permalink] [raw]
Subject: Re: Aic7xxx and Aic79xx Driver Updates

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

2003-05-02 04:13:30

by Justin T. Gibbs

[permalink] [raw]
Subject: Re: Aic7xxx and Aic79xx Driver Updates

> 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
}


2003-05-02 05:44:13

by Willy Tarreau

[permalink] [raw]
Subject: Re: Aic7xxx and Aic79xx Driver Updates

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

2003-05-02 14:18:24

by James Bottomley

[permalink] [raw]
Subject: Re: Aic7xxx and Aic79xx Driver Updates

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 [email protected] +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 [email protected] +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 [email protected] +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


2003-05-02 17:39:25

by Justin T. Gibbs

[permalink] [raw]
Subject: Re: Aic7xxx and Aic79xx Driver Updates

> 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 [email protected] +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

2003-05-02 18:06:41

by Linus Torvalds

[permalink] [raw]
Subject: Re: Aic7xxx and Aic79xx Driver Updates


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

2003-05-02 17:55:51

by James Bottomley

[permalink] [raw]
Subject: Re: Aic7xxx and Aic79xx Driver Updates

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 [email protected] +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


2003-05-03 22:49:59

by Justin T. Gibbs

[permalink] [raw]
Subject: Re: Aic7xxx and Aic79xx Driver Updates

>> 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

2003-05-03 22:51:04

by Justin T. Gibbs

[permalink] [raw]
Subject: Re: Aic7xxx and Aic79xx Driver Updates

> 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

2003-05-07 03:09:55

by Lukasz Trabinski

[permalink] [raw]
Subject: Re: Aic7xxx and Aic79xx Driver Updates

In article <[email protected]> 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 ]*

2003-05-07 14:19:39

by Justin T. Gibbs

[permalink] [raw]
Subject: Re: Aic7xxx and Aic79xx Driver Updates

> In article <[email protected]> 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