Problems with dma_alloc_writecombine

Problems with dma_alloc_writecombine

[Dave Hylands]

Hi,

We've observed a problem with dma_alloc_writecombine when the system
is under heavy load (heavy bus traffic).

We've observed the problem under 2.6.27.18 and 2.6.32.9 (the 2
versions of linux we're using).

Our processor is an ARM1176 (at least I think that's what it is). The
first few lines of the boot show:
Linux version 2.6.32.9 (dhylands at lc-rmna-017) (gcc version 4.3.2 (Wind
River Linux Sourcery G++ 4.3-85) ) #2 PREEMPT Fri Aug 20 18:35:25 PDT
2010
CPU: ARMv6-compatible processor [410fb767] revision 7 (ARMv7), cr=00c5387f
CPU: VIPT aliasing data cache, VIPT aliasing instruction cache

We've managed to reduce the problem to the following snippet, which is
run from a ktrhread in a continuous loop:

  void *virtAddr;
  dma_addr_t physAddr;
  unsigned int numBytes = 256;

  for (;;) {
      virtAddr = dma_alloc_writecombine(NULL,
            numBytes, &physAddr, GFP_KERNEL);
      if (virtAddr == NULL) {
         printk(KERN_ERR "Running out of memory\n");
         break;
      }

      /* access DMA memory allocated */
      tmp = virtAddr;
      *tmp = 0x77;

      /* free DMA memory */
      dma_free_writecombine(NULL,
            numBytes, virtAddr, physAddr);

        ...sleep here...
    }

By itself, the code will run forever with no issues. However, as we
increase our bus traffic (typically using DMA) then the *tmp = 0x77
line will eventually cause a page fault. If we add a small delay (a
few microseconds) before the *tmp = 0x77, then we don't see a page
fault, even under heavy load.

This suggests to me that there is some circumstance under which the
write to the PTE hasn't actually been comitted to memory by the time
the *tmp = 0x77 line is executed. We're investigating the bus
priorities to see if the CPU is lower or higher than the DMA
operations.

So far, the evidence suggests that the set_pte_ext inside __dma_alloc
somehow isn't getting written out to memory before the *tmp = 0x77
line.

It feels like the MMU tried to access the PTE while the write (for the
PTE entry) was still in the write fifo. Is this possible?
Would adding a read of the PTE force the CPU to wait until the write
buffer was sufficiently drained such the PTE write is actually
committed to memory?

-- 
Dave Hylands
Shuswap, BC, Canada
http://www.DaveHylands.com/

Problems with dma_alloc_writecombine

[Dave Hylands]

Replying to my own post....

On Fri, Aug 20, 2010 at 8:14 PM, Dave Hylands wrote:
> Hi,
>
> We've observed a problem with dma_alloc_writecombine when the system
> is under heavy load (heavy bus traffic).

I found the problem (well I think I found the problem)

The armv6_set_pte_ext function (from arch/arm/mm/proc-macros.S) does
this at the end:

	mcr	p15, 0, r0, c7, c10, 1		@ flush_pte

which doesn't wait for the write to hit the memory. I added

	mcr	p15, 0, r0, c7, c10, 4		@ dsb

which ensures that the write buffer has been drained before
continuing, and my problem goes away.

I wasn't sure if the dsb is a complete superset of flush_pte. If it
is, then the flush_pte can be replaced with a dsb (for my test, I
added the dsb).

I found some references in the ARM literature:
<http://infocenter.arm.com/help/topic/com.arm.doc.genc007826/Barrier_Litmus_Tests_and_Cookbook_A08.pdf>

In "9.2.1 Ensuring the visibility of updates to instructions for a
uniprocessor" (page 24) they use the dsb instruction, where the
armv6_set_pte_ext function doesn't. Their example also covers the more
general case where the page could refer to instructions or data,
whereas the dma stuff is data only.

I've checked the sources, and the problem still seems to exist in the
latest kernel (2.6.35).

I've never submitted a patch to the kernel before. Is there a web page
which describes the process?

I gather from other posts that using gmail is unacceptable - which is
fine, I have other email accounts I can use to actually submit the
patch.

-- 
Dave Hylands
Shuswap, BC, Canada
http://www.DaveHylands.com/

Problems with dma_alloc_writecombine

[Baruch Siach]

Hi Dave,

On Tue, Aug 24, 2010 at 09:50:15AM -0700, Dave Hylands wrote:
> I've never submitted a patch to the kernel before. Is there a web page
> which describes the process?

See the Documentation/SubmittingPatches document from the kernel source tree.  
You may also read this document via the git web interface at 
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=blob;f=Documentation/SubmittingPatches

> I gather from other posts that using gmail is unacceptable - which is
> fine, I have other email accounts I can use to actually submit the
> patch.

As far as I know only with the web GUI of gmail is problematic. You may still 
use your gmail account with other email clients. See 
Documentation/email-clients.txt or 
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=blob;f=Documentation/email-clients.txt

baruch

-- 
                                                     ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch at tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -

Problems with dma_alloc_writecombine

[Russell King - ARM Linux]

On Tue, Aug 24, 2010 at 09:50:15AM -0700, Dave Hylands wrote:
> Replying to my own post....
> 
> On Fri, Aug 20, 2010 at 8:14 PM, Dave Hylands wrote:
> > Hi,
> >
> > We've observed a problem with dma_alloc_writecombine when the system
> > is under heavy load (heavy bus traffic).
> 
> I found the problem (well I think I found the problem)
> 
> The armv6_set_pte_ext function (from arch/arm/mm/proc-macros.S) does
> this at the end:
> 
> 	mcr	p15, 0, r0, c7, c10, 1		@ flush_pte
> 
> which doesn't wait for the write to hit the memory. I added
> 
> 	mcr	p15, 0, r0, c7, c10, 4		@ dsb
> 
> which ensures that the write buffer has been drained before
> continuing, and my problem goes away.
> 
> I wasn't sure if the dsb is a complete superset of flush_pte. If it
> is, then the flush_pte can be replaced with a dsb (for my test, I
> added the dsb).
> 
> I found some references in the ARM literature:
> <http://infocenter.arm.com/help/topic/com.arm.doc.genc007826/Barrier_Litmus_Tests_and_Cookbook_A08.pdf>
> 
> In "9.2.1 Ensuring the visibility of updates to instructions for a
> uniprocessor" (page 24) they use the dsb instruction, where the
> armv6_set_pte_ext function doesn't. Their example also covers the more
> general case where the page could refer to instructions or data,
> whereas the dma stuff is data only.

Partly that's because it's overhead which we don't need there.  Probably
a better solution is to add the dsb() to __dma_alloc_remap() like this:

 arch/arm/mm/dma-mapping.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
index c704eed..4bc43e5 100644
--- a/arch/arm/mm/dma-mapping.c
+++ b/arch/arm/mm/dma-mapping.c
@@ -229,6 +229,8 @@ __dma_alloc_remap(struct page *page, size_t size, gfp_t gfp, pgprot_t prot)
 			}
 		} while (size -= PAGE_SIZE);
 
+		dsb();
+
 		return (void *)c->vm_start;
 	}
 	return NULL;

Problems with dma_alloc_writecombine

[Dave Hylands]

Hi Russell,

...snip...
> Partly that's because it's overhead which we don't need there. ?Probably
> a better solution is to add the dsb() to __dma_alloc_remap() like this:
>
> ?arch/arm/mm/dma-mapping.c | ? ?2 ++
> ?1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
> index c704eed..4bc43e5 100644
> --- a/arch/arm/mm/dma-mapping.c
> +++ b/arch/arm/mm/dma-mapping.c
> @@ -229,6 +229,8 @@ __dma_alloc_remap(struct page *page, size_t size, gfp_t gfp, pgprot_t prot)
> ? ? ? ? ? ? ? ? ? ? ? ?}
> ? ? ? ? ? ? ? ?} while (size -= PAGE_SIZE);
>
> + ? ? ? ? ? ? ? dsb();
> +
> ? ? ? ? ? ? ? ?return (void *)c->vm_start;
> ? ? ? ?}
> ? ? ? ?return NULL;

I can confirm that your patch also addresses our issue.

-- 
Dave Hylands
Shuswap, BC, Canada
http://www.DaveHylands.com/