Re: [PATCH] ipr: don't doublefree pages from scatterlist

From: Hugh Dickins
Date: Mon Feb 06 2006 - 17:08:43 EST


On Mon, 6 Feb 2006, Andi Kleen wrote:
> On Monday 06 February 2006 17:45, Hugh Dickins wrote:
> >
> > But all this presupposes that someone is suddenly going to change the
> > x86_64 gart_map_sg (and subfunctions), or else force its iommu=nomerge:
> > that won't be me.
>
> Ok i changed it to conform to the gospel. I gave it some basic pounding LTP/dd IO with
> and without IOMMU force, but it's not that well tested. More testing welcome.

Great, thanks Andi. One small correction to the comment...

> Don't touch the non DMA members in the sg list in dma_map_sg in the IOMMU
>
> Some drivers (in particular ipr) ran into problems because they

No, the problem hasn't knowingly been sighted on ipr, it was the
st driver that Ryan's been seeing it with - ipr just came from
my looking around for like instances.

> reused the sg lists after passing them to pci_map_sg(). The merging
> procedure in the K8 GART IOMMU corrupted the state. This patch
> changes it to only touch the dma* entries during merging,
> but not the other fields. Approach suggested by Dave Miller.
>
> I did some basic tests with CONFIG_IOMMU_DEBUG (LTP, large dd)
> and without and it hold up, but needs more testing.

Below is, I think, the 2.6.15 equivalent of the patch Andi posted.
Ryan cannot effectively test Andi's patch on 2.6.16-rc because Mike
Christie's scsi_execute_async changes have serendipitously fixed
the st instance. Ryan, would you be able to test the patch below
on 2.6.15 without my st.c,st.h patch?

But I fear Ryan may be growing a little tired of this surfeit
of fixes: he's waited months for one, now he's been given one
workaround and three fixes.

Thanks,
Hugh

--- 2.6.15/arch/x86_64/kernel/pci-gart.c 2006-01-03 03:21:10.000000000 +0000
+++ linux/arch/x86_64/kernel/pci-gart.c 2006-02-06 21:33:17.000000000 +0000
@@ -477,7 +477,6 @@ static int __dma_map_cont(struct scatter

BUG_ON(i > start && s->offset);
if (i == start) {
- *sout = *s;
sout->dma_address = iommu_bus_base;
sout->dma_address += iommu_page*PAGE_SIZE + s->offset;
sout->dma_length = s->length;
@@ -492,7 +491,7 @@ static int __dma_map_cont(struct scatter
SET_LEAK(iommu_page);
addr += PAGE_SIZE;
iommu_page++;
- }
+ }
}
BUG_ON(iommu_page - iommu_start != pages);
return 0;
@@ -504,7 +503,6 @@ static inline int dma_map_cont(struct sc
{
if (!need) {
BUG_ON(stopat - start != 1);
- *sout = sg[start];
sout->dma_length = sg[start].length;
return 0;
}
@@ -622,7 +620,7 @@ void dma_unmap_sg(struct device *dev, st
}
for (i = 0; i < nents; i++) {
struct scatterlist *s = &sg[i];
- if (!s->dma_length || !s->length)
+ if (!s->dma_length)
break;
dma_unmap_single(dev, s->dma_address, s->dma_length, dir);
}
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/