On Fri, 10 Jul 2009 16:12:48 +0200 Ingo Molnar <mingo@xxxxxxx> wrote: > > functionality and reimplemented the surrounding infrastructure in > > terms of that (and incorporating our additional requirements). I > > prototyped this (it is currently unworking, in fact it seems to > > have developed rather a taste for filesystems :-() but the > > diffstat of my WIP patch is: > > > > arch/x86/kernel/pci-swiotlb.c | 6 > > arch/x86/xen/pci-swiotlb.c | 2 > > drivers/pci/xen-iommu.c | 385 ++++++++++++++++++++++++++++++++++++++++-- > > include/linux/swiotlb.h | 12 + > > lib/swiotlb.c | 10 - > > 5 files changed, 385 insertions(+), 30 deletions(-) > > > > where a fair number of the lines in xen-iommu.c are copies of > > functions from swiotlb.c with minor modifications. As I say it > > doesn't work yet but I think it's roughly indicative of what such > > an approach would look like. I don't like it much but am happy to > > run with it if it looks to be the most acceptable approach. [...] > > +400 lines of code to avoid much fewer lines of generic code impact > on the lib/swiotlb.c side sounds like a bad technical choice to me. The amount of code is not the point. The way to impact on the lib/swiotlb.c is totally wrong from the perspective of the kernel design; it uses architecture code in the very original (xen) way. > It makes the swiotlb code less useful and basically forks a random > implementation of it in drivers/pci/xen-iommu.c. I don't think so. We always have the reasonable amount of duplication rather than integration in a dirty way (at least about IOMMU code). > Fujita-san, can you think of a solution that avoids the whole-sale > copying of hundreds of lines of code? Ian didn't give a pointer to his code in a public place so I'm not sure his claim is valid. But if his code does the right thing and the duplication is less than 400 lines, it doesn't sound that bad to me. -- To unsubscribe from this list: send the line "unsubscribe linux-ia64" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
![]() |