On Sun, 5 Feb 2006, Brian King wrote: > > No. I can't find any code in any architecture that modifies either the length, > page, or offset fields in the *_map_sg routines. >From looking at the source, the architectures I found to be doing scatterlist coalescing in some cases were alpha, ia64, parisc (some code under drivers), powerpc, sparc64 and x86_64. I agree with you that it would be possible for them to do the coalescing by just adjusting dma_address and dma_length (though it's architecture- dependent whether there are such fields at all), not interfering with the input page and length; and maybe some of them do proceed that way. I didn't find the coalescing code in any of them very easy to follow. So please examine arch/x86_64/kernel/pci_gart.c gart_map_sg (and dma_map_cont which it calls): x86_64 was the architecture on which the problem was really found with drivers/scsi/st.c, and avoided by that boot option iommu=nomerge. Lines like "*sout = *s;" and "*sout = sg[start];" are structure- copying whole scallerlist entries from one position in the list to another, without explicit mention of the page and length fields. > I'm still failing to see an actual bug in the ipr driver. Unless there is a > good reason for pushing additional complexity on every caller of pci_map_sg > to do additional bookkeeping, such as an architecture that is actually > modifying > fields in the scatterlist other than the dma_* fields, then I think it makes > more sense to clarify the API to say that pci_map_sg will not modify these > fields. The API is commendably clear that they may be modified. Like you I'd prefer it to be otherwise, and instead guarantee that they won't be: that would prevent this kind of surprise. But it's not how it is; and looking at the various pieces of coalescing code, I quickly abandoned my first inclination, to adjust them to make it so. Hugh - : send the line "unsubscribe linux-scsi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
