[vortex] Re: 3com 3c905c-txm

[Andrew Morton]

Donald Becker wrote:
>

Philosophical statement: you have been doing Linux n/w drivers for many years.  I have been doing them for three months.  Hence my approach has been one of caution.  I try
to keep the changes small, things which I can clearly justify.  Careful trawling of the mailing lists, careful followup with people who report problems.  Stabilisation,
incremental improvements.  I view performance improvements as being far, far lower priority than stability and robustness in the presence of exceptional influences.

> 
> As I mentioned in the previous reply to the linux-vortex mailing list, the
> 3c59x.c code in 2.3.99 is completely bogus.

You said it was wrong, but you left me guessing as to precisely why.

My guess is that you were referring to the Cyclone no-DnStall, no inw/outw code path?

If so, you introduced that in 0.99M.   Linus's 2.2 and 2.3 drivers are derived from your 0.99H and 0.99L drivers.  These drivers used DnStall for _all_ non-Vortex NICs.  So
these are not "changes".  They are failures to track.

Now, I really need a hint here.  I am aware of and have looked at your optimisations in post-0.99M versions.  The problem is that I cannot correlate these with the 3Com
docs.  3c90xc.pdf and 3c90xb.pdf both say "Use DnStall unless you're polling DPDs".  We are not polling DPDs.  I suspect that there is some piece of information which both
you and Bodgan have which I have missed.

I have not spent a lot of time trying to understand your later optimisations.  If it is a matter of correctness rather than performance then yes, your later changes should
be fed back into 2.2 and 2.3.  But unless I have the info to justify it, can generate appropriate test cases and can convince myself that it is correct wrt cache coherency
and write ordering on SPARC, PPC, Alpha and ARM then what can I do?  

Ditto all the above for your memory-space vs I/O-space optimisation.

> The DownStall is only useful on the Boomerang, not the later chips.
> Using DownStall with later chips will result in exactly what you are seeing
> -- hundred or thousands of wasted PCI transactions for each packet.

I have not done the histogram, but the vast majority of times it is a couple of cycles.  The problem is that the duration appears to be unbounded on collisiony LANs.  I
would really appreciate your help in understanding this issue.

> The later chips do not need any spinlocking with a properly designed
> transmit routine.  Especially not the painfully slow 'irqsave' versions of
> the spinlocks.

How so?  spin_lock_irqsave() is six instructions if the lock is free.

> I shouldn't be pointing out specific changes that were bad, because people
> will assume that the rest of the changes were OK.  Generally, if one set of
> changes are badly designed, it means that all of the changes should be
> reconsidered.

As I said above, it's mainly _lack_ of changes at issue here (I think).

Yes, I have made changes.  We believe we have identified several SMP and IRQ and CPU/hardware races as well as a couple of media selection issues.  I have been careful to
discuss these on lists which you are known to frequent.  The changes are carefully documented.  The few comments you have made have, of course, been invaluable.   More such
comments would be equally valuable.

After all, we are all on the same side here.

-- 
-akpm-