[BUG?] Recent feral ISP interaction with alpha dma

[BUG?] Recent feral ISP interaction with alpha dma

[Bailey, Scott]

Here's a weird problem for which I maybe have an answer except I'm not sure
if it's right. Hopefully the "many eyes" safety net will stop me from doing
anything truly stupid. :-)

I have an Alphaserver 4100 where I have been testing a build of kernel
2.4.24 with the most recent snapshot of the feral isp driver. (A previous
release of the driver, patched onto 2.4.22, is running great but I want to
get onto the newer kernel.)

The system boots happily enough, but eventually I start seeing sequences of:

pci_map_sg failed: could not allocate dma page tables
isp2: unable to dma map request

and processes start wedging.

After poking around in the source, I am suspecting my problem in the feral
isp_pci.c:tdma_mk() where a little snippet goes:

while (resid > 0) {
    nseg++;
    resid -= sg_dma_len(sg);
    sg++;
}

The previous (working) version of this code is:

while (resid > 0) {
    nseg++;
    resid -= sg->length;
    sg++;
}

The problem is, that looking at arch/alpha/kernel/pci_iommu.c:sg_classify()
in the recent 2.4 kernels, I see that sg->dma_length only gets filled in for
scatterlist elements that are leaders. I suspect the non-leader elements
contain crud that confuse the resid count in unpredictable ways.

The question for everybody: is it better to fix this by reverting the
isp_pci.c stuff to refer to sg->length again instead of sg->dma_length, so
that I'm always referencing a valid quantity, or should I tweak pci_iommu.c
so it sets this value to 0 for non-leader elements (and ignore the fact that
code may still not be paying attention to sg->dma_address before making
decisions about the element)?

I couldn't figure out what other architectures were doing from sniffing
around the other directories.

What will break the least? :-)

Thanks,

	Scott Bailey
	scott <dot> bailey <at> eds <dot> com

Re: [BUG?] Recent feral ISP interaction with alpha dma

[Matthew Jacob]

Thanks for spotting this...

Hmm- I guess I never really asked anyone what sg_dma_len was supposed to
apply to.

The best comment about this seems to be in the mips header

/*
 * These macros should be used after a pci_map_sg call has been done
 * to get bus addresses of each of the SG entries and their lengths.
 * You should only work with the number of sg entries pci_map_sg
 * returns, or alternatively stop on the first sg_dma_len(sg) which
 * is 0.
 */

So- I believe I'm using it correctly (well, not quite as I'm not
checking for a length of zero) - I call pci_map_sg, and for each
platform I should be using sg_dma_len- after all, there may be
architectures which don't have sg->length.

The scatterlist code in sg_classify isn't quite what you need to look
at- this sets up some kind of private scheme in alpha which then gets
decoded into an output list in sg_fill where it looks like more
dma_length entries get filled than the 'leader'.

I just gave away my Alphaserver 4100, but I know that the 2.4.18 kernels
used to work with my stuff on it- I wonder what the real issue is?

On Wed, 3 Mar 2004, Bailey, Scott wrote:

> Here's a weird problem for which I maybe have an answer except I'm not sure
> if it's right. Hopefully the "many eyes" safety net will stop me from doing
> anything truly stupid. :-)
>
> I have an Alphaserver 4100 where I have been testing a build of kernel
> 2.4.24 with the most recent snapshot of the feral isp driver. (A previous
> release of the driver, patched onto 2.4.22, is running great but I want to
> get onto the newer kernel.)
>
> The system boots happily enough, but eventually I start seeing sequences of:
>
> pci_map_sg failed: could not allocate dma page tables
> isp2: unable to dma map request
>
> and processes start wedging.
>
> After poking around in the source, I am suspecting my problem in the feral
> isp_pci.c:tdma_mk() where a little snippet goes:
>
> while (resid > 0) {
>     nseg++;
>     resid -= sg_dma_len(sg);
>     sg++;
> }
>
> The previous (working) version of this code is:
>
> while (resid > 0) {
>     nseg++;
>     resid -= sg->length;
>     sg++;
> }
>
> The problem is, that looking at arch/alpha/kernel/pci_iommu.c:sg_classify()
> in the recent 2.4 kernels, I see that sg->dma_length only gets filled in for
> scatterlist elements that are leaders. I suspect the non-leader elements
> contain crud that confuse the resid count in unpredictable ways.
>
> The question for everybody: is it better to fix this by reverting the
> isp_pci.c stuff to refer to sg->length again instead of sg->dma_length, so
> that I'm always referencing a valid quantity, or should I tweak pci_iommu.c
> so it sets this value to 0 for non-leader elements (and ignore the fact that
> code may still not be paying attention to sg->dma_address before making
> decisions about the element)?
>
> I couldn't figure out what other architectures were doing from sniffing
> around the other directories.
>
> What will break the least? :-)
>
> Thanks,
>
> 	Scott Bailey
> 	scott <dot> bailey <at> eds <dot> com
>

RE: [BUG?] Recent feral ISP interaction with alpha dma

[Bailey, Scott]

>The scatterlist code in sg_classify isn't quite what you need to look
>at- this sets up some kind of private scheme in alpha which then gets
>decoded into an output list in sg_fill where it looks like more
>dma_length entries get filled than the 'leader'.

Okay, I see that now. I was so busy looking for sg-> stuff that the
rewriting using out-> went right past me. My head still hurts from trying to
understand the transformations. :-)

Anyway, my kernel built from the Debian kernel-source-2.4.22-5 package with
I think about the 10/31/2003 release of the feral ISP driver works like a
dream on my Alphaserver 4100, so everything is definitely okay at that
point. There appear to have been no intervening changes to pci_iommu.c, so
that rules any really obvious typos... :-) Thus I wonder if the isp driver
may be feeding subtly different input to these routines.

Looking at the current state of affairs with the Debian
kernel-source-2.4.24-3 package and using the feral driver sources slurped
from your bitkeeper repository, I get the dma errors:

	pci_map_sg failed: could not allocate dma page tables
	isp2: unable to dma map request

There are three routines in pci_isp.c that call pci_map_sg:

- tdma_mk(). I mentioned this in my first e-mail, but on reflection
(actually, staring at paper printout) I see it actually is encapsulated in a
"#ifdef LINUX_ISP_TARGET_MODE" block, and I don't have that defined, so it
couldn't be this routine.

- tdma_mkfc(). Except that I believe this is still within the same #ifdef
block, and appears to be used for fibre channel, which I don't have, so this
also is eliminated as a suspect.

- isp_pci_dmasetup(). By process of elimination, I believe the call to
pci_map_sg() must have come from here. The "unable to dma map request" could
only have occurred when:

1. pci_isp.c:isp_pci_dmasetup() is called with Cmnd->use_sg != 0
2. the call to arch/alpha/kernel/pci_iommu.c:pci_map_sg() returns a value of
0

So my earlier blather about page counting appears to be totally irrelevant.
:-)

But I don't see any changes between old (happy) and new (dma-challenged)
isp_pci_dmasetup() prior to the point of the error, and you do minimal
transformation of inputs to get arguments for pci_map_sg(), so I get the
feeling maybe you are getting screwed by bad data coming in.

The only place arch/alpha/kernel/pci_iommu.c:pci_map_sg() displays the
"could not allocate dma page tables" error message is after a call to
sg_fill() fails. The circumstances would appear to require that the
scatterlist contain more than 1 entry, and I see sg_fill() will only be
called for scatterlist leaders.

Maybe my disk subsystem is the only thing that is generating multisegment
DMA activity, and this is just a symptom of more general brokenness? I dread
it, but it looks like I may need to add some instrumentation to (and/or
enable some of the debugging stuff in) pci_iommu.c.

Any other gratuitous advice is welcomed,

	Scott
	scott <dot> bailey <at> eds <dot> com

RE: [BUG?] Recent feral ISP interaction with alpha dma

[Bailey, Scott]

Matthew, just a quick update for your amusement or peace of mind or
whatever... :-)

I have continued slogging along with this, mostly upgrading to newer kernel
releases as they appeared and trying to ignore the dma errors. I now am up
to kernel 2.4.27, compiled with gcc 3.3.4 (actually Debian's gcc
3.3.4-6sarge1.0.1 package) and amazingly the errors seem to have disappeared
for me. They were always a bit sporadic, so this isn't a totally sure thing,
but I've been running for a couple days now and done all the things that
tending to spazz it out before, and everything has been quite nice.

Maybe something between 2.4.26 and 2.4.27 fixed this, but I have given up
trying to figure out what.

Anyway, I wanted to let you know that your driver is still working fine for
me and I'm really glad to have it, since our Alphaservers are just infested
with QLogic-based KZPBA controllers :-) and I really need to be able to use
disks and tape drives.

Thanks again,

	Scott

R. Scott Bailey
EDS - Software Services Linux/Tru64 UNIX Capability
MS 2O
1075 W. Entrance Dr.
Auburn Hills, MI  48326
 
( Phone:+1-248-276-5770 (8-351)
+ mailto:scott.bailey@eds.com


-----Original Message-----
From: Matthew Jacob [mailto:mjacob@feral.com] 
Sent: Wednesday, March 03, 2004 8:38 PM
To: Bailey, Scott
Cc: 'linux-kernel@vger.kernel.org'
Subject: Re: [BUG?] Recent feral ISP interaction with alpha dma



Thanks for spotting this...

Hmm- I guess I never really asked anyone what sg_dma_len was supposed to
apply to.

The best comment about this seems to be in the mips header

/*
 * These macros should be used after a pci_map_sg call has been done
 * to get bus addresses of each of the SG entries and their lengths.
 * You should only work with the number of sg entries pci_map_sg
 * returns, or alternatively stop on the first sg_dma_len(sg) which
 * is 0.
 */

So- I believe I'm using it correctly (well, not quite as I'm not
checking for a length of zero) - I call pci_map_sg, and for each
platform I should be using sg_dma_len- after all, there may be
architectures which don't have sg->length.

The scatterlist code in sg_classify isn't quite what you need to look
at- this sets up some kind of private scheme in alpha which then gets
decoded into an output list in sg_fill where it looks like more
dma_length entries get filled than the 'leader'.

I just gave away my Alphaserver 4100, but I know that the 2.4.18 kernels
used to work with my stuff on it- I wonder what the real issue is?

On Wed, 3 Mar 2004, Bailey, Scott wrote:

> Here's a weird problem for which I maybe have an answer except I'm not
sure
> if it's right. Hopefully the "many eyes" safety net will stop me from
doing
> anything truly stupid. :-)
>
> I have an Alphaserver 4100 where I have been testing a build of kernel
> 2.4.24 with the most recent snapshot of the feral isp driver. (A previous
> release of the driver, patched onto 2.4.22, is running great but I want to
> get onto the newer kernel.)
>
> The system boots happily enough, but eventually I start seeing sequences
of:
>
> pci_map_sg failed: could not allocate dma page tables
> isp2: unable to dma map request
>
> and processes start wedging.
>
> After poking around in the source, I am suspecting my problem in the feral
> isp_pci.c:tdma_mk() where a little snippet goes:
>
> while (resid > 0) {
>     nseg++;
>     resid -= sg_dma_len(sg);
>     sg++;
> }
>
> The previous (working) version of this code is:
>
> while (resid > 0) {
>     nseg++;
>     resid -= sg->length;
>     sg++;
> }
>
> The problem is, that looking at
arch/alpha/kernel/pci_iommu.c:sg_classify()
> in the recent 2.4 kernels, I see that sg->dma_length only gets filled in
for
> scatterlist elements that are leaders. I suspect the non-leader elements
> contain crud that confuse the resid count in unpredictable ways.
>
> The question for everybody: is it better to fix this by reverting the
> isp_pci.c stuff to refer to sg->length again instead of sg->dma_length, so
> that I'm always referencing a valid quantity, or should I tweak
pci_iommu.c
> so it sets this value to 0 for non-leader elements (and ignore the fact
that
> code may still not be paying attention to sg->dma_address before making
> decisions about the element)?
>
> I couldn't figure out what other architectures were doing from sniffing
> around the other directories.
>
> What will break the least? :-)
>
> Thanks,
>
> 	Scott Bailey
> 	scott <dot> bailey <at> eds <dot> com
>