Re: [00/15] swiotlb cleanup
* FUJITA Tomonori fujita.tomon...@lab.ntt.co.jp wrote: On Mon, 13 Jul 2009 13:20:22 +0900 FUJITA Tomonori fujita.tomon...@lab.ntt.co.jp wrote: On Fri, 10 Jul 2009 16:12:48 +0200 Ingo Molnar mi...@elte.hu 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. btw, '+400 lines of code to avoid much fewer lines of generic code impact on the lib/swiotlb.c' doesn't sound true to me. Here is a patch in the way that Xen people want to do: http://patchwork.kernel.org/patch/26343/ --- arch/x86/Kconfig |4 + arch/x86/include/asm/io.h|2 + arch/x86/include/asm/pci_x86.h |1 + arch/x86/include/asm/xen/iommu.h | 12 ++ arch/x86/kernel/pci-dma.c|3 + arch/x86/pci/Makefile|1 + arch/x86/pci/init.c |6 + arch/x86/pci/xen.c | 51 +++ drivers/pci/Makefile |2 + drivers/pci/xen-iommu.c | 271 ++ Even with the way that Xen people want to do, drivers/pci/xen-iommu.c is about 300 lines. And my patchset removes the nice amount of lines for dom0 support. I don't see much difference wrt lines. ok, that kind of impact looks reasonable. If we are wrong and the Xen model becomes duplicated anywhere else it can still be generalized into core swiotlb code. Ingo ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [00/15] swiotlb cleanup
On Jul 13, 2009, at 10:13 PM, Becky Bruce wrote: On Jul 10, 2009, at 12:12 AM, Ingo Molnar wrote: * FUJITA Tomonori fujita.tomon...@lab.ntt.co.jp wrote: - removes unused (and unnecessary) hooks in swiotlb. - adds dma_capable() and converts swiotlb to use it. It can be used to know if a memory area is dma capable or not. I added is_buffer_dma_capable() for the same purpose long ago but it turned out that the function doesn't work on POWERPC. This can be applied cleanly to linux-next, -mm, and mainline. This patchset touches multiple architectures (ia64, powerpc, x86) so I guess that -mm is appropriate for this patchset (I don't care much what tree would merge this though). This is tested on x86 but only compile tested on POWERPC and IA64. Thanks, = arch/ia64/include/asm/dma-mapping.h| 18 ++ arch/powerpc/include/asm/dma-mapping.h | 23 +++ arch/powerpc/kernel/dma-swiotlb.c | 48 +--- arch/x86/include/asm/dma-mapping.h | 18 ++ arch/x86/kernel/pci-dma.c |2 +- arch/x86/kernel/pci-gart_64.c |5 +- arch/x86/kernel/pci-nommu.c|2 +- arch/x86/kernel/pci-swiotlb.c | 25 include/linux/dma-mapping.h|5 -- include/linux/swiotlb.h| 11 lib/swiotlb.c | 102 +--- 11 files changed, 92 insertions(+), 167 deletions(-) Hm, the functions and facilities you remove here were added as part of preparatory patches for Xen guest support. You were aware of them, you were involved in discussions about those aspects with Ian and Jeremy but still you chose not to Cc: either of them and you failed to address that aspect in the changelogs. I'd like the Xen code to become cleaner more than anyone else here i guess, but patch submission methods like this are not really helpful. A far better method is to be open about such disagreements, to declare them, to Cc: everyone who disagrees, and to line out the arguments in the changelogs as well - instead of just curtly declaring those APIs 'unused' and failing to Cc: involved parties. Alas, on the technical level the cleanups themselves look mostly fine to me. Ian, Jeremy, the changes will alter Xen's use of swiotlb, but can the Xen side still live with these new methods - in particular is dma_capable() sufficient as a mechanism and can the Xen side filter out DMA allocations to make them physically continuous? Ben, Tony, Becky, any objections wrt. the PowerPC / IA64 impact? If everyone agrees i can apply them to the IOMMU tree, test it and push it out to -next, etc. Ingo, With the exception of the patch I commented on, I think these look OK from the powerpc point of view. I've successfully booted one of my test platforms with the entire series applied and will run some more extensive (i.e. not Whee! A prompt!) tests tomorrow. Well, I am still testing. I've observed one unexpected LTP testcase failure with these patches applied, but so far have been unable to reproduce it. So these patches are probably OK, but I will look into this some more next week. -Becky -Becky ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [00/15] swiotlb cleanup
On Mon, 13 Jul 2009 13:20:22 +0900 FUJITA Tomonori fujita.tomon...@lab.ntt.co.jp wrote: On Fri, 10 Jul 2009 16:12:48 +0200 Ingo Molnar mi...@elte.hu 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. btw, '+400 lines of code to avoid much fewer lines of generic code impact on the lib/swiotlb.c' doesn't sound true to me. Here is a patch in the way that Xen people want to do: http://patchwork.kernel.org/patch/26343/ --- arch/x86/Kconfig |4 + arch/x86/include/asm/io.h|2 + arch/x86/include/asm/pci_x86.h |1 + arch/x86/include/asm/xen/iommu.h | 12 ++ arch/x86/kernel/pci-dma.c|3 + arch/x86/pci/Makefile|1 + arch/x86/pci/init.c |6 + arch/x86/pci/xen.c | 51 +++ drivers/pci/Makefile |2 + drivers/pci/xen-iommu.c | 271 ++ Even with the way that Xen people want to do, drivers/pci/xen-iommu.c is about 300 lines. And my patchset removes the nice amount of lines for dom0 support. I don't see much difference wrt lines. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [00/15] swiotlb cleanup
On Mon, 13 Jul 2009 10:40:29 +0100 Ian Campbell ian.campb...@eu.citrix.com wrote: On Mon, 2009-07-13 at 05:20 +0100, FUJITA Tomonori wrote: On Fri, 10 Jul 2009 15:02:00 +0100 Ian Campbell ian.campb...@citrix.com wrote: On Fri, 2009-07-10 at 14:35 +0900, FUJITA Tomonori wrote: I don't think that we need to take account of dom0 support; we don't have a clear idea about an acceptable dom0 design (it needs to use swiotlb code? I don't know yet), we don't even know we will have dom0 support in mainline. That's why I didn't CC this patchset to Xen camp. The core domain 0 patches which were the subject of the discussions a few week back are completely orthogonal to the swiotlb side of things ? If we don't merge dom0 patch, we don't need dom0 changes to swiotlb. We don't know we would have dom0 support in mainline. Or I overlooked something? [...] As far as I know, you have not posted anything about changes to swiotlb for domU. I can't discuss it. If you want, please send patches. There are no separate domU swiotlb patches -- the exact the same patches as we have already been discussing are useful and necessary for both domU and dom0. Hmm, you guys introduced the swiotlb hooks by saying that it's for only dom0. = http://lkml.org/lkml/2008/12/16/351 Hi Ingo, Here's some patches to clean up swiotlb to prepare for some Xen dom0 patches. These have been posted before, but undergone a round of cleanups to address various comments. = I don't see any comments like 'this is useful to dom0 too'. I'm still not sure what exactly part is useful to domU. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [00/15] swiotlb cleanup
On Jul 10, 2009, at 12:12 AM, Ingo Molnar wrote: * FUJITA Tomonori fujita.tomon...@lab.ntt.co.jp wrote: - removes unused (and unnecessary) hooks in swiotlb. - adds dma_capable() and converts swiotlb to use it. It can be used to know if a memory area is dma capable or not. I added is_buffer_dma_capable() for the same purpose long ago but it turned out that the function doesn't work on POWERPC. This can be applied cleanly to linux-next, -mm, and mainline. This patchset touches multiple architectures (ia64, powerpc, x86) so I guess that -mm is appropriate for this patchset (I don't care much what tree would merge this though). This is tested on x86 but only compile tested on POWERPC and IA64. Thanks, = arch/ia64/include/asm/dma-mapping.h| 18 ++ arch/powerpc/include/asm/dma-mapping.h | 23 +++ arch/powerpc/kernel/dma-swiotlb.c | 48 +--- arch/x86/include/asm/dma-mapping.h | 18 ++ arch/x86/kernel/pci-dma.c |2 +- arch/x86/kernel/pci-gart_64.c |5 +- arch/x86/kernel/pci-nommu.c|2 +- arch/x86/kernel/pci-swiotlb.c | 25 include/linux/dma-mapping.h|5 -- include/linux/swiotlb.h| 11 lib/swiotlb.c | 102 +--- 11 files changed, 92 insertions(+), 167 deletions(-) Hm, the functions and facilities you remove here were added as part of preparatory patches for Xen guest support. You were aware of them, you were involved in discussions about those aspects with Ian and Jeremy but still you chose not to Cc: either of them and you failed to address that aspect in the changelogs. I'd like the Xen code to become cleaner more than anyone else here i guess, but patch submission methods like this are not really helpful. A far better method is to be open about such disagreements, to declare them, to Cc: everyone who disagrees, and to line out the arguments in the changelogs as well - instead of just curtly declaring those APIs 'unused' and failing to Cc: involved parties. Alas, on the technical level the cleanups themselves look mostly fine to me. Ian, Jeremy, the changes will alter Xen's use of swiotlb, but can the Xen side still live with these new methods - in particular is dma_capable() sufficient as a mechanism and can the Xen side filter out DMA allocations to make them physically continuous? Ben, Tony, Becky, any objections wrt. the PowerPC / IA64 impact? If everyone agrees i can apply them to the IOMMU tree, test it and push it out to -next, etc. Ingo, With the exception of the patch I commented on, I think these look OK from the powerpc point of view. I've successfully booted one of my test platforms with the entire series applied and will run some more extensive (i.e. not Whee! A prompt!) tests tomorrow. -Becky ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [00/15] swiotlb cleanup
On Fri, 10 Jul 2009 16:12:48 +0200 Ingo Molnar mi...@elte.hu 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. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [00/15] swiotlb cleanup
On Fri, 10 Jul 2009 15:02:00 +0100 Ian Campbell ian.campb...@citrix.com wrote: On Fri, 2009-07-10 at 14:35 +0900, FUJITA Tomonori wrote: I don't think that we need to take account of dom0 support; we don't have a clear idea about an acceptable dom0 design (it needs to use swiotlb code? I don't know yet), we don't even know we will have dom0 support in mainline. That's why I didn't CC this patchset to Xen camp. The core domain 0 patches which were the subject of the discussions a few week back are completely orthogonal to the swiotlb side of things ? If we don't merge dom0 patch, we don't need dom0 changes to swiotlb. We don't know we would have dom0 support in mainline. Or I overlooked something? and whatever form they eventually take I do not think it will have any impact on the shape of the solution which we arrive at for swiotlb. I don't think that assuming that domain 0 can never be done in a way which everyone finds acceptable and therefore discounting all consideration of it is a useful way to make progress with these issues. The DMA use case is much more tightly tied to the paravirtualized MMU (which is already in the kernel for domU purposes) than it is to the domain 0 patches anyway. Although domain 0 is probably the main use case, at least today, swiotlb support is also used in a Xen domU as part of the support for direct assignment of PCI devices to paravirtualised guests (pci passthrough). The pci frontend driver depends on some bits of the domain 0 physical interrupt patches as well as swiotlb which is why I/we haven't tried to upstream that particular series yet. As far as I know, you have not posted anything about changes to swiotlb for domU. I can't discuss it. If you want, please send patches. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [00/15] swiotlb cleanup
On Fri, 2009-07-10 at 14:35 +0900, FUJITA Tomonori wrote: I don't think that we need to take account of dom0 support; we don't have a clear idea about an acceptable dom0 design (it needs to use swiotlb code? I don't know yet), we don't even know we will have dom0 support in mainline. That's why I didn't CC this patchset to Xen camp. The core domain 0 patches which were the subject of the discussions a few week back are completely orthogonal to the swiotlb side of things and whatever form they eventually take I do not think it will have any impact on the shape of the solution which we arrive at for swiotlb. I don't think that assuming that domain 0 can never be done in a way which everyone finds acceptable and therefore discounting all consideration of it is a useful way to make progress with these issues. The DMA use case is much more tightly tied to the paravirtualized MMU (which is already in the kernel for domU purposes) than it is to the domain 0 patches anyway. Although domain 0 is probably the main use case, at least today, swiotlb support is also used in a Xen domU as part of the support for direct assignment of PCI devices to paravirtualised guests (pci passthrough). The pci frontend driver depends on some bits of the domain 0 physical interrupt patches as well as swiotlb which is why I/we haven't tried to upstream that particular series yet. Ian. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [00/15] swiotlb cleanup
On Fri, 2009-07-10 at 06:12 +0100, Ingo Molnar wrote: Hm, the functions and facilities you remove here were added as part of preparatory patches for Xen guest support. You were aware of them, you were involved in discussions about those aspects with Ian and Jeremy but still you chose not to Cc: either of them and you failed to address that aspect in the changelogs. Thanks for adding me Ingo. Alas, on the technical level the cleanups themselves look mostly fine to me. Ian, Jeremy, the changes will alter Xen's use of swiotlb, but can the Xen side still live with these new methods - in particular is dma_capable() sufficient as a mechanism and can the Xen side filter out DMA allocations to make them physically continuous? I've not examined the series in detail it looks OK but I don't think it is quite sufficient. The Xen determination of whether a buffer is dma_capable or not is based on the physical address while dma_capable takes only the dma address. I'm not sure we can invert our conditions to work back from dma address to physical since given a start dma address and a length we would need to check that dma_to_phys(dma+PAGE_SIZE) == dma_to_phys(dma)+PAGE_SIZE etc. However dma+PAGE_SIZE might belong to a different domain so translating it to a physical address in isolation tells us nothing especially useful since it would give us the physical address in that other guest which is useless to us. If we could pass both physical and dma address to dma_capable I think that would probably be sufficient for our purposes. As well as that Xen needs some way to influence the allocation of the actual bounce buffer itself since we need to arrange for it to be machine address contiguous as well as physical address contiguous. This series explicitly removes those hooks without replacement. My most recent proposal was to have a new swiotlb_init variant which was given a preallocated buffer which this series doesn't necessarily preclude. The phys_to_dma and dma_to_phys translation points are the last piece Xen needs and seem to be preserved in this series. However Fujita's objection to all of the previous swiotlb-for-xen proposals was around the addition of the Xen hooks in whichever location. Originally these hooks were via __weak functions and later proposals implemented them via function pointer hooks in the x86 implementations of the arch-abstract interfaces (phys-dma and dma_capable etc). I don't think this series addresses those objections (fair enough -- it wasn't intended to) or leads to any new approach to solving the issue, although I also don't think it makes the issue any harder to address. I don't think it will be possible to make progress on Xen usage of swiotlb until a solution can be found to this conflict of opinion. Fujita suggested that we export the core sync_single() 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. To be honest at the moment I've deliberately been taking some time away from this stuff to try and gain a bit of perspective so I haven't looked at it in a while. Ian. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [00/15] swiotlb cleanup
* Ian Campbell ian.campb...@eu.citrix.com wrote: I've not examined the series in detail it looks OK but I don't think it is quite sufficient. The Xen determination of whether a buffer is dma_capable or not is based on the physical address while dma_capable takes only the dma address. I'm not sure we can invert our conditions to work back from dma address to physical since given a start dma address and a length we would need to check that dma_to_phys(dma+PAGE_SIZE) == dma_to_phys(dma)+PAGE_SIZE etc. However dma+PAGE_SIZE might belong to a different domain so translating it to a physical address in isolation tells us nothing especially useful since it would give us the physical address in that other guest which is useless to us. If we could pass both physical and dma address to dma_capable I think that would probably be sufficient for our purposes. As well as that Xen needs some way to influence the allocation of the actual bounce buffer itself since we need to arrange for it to be machine address contiguous as well as physical address contiguous. This series explicitly removes those hooks without replacement. My most recent proposal was to have a new swiotlb_init variant which was given a preallocated buffer which this series doesn't necessarily preclude. The phys_to_dma and dma_to_phys translation points are the last piece Xen needs and seem to be preserved in this series. However Fujita's objection to all of the previous swiotlb-for-xen proposals was around the addition of the Xen hooks in whichever location. Originally these hooks were via __weak functions and later proposals implemented them via function pointer hooks in the x86 implementations of the arch-abstract interfaces (phys-dma and dma_capable etc). I don't think this series addresses those objections (fair enough -- it wasn't intended to) or leads to any new approach to solving the issue, although I also don't think it makes the issue any harder to address. I don't think it will be possible to make progress on Xen usage of swiotlb until a solution can be found to this conflict of opinion. Fujita suggested that we export the core sync_single() 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. It makes the swiotlb code less useful and basically forks a random implementation of it in drivers/pci/xen-iommu.c. Fujita-san, can you think of a solution that avoids the whole-sale copying of hundreds of lines of code? Ingo ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [00/15] swiotlb cleanup
* FUJITA Tomonori fujita.tomon...@lab.ntt.co.jp wrote: - removes unused (and unnecessary) hooks in swiotlb. - adds dma_capable() and converts swiotlb to use it. It can be used to know if a memory area is dma capable or not. I added is_buffer_dma_capable() for the same purpose long ago but it turned out that the function doesn't work on POWERPC. This can be applied cleanly to linux-next, -mm, and mainline. This patchset touches multiple architectures (ia64, powerpc, x86) so I guess that -mm is appropriate for this patchset (I don't care much what tree would merge this though). This is tested on x86 but only compile tested on POWERPC and IA64. Thanks, = arch/ia64/include/asm/dma-mapping.h| 18 ++ arch/powerpc/include/asm/dma-mapping.h | 23 +++ arch/powerpc/kernel/dma-swiotlb.c | 48 +--- arch/x86/include/asm/dma-mapping.h | 18 ++ arch/x86/kernel/pci-dma.c |2 +- arch/x86/kernel/pci-gart_64.c |5 +- arch/x86/kernel/pci-nommu.c|2 +- arch/x86/kernel/pci-swiotlb.c | 25 include/linux/dma-mapping.h|5 -- include/linux/swiotlb.h| 11 lib/swiotlb.c | 102 +--- 11 files changed, 92 insertions(+), 167 deletions(-) Hm, the functions and facilities you remove here were added as part of preparatory patches for Xen guest support. You were aware of them, you were involved in discussions about those aspects with Ian and Jeremy but still you chose not to Cc: either of them and you failed to address that aspect in the changelogs. I'd like the Xen code to become cleaner more than anyone else here i guess, but patch submission methods like this are not really helpful. A far better method is to be open about such disagreements, to declare them, to Cc: everyone who disagrees, and to line out the arguments in the changelogs as well - instead of just curtly declaring those APIs 'unused' and failing to Cc: involved parties. Alas, on the technical level the cleanups themselves look mostly fine to me. Ian, Jeremy, the changes will alter Xen's use of swiotlb, but can the Xen side still live with these new methods - in particular is dma_capable() sufficient as a mechanism and can the Xen side filter out DMA allocations to make them physically continuous? Ben, Tony, Becky, any objections wrt. the PowerPC / IA64 impact? If everyone agrees i can apply them to the IOMMU tree, test it and push it out to -next, etc. Ingo ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [00/15] swiotlb cleanup
On Fri, 10 Jul 2009 07:12:36 +0200 Ingo Molnar mi...@elte.hu wrote: * FUJITA Tomonori fujita.tomon...@lab.ntt.co.jp wrote: - removes unused (and unnecessary) hooks in swiotlb. - adds dma_capable() and converts swiotlb to use it. It can be used to know if a memory area is dma capable or not. I added is_buffer_dma_capable() for the same purpose long ago but it turned out that the function doesn't work on POWERPC. This can be applied cleanly to linux-next, -mm, and mainline. This patchset touches multiple architectures (ia64, powerpc, x86) so I guess that -mm is appropriate for this patchset (I don't care much what tree would merge this though). This is tested on x86 but only compile tested on POWERPC and IA64. Thanks, = arch/ia64/include/asm/dma-mapping.h| 18 ++ arch/powerpc/include/asm/dma-mapping.h | 23 +++ arch/powerpc/kernel/dma-swiotlb.c | 48 +--- arch/x86/include/asm/dma-mapping.h | 18 ++ arch/x86/kernel/pci-dma.c |2 +- arch/x86/kernel/pci-gart_64.c |5 +- arch/x86/kernel/pci-nommu.c|2 +- arch/x86/kernel/pci-swiotlb.c | 25 include/linux/dma-mapping.h|5 -- include/linux/swiotlb.h| 11 lib/swiotlb.c | 102 +--- 11 files changed, 92 insertions(+), 167 deletions(-) Hm, the functions and facilities you remove here were added as part of preparatory patches for Xen guest support. You were aware of them, you were involved in discussions about those aspects with Ian and Jeremy but still you chose not to Cc: either of them and you failed to address that aspect in the changelogs. I'd like the Xen code to become cleaner more than anyone else here i guess, but patch submission methods like this are not really helpful. A far better method is to be open about such disagreements, to declare them, to Cc: everyone who disagrees, and to line out the arguments in the changelogs as well - instead of just curtly declaring those APIs 'unused' and failing to Cc: involved parties. Alas, on the technical level the cleanups themselves look mostly fine to me. Ian, Jeremy, the changes will alter Xen's use of swiotlb, but can the Xen side still live with these new methods - in particular is dma_capable() sufficient as a mechanism and can the Xen side filter out DMA allocations to make them physically continuous? dma_capable() doesn't work for Xen in the way that Ian hopes. As I said to him again and again, he tries to use arch code in the very original way, and it's unacceptable (of course, he disagreed with it). I don't think that we need to take account of dom0 support; we don't have a clear idea about an acceptable dom0 design (it needs to use swiotlb code? I don't know yet), we don't even know we will have dom0 support in mainline. That's why I didn't CC this patchset to Xen camp. I think that it's more reasonable to think about how the code can works for dom0 support when Xen people come with the new dom0 code. Ben, Tony, Becky, any objections wrt. the PowerPC / IA64 impact? If everyone agrees i can apply them to the IOMMU tree, test it and push it out to -next, etc. Ingo -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
