Messages in this thread | | | Date | Thu, 25 Oct 2007 10:53:06 +0200 | From | Benny Halevy <> | Subject | Re: [BUG] 2.6.23-git18 Kernel oops in sg helpers |
| |
On Oct. 24, 2007, 10:50 +0200, Benny Halevy <bhalevy@panasas.com> wrote: > On Oct. 24, 2007, 10:32 +0200, Jens Axboe <jens.axboe@oracle.com> wrote: >> On Wed, Oct 24 2007, FUJITA Tomonori wrote: >>> On Tue, 23 Oct 2007 20:49:40 +0530 >>> Kamalesh Babulal <kamalesh@linux.vnet.ibm.com> wrote: >>> >>>> Hi, >>>> >>>> Kernel oops is triggered while running fsx-linux test, followed by cpu softlock >>>> over the AMD box >>>> >>>> Unable to handle kernel NULL pointer dereference at 0000000000000018 RIP: >>>> [<ffffffff8021f2f6>] gart_map_sg+0x26c/0x406 >>>> PGD 10185b067 PUD 10075b067 PMD 0 >>> Does this work? >>> >>> >>> diff --git a/arch/x86/kernel/pci-gart_64.c b/arch/x86/kernel/pci-gart_64.c >>> index c56e9ee..ae7e016 100644 >>> --- a/arch/x86/kernel/pci-gart_64.c >>> +++ b/arch/x86/kernel/pci-gart_64.c >>> @@ -338,7 +338,6 @@ static int __dma_map_cont(struct scatterlist *start, int nelems, >>> >>> BUG_ON(s != start && s->offset); >>> if (s == start) { >>> - *sout = *s; >>> sout->dma_address = iommu_bus_base; >>> sout->dma_address += iommu_page*PAGE_SIZE + s->offset; >>> sout->dma_length = s->length; >>> @@ -365,7 +364,7 @@ static inline int dma_map_cont(struct scatterlist *start, int nelems, >>> { >>> if (!need) { >>> BUG_ON(nelems != 1); >>> - *sout = *start; >>> + sout->dma_address = start->dma_address; > > I don't see this could fix anything since "s" above and "start" here are still > dereferenced. Also, this makes sout->dma_address inconsistent with sout->page_link > and with the end marker.
OK, it took me a day to figure out why the fix is working :) The end of list marker was copied into sout and later, in line 432 sg_next(sgmap) returned NULL since sgmap became the last entry in the list (which is strangely correct in the dma mapped vector).
431: if (out < nents) { 432: sgmap = sg_next(sgmap); 433: sgmap->dma_length = 0; 434: }
Alas, the dma mapping convention apparently requires dma_length == 0 as a terminator if the "compressed" list for dma mapping is shorter than the sg list.
Although this change does not keep each sg->dma_address in sync with each sg->page_link, previously there was nothing to keep sg->length in sync with sg->dma_length so I actually think that keeping the dma mapping and the page mappings orthogonal and independent may be even better since the original sg list can still be reused safely even after dma mapping.
> > Benny > >>> sout->dma_length = start->length; >>> return 0; >>> } >>> -- >>> 1.5.2.4 >> Care to write up a proper changelog? >> > > - > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ >
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |