From: Christoph Hellwig <hch@lst.de> Date: 2015-08-12 07:09:10
Dan Williams started to look into addressing I/O to and from
Persistent Memory in his series from June:
http://thread.gmane.org/gmane.linux.kernel.cross-arch/27944
I've started looking into DMA mapping of these SGLs specifically instead
of the map_pfn method in there. In addition to supporting NVDIMM backed
I/O I also suspect this would be highly useful for media drivers that
go through nasty hoops to be able to DMA from/to their ioremapped regions,
with vb2_dc_get_userptr in drivers/media/v4l2-core/videobuf2-dma-contig.c
being a prime example for the unsafe hacks currently used.
It turns out most DMA mapping implementation can handle SGLs without
page structures with some fairly simple mechanical work. Most of it
is just about consistently using sg_phys. For implementations that
need to flush caches we need a new helper that skips these cache
flushes if a entry doesn't have a kernel virtual address.
However the ccio (parisc) and sba_iommu (parisc & ia64) IOMMUs seem
to be operate mostly on virtual addresses. It's a fairly odd concept
that I don't fully grasp, so I'll need some help with those if we want
to bring this forward.
Additional this series skips ARM entirely for now. The reason is
that most arm implementations of the .map_sg operation just iterate
over all entries and call ->map_page for it, which means we'd need
to convert those to a ->map_pfn similar to Dan's previous approach.
@@ -61,8 +61,7 @@ static int dma_direct_map_sg(struct device *dev, struct scatterlist *sgl,/* FIXME this part of code is untested */for_each_sg(sgl,sg,nents,i){sg->dma_address=sg_phys(sg);-__dma_sync(page_to_phys(sg_page(sg))+sg->offset,-sg->length,direction);+__dma_sync(sg_phys(sg),sg->length,direction);}returnnents;
From: Christoph Hellwig <hch@lst.de> Date: 2015-08-12 07:09:08
Just remove a BUG_ON, the code handles them just fine as-is.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
arch/x86/kernel/pci-nommu.c | 1 -
1 file changed, 1 deletion(-)
From: Christoph Hellwig <hch@lst.de> Date: 2015-08-12 07:09:12
For the iommu offset we just need and offset into the page. Calculate
that using the physical address instead of using the virtual address
so that we don't require a virtual mapping.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
arch/x86/kernel/pci-calgary_64.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
From: Christoph Hellwig <hch@lst.de> Date: 2015-08-12 07:09:16
For the iommu offset we just need and offset into the page. Calculate
that using the physical address instead of using the virtual address
so that we don't require a virtual mapping.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
arch/powerpc/kernel/iommu.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
From: Christoph Hellwig <hch@lst.de> Date: 2015-08-12 07:09:23
Just remove a BUG_ON, the code handles them just fine as-is.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
arch/mn10300/include/asm/dma-mapping.h | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
From: Christoph Hellwig <hch@lst.de> Date: 2015-08-12 07:09:31
Pass a PFN to iommu_get_one instad of calculating it locall from a
page structure so that we don't need pages for every address we can
DMA to or from.
Also further restrict the cache flushing as we now have a non-highmem
way of not kernel virtual mapped physical addresses.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
arch/sparc/mm/iommu.c | 17 +++++++++--------
1 file changed, 9 insertions(+), 8 deletions(-)
@@ -1267,7 +1265,7 @@ sba_fill_pdir(**LookforaVCONTIGchunk*/if(cnt){-unsignedlongvaddr=(unsignedlong)sba_sg_address(startsg);+unsignedlongvaddr=(unsignedlong)sg_virt(startsg);ASSERT(pdirp);/* Since multiple Vcontig blocks could make up
@@ -1335,7 +1333,7 @@ sba_coalesce_chunks(struct ioc *ioc, struct device *dev,intidx;while(nents>0){-unsignedlongvaddr=(unsignedlong)sba_sg_address(startsg);+unsignedlongvaddr=(unsignedlong)sg_virt(startsg);/***Prepareforfirst/nextDMAstream
@@ -1380,7 +1378,7 @@ sba_coalesce_chunks(struct ioc *ioc, struct device *dev,****appendthenexttransaction?*/-vaddr=(unsignedlong)sba_sg_address(startsg);+vaddr=(unsignedlong)sg_virt(startsg);if(vcontig_end==vaddr){vcontig_len+=startsg->length;
From: Christoph Hellwig <hch@lst.de> Date: 2015-08-12 07:09:43
Make all cache invalidation conditional on sg_has_page() and use
sg_phys to get the physical address directly.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
arch/arc/include/asm/dma-mapping.h | 26 +++++++++++++++++++-------
1 file changed, 19 insertions(+), 7 deletions(-)
From: Christoph Hellwig <hch@lst.de> Date: 2015-08-12 07:09:51
Switch from sg_virt to sg_phys as blackfin like all nommu architectures
has a 1:1 virtual to physical mapping.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
arch/blackfin/kernel/dma-mapping.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
From: Christoph Hellwig <hch@lst.de> Date: 2015-08-12 07:10:03
Only call kmap_atomic_primary when the SG entry is mapped into
kernel virtual space.
XXX: the code already looks odd due to the lack of pairing between
kmap_atomic_primary and kunmap_atomic_primary. Does it work either
before or after this patch?
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
arch/frv/mb93090-mb00/pci-dma.c | 3 +++
1 file changed, 3 insertions(+)
From: Christoph Hellwig <hch@lst.de> Date: 2015-08-12 07:10:09
Make all cache invalidation conditional on sg_has_page() and use
sg_phys to get the physical address directly. To do this consolidate
the two platform callouts using pages and virtual addresses into a
single one using a physical address.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
arch/mips/bmips/dma.c | 9 ++------
arch/mips/include/asm/mach-ath25/dma-coherence.h | 10 ++-------
arch/mips/include/asm/mach-bmips/dma-coherence.h | 4 ++--
.../include/asm/mach-cavium-octeon/dma-coherence.h | 11 ++--------
arch/mips/include/asm/mach-generic/dma-coherence.h | 12 +++--------
arch/mips/include/asm/mach-ip27/dma-coherence.h | 16 +++-----------
arch/mips/include/asm/mach-ip32/dma-coherence.h | 19 +++-------------
arch/mips/include/asm/mach-jazz/dma-coherence.h | 11 +++-------
.../include/asm/mach-loongson64/dma-coherence.h | 16 +++-----------
arch/mips/mm/dma-default.c | 25 ++++++++++++----------
10 files changed, 37 insertions(+), 96 deletions(-)
From: Christoph Hellwig <hch@lst.de> Date: 2015-08-12 07:10:15
Make all cache invalidation conditional on sg_has_page() and use
sg_phys to get the physical address directly.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
arch/parisc/kernel/pci-dma.c | 29 ++++++++++++++++++-----------
1 file changed, 18 insertions(+), 11 deletions(-)
@@ -483,11 +483,13 @@ static int pa11_dma_map_sg(struct device *dev, struct scatterlist *sglist, int nBUG_ON(direction==DMA_NONE);for_each_sg(sglist,sg,nents,i){-unsignedlongvaddr=(unsignedlong)sg_virt(sg);--sg_dma_address(sg)=(dma_addr_t)virt_to_phys(vaddr);+sg_dma_address(sg)=sg_phys(sg);sg_dma_len(sg)=sg->length;-flush_kernel_dcache_range(vaddr,sg->length);++if(sg_has_page(sg)){+flush_kernel_dcache_range((unsignedlong)sg_virt(sg),+sg->length);+}}returnnents;}
@@ -504,9 +506,10 @@ static void pa11_dma_unmap_sg(struct device *dev, struct scatterlist *sglist, in/* once we do combining we'll need to use phys_to_virt(sg_dma_address(sglist)) */-for_each_sg(sglist,sg,nents,i)-flush_kernel_vmap_range(sg_virt(sg),sg->length);-return;+for_each_sg(sglist,sg,nents,i){+if(sg_has_page(sg))+flush_kernel_vmap_range(sg_virt(sg),sg->length);+}}staticvoidpa11_dma_sync_single_for_cpu(structdevice*dev,dma_addr_tdma_handle,unsignedlongoffset,size_tsize,enumdma_data_directiondirection)
@@ -530,8 +533,10 @@ static void pa11_dma_sync_sg_for_cpu(struct device *dev, struct scatterlist *sgl/* once we do combining we'll need to use phys_to_virt(sg_dma_address(sglist)) */-for_each_sg(sglist,sg,nents,i)-flush_kernel_vmap_range(sg_virt(sg),sg->length);+for_each_sg(sglist,sg,nents,i){+if(sg_has_page(sg))+flush_kernel_vmap_range(sg_virt(sg),sg->length);+}}staticvoidpa11_dma_sync_sg_for_device(structdevice*dev,structscatterlist*sglist,intnents,enumdma_data_directiondirection)
@@ -541,8 +546,10 @@ static void pa11_dma_sync_sg_for_device(struct device *dev, struct scatterlist */* once we do combining we'll need to use phys_to_virt(sg_dma_address(sglist)) */-for_each_sg(sglist,sg,nents,i)-flush_kernel_vmap_range(sg_virt(sg),sg->length);+for_each_sg(sglist,sg,nents,i){+if(sg_has_page(sg))+flush_kernel_vmap_range(sg_virt(sg),sg->length);+}}structhppa_dma_opspcxl_dma_ops={
From: Christoph Hellwig <hch@lst.de> Date: 2015-08-12 07:10:57
Just remove a BUG_ON, the code handles them just fine as-is.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
drivers/iommu/intel-iommu.c | 1 -
1 file changed, 1 deletion(-)
From: Christoph Hellwig <hch@lst.de> Date: 2015-08-12 07:12:29
Make all cache invalidation conditional on sg_has_page() and use
sg_phys to get the physical address directly.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
arch/openrisc/kernel/dma.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
From: Christoph Hellwig <hch@lst.de> Date: 2015-08-12 07:14:31
Make all cache invalidation conditional on sg_has_page() and use
sg_phys to get the physical address directly, bypassing the noop
page_to_bus.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
arch/avr32/include/asm/dma-mapping.h | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
@@ -328,8 +326,10 @@ dma_sync_sg_for_device(struct device *dev, struct scatterlist *sglist,inti;structscatterlist*sg;-for_each_sg(sglist,sg,nents,i)-dma_cache_sync(dev,sg_virt(sg),sg->length,direction);+for_each_sg(sglist,sg,nents,i){+if(sg_has_page(sg))+dma_cache_sync(dev,sg_virt(sg),sg->length,direction);+}}/* Now for the API extensions over the pci_ one */
From: Christoph Hellwig <hch@lst.de> Date: 2015-08-12 07:14:57
Make all cache invalidation conditional on sg_has_page() and use
sg_phys to get the physical address directly.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
arch/nios2/mm/dma-mapping.c | 29 +++++++++++++++--------------
1 file changed, 15 insertions(+), 14 deletions(-)
From: Christoph Hellwig <hch@lst.de> Date: 2015-08-12 07:16:12
Use sg_phys() instead of page_to_phys(sg_page(sg)) so that we don't
require a page structure for all DMA memory.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
arch/s390/pci/pci_dma.c | 20 ++++++++++++++------
1 file changed, 14 insertions(+), 6 deletions(-)
From: Christoph Hellwig <hch@lst.de> Date: 2015-08-12 07:16:45
For the iommu offset we just need and offset into the page. Calculate
that using the physical address instead of using the virtual address
so that we don't require a virtual mapping.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
arch/sparc/mm/io-unit.c | 23 ++++++++++++-----------
1 file changed, 12 insertions(+), 11 deletions(-)
@@ -91,13 +91,14 @@ static int __init iounit_init(void)subsys_initcall(iounit_init);/* One has to hold iounit->lock to call this */-staticunsignedlongiounit_get_area(structiounit_struct*iounit,unsignedlongvaddr,intsize)+staticdma_addr_tiounit_get_area(structiounit_struct*iounit,+unsignedlongpaddr,intsize){inti,j,k,npages;-unsignedlongrotor,scan,limit;+unsignedlongrotor,scan,limit,dma_addr;iopte_tiopte;-npages=((vaddr&~PAGE_MASK)+size+(PAGE_SIZE-1))>>PAGE_SHIFT;+npages=((paddr&~PAGE_MASK)+size+(PAGE_SIZE-1))>>PAGE_SHIFT;/* A tiny bit of magic ingredience :) */switch(npages){
From: Christoph Hellwig <hch@lst.de> Date: 2015-08-12 07:17:17
Use
sg_phys(sg) & PAGE_MASK
instead of
page_to_pfn(sg_page(sg)) << PAGE_SHIFT
to get at the page-aligned physical address ofa SG entry, so that
we don't require a page backing for SG entries.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
arch/sparc/kernel/ldc.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
From: Christoph Hellwig <hch@lst.de> Date: 2015-08-12 07:18:14
Use sg_phys() instead of virt_to_phys(sg_virt(sg)) so that we don't
require a kernel virtual address.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
arch/ia64/sn/pci/pci_dma.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
From: Christoph Hellwig <hch@lst.de> Date: 2015-08-12 07:18:50
Use sg_phys() instead of virt_to_phys(sg_virt(sg)) so that we don't
require a kernel virtual address.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
arch/c6x/kernel/dma.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
From: Christoph Hellwig <hch@lst.de> Date: 2015-08-12 07:19:38
Use sg_phys() instead of virt_to_phys(sg_virt(sg)) so that we don't
require a kernel virtual address, and switch a few debug printfs to
print physical instead of virtual addresses.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
arch/alpha/kernel/pci_iommu.c | 36 +++++++++++++++---------------------
1 file changed, 15 insertions(+), 21 deletions(-)
@@ -248,20 +248,17 @@ static int pci_dac_dma_supported(struct pci_dev *dev, u64 mask)untileitherpci_unmap_singleorpci_dma_sync_singleisperformed.*/staticdma_addr_t-pci_map_single_1(structpci_dev*pdev,void*cpu_addr,size_tsize,+pci_map_single_1(structpci_dev*pdev,unsignedlongpaddr,size_tsize,intdac_allowed){structpci_controller*hose=pdev?pdev->sysdata:pci_isa_hose;dma_addr_tmax_dma=pdev?pdev->dma_mask:ISA_DMA_MASK;structpci_iommu_arena*arena;longnpages,dma_ofs,i;-unsignedlongpaddr;dma_addr_tret;unsignedintalign=0;structdevice*dev=pdev?&pdev->dev:NULL;-paddr=__pa(cpu_addr);-#if !DEBUG_NODIRECT/* First check to see if we can use the direct map window. */if(paddr+size+__direct_map_base-1<=max_dma
@@ -269,7 +266,7 @@ pci_map_single_1(struct pci_dev *pdev, void *cpu_addr, size_t size,ret=paddr+__direct_map_base;DBGA2("pci_map_single: [%p,%zx] -> direct %llx from %pf\n",-cpu_addr,size,ret,__builtin_return_address(0));+paddr,size,ret,__builtin_return_address(0));returnret;}
From: Christoph Hellwig <hch@lst.de> Date: 2015-08-12 07:20:01
Use sg_phys() instead of virt_to_phys(sg_virt(sg)) so that we don't
require a kernel virtual address.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
arch/alpha/kernel/pci-noop.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
From: Christoph Hellwig <hch@lst.de> Date: 2015-08-12 07:20:52
Use sg_pfn to get a the PFN and skip checks that require a kernel
virtual address.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
lib/dma-debug.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
Around Wed 12 Aug 2015 09:05:39 +0200 or thereabout, Christoph Hellwig wrote:
Make all cache invalidation conditional on sg_has_page() and use
sg_phys to get the physical address directly, bypassing the noop
page_to_bus.
Signed-off-by: Christoph Hellwig <hch@lst.de>
@@ -328,8 +326,10 @@ dma_sync_sg_for_device(struct device *dev, struct scatterlist *sglist,inti;structscatterlist*sg;-for_each_sg(sglist,sg,nents,i)-dma_cache_sync(dev,sg_virt(sg),sg->length,direction);+for_each_sg(sglist,sg,nents,i){+if(sg_has_page(sg))+dma_cache_sync(dev,sg_virt(sg),sg->length,direction);+}}/* Now for the API extensions over the pci_ one */
On Wednesday 12 August 2015 12:39 PM, Christoph Hellwig wrote:=0A=
Make all cache invalidation conditional on sg_has_page() and use=0A=
sg_phys to get the physical address directly.=0A=
=0A=
Signed-off-by: Christoph Hellwig <hch@lst.de>=0A=
=0A=
With a minor nit below.=0A=
=0A=
Acked-by: Vineet Gupta <redacted>=0A=
=0A=
From: Sebastian Ott <hidden> Date: 2015-08-12 11:52:18
On Wed, 12 Aug 2015, Christoph Hellwig wrote:
Use sg_phys() instead of page_to_phys(sg_page(sg)) so that we don't
require a page structure for all DMA memory.
Signed-off-by: Christoph Hellwig <hch@lst.de>
--
1.9.1
--
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/
Dan Williams started to look into addressing I/O to and from
Persistent Memory in his series from June:
http://thread.gmane.org/gmane.linux.kernel.cross-arch/27944
I've started looking into DMA mapping of these SGLs specifically instead
of the map_pfn method in there. In addition to supporting NVDIMM backed
I/O I also suspect this would be highly useful for media drivers that
go through nasty hoops to be able to DMA from/to their ioremapped regions,
with vb2_dc_get_userptr in drivers/media/v4l2-core/videobuf2-dma-contig.c
being a prime example for the unsafe hacks currently used.
The support I have suggested and submitted for zone-less sections.
(In my add_persistent_memory() patchset)
Would work perfectly well and transparent for all such multimedia cases.
(All hacks removed). In fact I have loaded pmem (with-pages) on a VRAM
a few times and it is great easy fun. (I wanted to experiment with cached
memory over a pcie)
It turns out most DMA mapping implementation can handle SGLs without
page structures with some fairly simple mechanical work. Most of it
is just about consistently using sg_phys. For implementations that
need to flush caches we need a new helper that skips these cache
flushes if a entry doesn't have a kernel virtual address.
However the ccio (parisc) and sba_iommu (parisc & ia64) IOMMUs seem
to be operate mostly on virtual addresses. It's a fairly odd concept
that I don't fully grasp, so I'll need some help with those if we want
to bring this forward.
Additional this series skips ARM entirely for now. The reason is
that most arm implementations of the .map_sg operation just iterate
over all entries and call ->map_page for it, which means we'd need
to convert those to a ->map_pfn similar to Dan's previous approach.
All this endless work for nothing more than uglyfing the Kernel, and
It will never end. When a real and fully working solution is right
here for more then a year.
If you are really up for a deep audit and a mammoth testing effort,
why not do a more worthy, and order of magnitude smaller work and support
2M and 1G variable sized "pages". All the virtual-vs-phisical-vs-caching
just works.
Most of the core work is there. Block layer and lots of other subsytems
already support sending a single page-pointer with bvec_offset bvec_len
bigger then 4K. Other system will be small fixes sprinkled around but
not at all this endless stream of subsystem after another of patches.
And for why.
The novelty of pages is the section object, the section is reached
from page* from virtual as well as physical planes. And is a center
that translate from all plains to all plains. You keep this concept
only make 2M-page sections and 1G-page sections.
It is a bit of work but is worth while, and accelerating tremendously
lots of workloads. Not like this abomination which only branches
things more and more, and making things fatter and slower.
It all feels like a typhoon, the inertia of tones and tons of
men hours work, in a huge wave. How will you ever stop such a
rushing mass. I'm trying to dock under but, surly it makes me sad.
Thanks
Boaz
On Wed, Aug 12, 2015 at 12:05 AM, Christoph Hellwig [off-list ref] wrote:
Make all cache invalidation conditional on sg_has_page() and use
sg_phys to get the physical address directly.
So this worries me a bit (I'm just reacting to one random patch in the series).
The reason?
I think this wants a big honking comment somewhere saying "non-sg_page
accesses are not necessarily cache coherent").
Now, I don't think that's _wrong_, but it's an important distinction:
if you look up pages in the page tables directly, there's a very
subtle difference between then saving just the pfn and saving the
"struct page" of the result.
On sane architectures, this whole cache flushing thing doesn't matter.
Which just means that it's going to be even more subtle on the odd
broken ones..
I'm assuming that anybody who wants to use the page-less
scatter-gather lists always does so on memory that isn't actually
virtually mapped at all, or only does so on sane architectures that
are cache coherent at a physical level, but I'd like that assumption
*documented* somewhere.
(And maybe it is, and I just didn't get to that patch yet)
Linus
[ Again, I'm responding to one random patch - this pattern was in
other patches too. ]
A question: do we actually expect to mix page-less and pageful SG
entries in the same SG list?
How does that happen?
(I'm not saying it can't, I'm just wondering where people expect this
to happen).
IOW, maybe it would be valid to have a rule saying "a SG list is
either all pageful or pageless, never mixed", and then have the "if"
statement outside the loop rather than inside.
Linus
From: James Bottomley <James.Bottomley@HansenPartnership.com> Date: 2015-08-12 17:00:16
On Wed, 2015-08-12 at 09:05 +0200, Christoph Hellwig wrote:
Dan Williams started to look into addressing I/O to and from
Persistent Memory in his series from June:
http://thread.gmane.org/gmane.linux.kernel.cross-arch/27944
I've started looking into DMA mapping of these SGLs specifically instead
of the map_pfn method in there. In addition to supporting NVDIMM backed
I/O I also suspect this would be highly useful for media drivers that
go through nasty hoops to be able to DMA from/to their ioremapped regions,
with vb2_dc_get_userptr in drivers/media/v4l2-core/videobuf2-dma-contig.c
being a prime example for the unsafe hacks currently used.
It turns out most DMA mapping implementation can handle SGLs without
page structures with some fairly simple mechanical work. Most of it
is just about consistently using sg_phys. For implementations that
need to flush caches we need a new helper that skips these cache
flushes if a entry doesn't have a kernel virtual address.
However the ccio (parisc) and sba_iommu (parisc & ia64) IOMMUs seem
to be operate mostly on virtual addresses. It's a fairly odd concept
that I don't fully grasp, so I'll need some help with those if we want
to bring this forward.
I can explain that. I think this doesn't apply to ia64 because it's
cache is PIPT, but on parisc, we have a VIPT cache.
On normal physically indexed architectures, when the iommu sees a DMA
transfer to/from physical memory, it also notifies the CPU to flush the
internal CPU caches of those lines. This is usually an interlocking
step of the transfer to make sure the page is coherent before transfer
to/from the device (it's why the ia32 for instance is a coherent
architecture). Because the system is physically indexed, there's no
need to worry about aliases.
On Virtually Indexed systems, like parisc, there is an aliasing problem.
The CCIO iommu unit (and all other iommu systems on parisc) have what's
called a local coherence index (LCI). You program it as part of the
IOMMU page table and it tells the system which Virtual line in the cache
to flush as part of the IO transaction, thus still ensuring cache
coherence. That's why we have to know the virtual as well as physical
addresses for the page. The problem we have in Linux is that we have
two virtual addresses, which are often incoherent aliases: the user
virtual address and a kernel virtual address but we can only make the
page coherent with a single alias (only one LCI). The way I/O on Linux
currently works is that get_user_pages actually flushes the user virtual
address, so that's expected to be coherent, so the address we program
into the VCI is the kernel virtual address. Usually nothing in the
kernel has ever touched the page, so there's nothing to flush, but we do
it just in case.
In theory, for these non kernel page backed SG entries, we can make the
process more efficient by not flushing in gup and instead programming
the user virtual address into the local coherence index. However,
simply zeroing the LCI will also work (except that poor VI zero line
will get flushed repeatedly, so it's probably best to pick a known
untouched line in the kernel).
James
From: Grant Grundler <hidden> Date: 2015-08-12 17:56:34
On Wed, Aug 12, 2015 at 10:00 AM, James Bottomley
[off-list ref] wrote:
On Wed, 2015-08-12 at 09:05 +0200, Christoph Hellwig wrote:
...
quoted
However the ccio (parisc) and sba_iommu (parisc & ia64) IOMMUs seem
to be operate mostly on virtual addresses. It's a fairly odd concept
that I don't fully grasp, so I'll need some help with those if we want
to bring this forward.
James explained the primary function of IOMMUs on parisc (DMA-Cache
coherency) much better than I ever could.
Three more observations:
1) the IOMMU can be bypassed by 64-bit DMA devices on IA64.
2) IOMMU enables 32-bit DMA devices to reach > 32-bit physical memory
and thus avoiding bounce buffers. parisc and older IA-64 have some
32-bit PCI devices - e.g. IDE boot HDD.
3) IOMMU acts as a proxy for IO devices by fetching cachelines of data
for PA-RISC systems whose memory controllers ONLY serve cacheline
sized transactions. ie. 32-bit DMA results in the IOMMU fetching the
cacheline and updating just the 32-bits in a DMA cache coherent
fashion.
Bonus thought:
4) IOMMU can improve DMA performance in some cases using "hints"
provided by the OS (e.g. prefetching DMA data or using READ_CURRENT
bus transactions instead of normal memory fetches.)
cheers,
grant
Hi,
On Wed, Aug 12, 2015 at 10:42 PM, Boaz Harrosh [off-list ref] wrote:
On 08/12/2015 10:05 AM, Christoph Hellwig wrote:
quoted
It turns out most DMA mapping implementation can handle SGLs without
page structures with some fairly simple mechanical work. Most of it
is just about consistently using sg_phys. For implementations that
need to flush caches we need a new helper that skips these cache
flushes if a entry doesn't have a kernel virtual address.
However the ccio (parisc) and sba_iommu (parisc & ia64) IOMMUs seem
to be operate mostly on virtual addresses. It's a fairly odd concept
that I don't fully grasp, so I'll need some help with those if we want
to bring this forward.
Additional this series skips ARM entirely for now. The reason is
that most arm implementations of the .map_sg operation just iterate
over all entries and call ->map_page for it, which means we'd need
to convert those to a ->map_pfn similar to Dan's previous approach.
[snip]
It is a bit of work but is worth while, and accelerating tremendously
lots of workloads. Not like this abomination which only branches
things more and more, and making things fatter and slower.
As a random guy reading a big bunch of patches on code I know almost
nothing about, parts of this comment really resonated with me:
overall, we seem to be adding a lot of if statements to code that
appears to be in a hot path.
I.e. ~90% of this patch set seems to be just mechanically dropping
BUG_ON()s and converting open coded stuff to use accessor functions
(which should be macros or get inlined, right?) - and the remaining
bit is not flushing if we don't have a physical page somewhere.
Would it make sense to split this patch set into a few bits: one to
drop all the useless BUG_ON()s, one to convert all the open coded
stuff to accessor functions, then another to do the actual page-less
sg stuff?
Thanks,
--
Julian Calaby
Email: julian.calaby@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/
From: Christoph Hellwig <hch@lst.de> Date: 2015-08-13 14:31:57
On Wed, Aug 12, 2015 at 09:01:02AM -0700, Linus Torvalds wrote:
I'm assuming that anybody who wants to use the page-less
scatter-gather lists always does so on memory that isn't actually
virtually mapped at all, or only does so on sane architectures that
are cache coherent at a physical level, but I'd like that assumption
*documented* somewhere.
From: Christoph Hellwig <hch@lst.de> Date: 2015-08-13 14:33:34
On Wed, Aug 12, 2015 at 09:05:15AM -0700, Linus Torvalds wrote:
[ Again, I'm responding to one random patch - this pattern was in
other patches too. ]
A question: do we actually expect to mix page-less and pageful SG
entries in the same SG list?
How does that happen?
Both for DAX and the video buffer case people could do direct I/O
spanning the boundary between such a VMA and a normal one unless
we add special code to prevent that. Right now I don't think it's
all that useful, but then again it doesn't seem harmful either
and adding those checks might add up.
From: Christoph Hellwig <hch@lst.de> Date: 2015-08-13 14:35:36
On Thu, Aug 13, 2015 at 09:37:37AM +1000, Julian Calaby wrote:
I.e. ~90% of this patch set seems to be just mechanically dropping
BUG_ON()s and converting open coded stuff to use accessor functions
(which should be macros or get inlined, right?) - and the remaining
bit is not flushing if we don't have a physical page somewhere.
Which is was 90%. By lines changed most actually is the diffs for
the cache flushing.
Would it make sense to split this patch set into a few bits: one to
drop all the useless BUG_ON()s, one to convert all the open coded
stuff to accessor functions, then another to do the actual page-less
sg stuff?
Without the ifs the BUG_ON() actually are useful to assert we
never feed the sort of physical addresses we can't otherwise support,
so I don't think that part is doable.
A simple series to make more use of sg_phys and add sg_pfn might
still be useful, though.
From: Christoph Hellwig <hch@lst.de> Date: 2015-08-13 14:40:42
On Wed, Aug 12, 2015 at 03:42:47PM +0300, Boaz Harrosh wrote:
The support I have suggested and submitted for zone-less sections.
(In my add_persistent_memory() patchset)
Would work perfectly well and transparent for all such multimedia cases.
(All hacks removed). In fact I have loaded pmem (with-pages) on a VRAM
a few times and it is great easy fun. (I wanted to experiment with cached
memory over a pcie)
And everyone agree that it was both buggy and incomplete.
Dan has done a respin of the page backed nvdimm work with most of
these comments addressed.
I have to say I hate both pfn-based I/O [1] and page backed nvdimms with
passion, so we're looking into the lesser evil with an open mind.
[1] not the SGL part posted here, which I think is quite sane. The bio
side is much worse, though.
On Wed, Aug 12, 2015 at 03:42:47PM +0300, Boaz Harrosh wrote:
quoted
The support I have suggested and submitted for zone-less sections.
(In my add_persistent_memory() patchset)
Would work perfectly well and transparent for all such multimedia cases.
(All hacks removed). In fact I have loaded pmem (with-pages) on a VRAM
a few times and it is great easy fun. (I wanted to experiment with cached
memory over a pcie)
And everyone agree that it was both buggy and incomplete.
What? No one ever said anything about bugs. Is the first ever I hear of it.
I was always in the notion that no one even tried it out.
I'm smoking these page-full nvidimms for more than a year. With RDMA to
pears and swap out to disks. So is not that bad I would say
Dan has done a respin of the page backed nvdimm work with most of
these comments addressed.
I would love some comments. All I got so far is silence. (And I do not
like Dan's patches comments will come next week)
I have to say I hate both pfn-based I/O [1] and page backed nvdimms with
passion, so we're looking into the lesser evil with an open mind.
[1] not the SGL part posted here, which I think is quite sane. The bio
side is much worse, though.
What can I say. I like the page-backed nvdimms. And the long term for me
is 2M pages. I hope we can sit one day soon and you explain to me whats
evil about it. I would really really like to understand
Thanks though
Boaz
Hi Christoph,
On Fri, Aug 14, 2015 at 12:35 AM, Christoph Hellwig [off-list ref] wrote:
On Thu, Aug 13, 2015 at 09:37:37AM +1000, Julian Calaby wrote:
quoted
I.e. ~90% of this patch set seems to be just mechanically dropping
BUG_ON()s and converting open coded stuff to use accessor functions
(which should be macros or get inlined, right?) - and the remaining
bit is not flushing if we don't have a physical page somewhere.
Which is was 90%. By lines changed most actually is the diffs for
the cache flushing.
I was talking in terms of changes made, not lines changed: by my
recollection, about a third of the patches didn't touch flush calls
and most of the lines changed looked like refactoring so that making
the flush call conditional would be easier.
I guess it smelled like you were doing lots of distinct changes in a
single patch and I got my numbers wrong.
quoted
Would it make sense to split this patch set into a few bits: one to
drop all the useless BUG_ON()s, one to convert all the open coded
stuff to accessor functions, then another to do the actual page-less
sg stuff?
Without the ifs the BUG_ON() actually are useful to assert we
never feed the sort of physical addresses we can't otherwise support,
so I don't think that part is doable.
My point is that there's a couple of patches that only remove
BUG_ON()s, which implies that for that particular driver it doesn't
matter if there's a physical page or not, so therefore that code is
purely "documentation".
Thanks,
--
Julian Calaby
Email: julian.calaby@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/
From: Dan Williams <hidden> Date: 2015-08-14 03:30:53
On Thu, Aug 13, 2015 at 7:31 AM, Christoph Hellwig [off-list ref] wrote:
On Wed, Aug 12, 2015 at 09:01:02AM -0700, Linus Torvalds wrote:
quoted
I'm assuming that anybody who wants to use the page-less
scatter-gather lists always does so on memory that isn't actually
virtually mapped at all, or only does so on sane architectures that
are cache coherent at a physical level, but I'd like that assumption
*documented* somewhere.
For virtually tagged caches I assume we would temporarily map with
kmap_atomic_pfn_t(), similar to how drm_clflush_pages() implements
powerpc support. However with DAX we could end up with multiple
virtual aliases for a page-less pfn.
From: James Bottomley <James.Bottomley@HansenPartnership.com> Date: 2015-08-14 03:59:27
On Thu, 2015-08-13 at 20:30 -0700, Dan Williams wrote:
On Thu, Aug 13, 2015 at 7:31 AM, Christoph Hellwig [off-list ref] wrote:
quoted
On Wed, Aug 12, 2015 at 09:01:02AM -0700, Linus Torvalds wrote:
quoted
I'm assuming that anybody who wants to use the page-less
scatter-gather lists always does so on memory that isn't actually
virtually mapped at all, or only does so on sane architectures that
are cache coherent at a physical level, but I'd like that assumption
*documented* somewhere.
For virtually tagged caches I assume we would temporarily map with
kmap_atomic_pfn_t(), similar to how drm_clflush_pages() implements
powerpc support. However with DAX we could end up with multiple
virtual aliases for a page-less pfn.
At least on some PA architectures, you have to be very careful.
Improperly managed, multiple aliases will cause the system to crash
(actually a machine check in the cache chequerboard). For the most
temperamental systems, we need the cache line flushed and the alias
mapping ejected from the TLB cache before we access the same page at an
inequivalent alias.
James
From: David Miller <davem@davemloft.net> Date: 2015-08-14 04:12:01
From: James Bottomley <James.Bottomley@HansenPartnership.com>
Date: Thu, 13 Aug 2015 20:59:20 -0700
On Thu, 2015-08-13 at 20:30 -0700, Dan Williams wrote:
quoted
On Thu, Aug 13, 2015 at 7:31 AM, Christoph Hellwig [off-list ref] wrote:
quoted
On Wed, Aug 12, 2015 at 09:01:02AM -0700, Linus Torvalds wrote:
quoted
I'm assuming that anybody who wants to use the page-less
scatter-gather lists always does so on memory that isn't actually
virtually mapped at all, or only does so on sane architectures that
are cache coherent at a physical level, but I'd like that assumption
*documented* somewhere.
For virtually tagged caches I assume we would temporarily map with
kmap_atomic_pfn_t(), similar to how drm_clflush_pages() implements
powerpc support. However with DAX we could end up with multiple
virtual aliases for a page-less pfn.
At least on some PA architectures, you have to be very careful.
Improperly managed, multiple aliases will cause the system to crash
(actually a machine check in the cache chequerboard). For the most
temperamental systems, we need the cache line flushed and the alias
mapping ejected from the TLB cache before we access the same page at an
inequivalent alias.
Also, I want to mention that on sparc64 we manage the cache aliasing
state in the page struct.
Until a page is mapped into userspace, we just record the most recent
cpu to store into that page with kernel side mappings. Once the page
ends up being mapped or the cpu doing kernel side stores changes, we
actually perform the cache flush.
Generally speaking, I think that all actual physical memory the kernel
operates on should have a struct page backing it. So this whole
discussion of operating on physical memory in scatter lists without
backing page structs feels really foreign to me.
From: Dan Williams <hidden> Date: 2015-08-14 16:17:52
On Thu, Aug 13, 2015 at 9:11 PM, David Miller [off-list ref] wrote:
From: James Bottomley <James.Bottomley@HansenPartnership.com>
quoted
At least on some PA architectures, you have to be very careful.
Improperly managed, multiple aliases will cause the system to crash
(actually a machine check in the cache chequerboard). For the most
temperamental systems, we need the cache line flushed and the alias
mapping ejected from the TLB cache before we access the same page at an
inequivalent alias.
Also, I want to mention that on sparc64 we manage the cache aliasing
state in the page struct.
Until a page is mapped into userspace, we just record the most recent
cpu to store into that page with kernel side mappings. Once the page
ends up being mapped or the cpu doing kernel side stores changes, we
actually perform the cache flush.
Generally speaking, I think that all actual physical memory the kernel
operates on should have a struct page backing it. So this whole
discussion of operating on physical memory in scatter lists without
backing page structs feels really foreign to me.
So the only way for page-less pfns to enter the system is through the
->direct_access() method provided by a pmem device's struct
block_device_operations. Architectures that require struct page for
cache management to must disable ->direct_access() in this case.
If an arch still wants to support pmem+DAX then it needs something
like this patchset (feedback welcome) to map pmem pfns:
https://lkml.org/lkml/2015/8/12/970
Effectively this would disable ->direct_access() on /dev/pmem0, but
permit ->direct_access() on /dev/pmem0m.