lkml.org 
[lkml]   [2010]   [Sep]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
Subject[PATCH 0/5] MIPS: HIGHMEM DMA on noncoherent MIPS32 processors
I had some time this weekend to revisit an old problem:

http://www.linux-mips.org/archives/linux-mips/2010-05/msg00009.html

PROBLEM #1:

When testing Dezhong's patch, I found that un-kmapped pages could get
passed into dma_map_sg(). The original patch did not handle this case.
This resulted in serious DMA coherency issues on my system, along the
lines of "/sbin/init segfaults on boot."

I found that this calling sequence, seen during SATA disk I/O, was the
culprit:

[<8002b3d4>] dma_map_sg+0x2ac/0x2b8
[<802cdb70>] ata_qc_issue+0x274/0x37c
[<802d383c>] ata_scsi_translate+0xb0/0x1e8
[<802d7274>] ata_scsi_queuecmd+0xc8/0x2b8
[<802b0630>] scsi_dispatch_cmd+0x118/0x2b8
[<802b6c64>] scsi_request_fn+0x420/0x4d8
[<80239550>] __blk_run_queue+0x84/0x19c
[<802364a0>] elv_insert+0x158/0x2e4
[<8023a398>] __make_request+0x11c/0x4d0
[<80238144>] generic_make_request+0x348/0x4a0
[<80238330>] submit_bio+0x94/0x13c
[<80114024>] mpage_bio_submit+0x30/0x40
[<80115a2c>] mpage_readpages+0x134/0x170
[<800a4fd4>] __do_page_cache_readahead+0x214/0x314

__do_page_cache_readahead() allocates a bunch of pages for the I/O
requests, but I do not see them getting mapped to kernel addresses via
kmap() / kmap_atomic(). This explains why (PageHighMem(page) &&
!kmap_high_get(page)) can be true.

I do not think there is anything wrong with this behavior, so it seems
like dma_map_sg() will need to handle it somehow. Looking at other
architectures:

The PPC approach (__dma_sync_page_highmem()) is to disable IRQs and call
kmap_atomic() to create a temporary mapping for the page. Disabling
IRQs is necessary because it is possible (but not required) for the
dma_* functions to be invoked from interrupt context; it also disables
preemption. kmap_atomic() guarantees a unique mapping per CPU.

Interestingly, PPC does not use kmap_high_get() at all.

The ARM approach (kmap_high_l1_vipt()) recognizes that it is not always
desirable to keep IRQs disabled during DMA flushes, so the ARM
maintainers implemented a sort of "reentrant kmap_atomic()" that allows
multiple contexts to share the same pte by saving and restoring whatever
was there prior to the DMA sync.

Due to its use of kmap_high_get(), the ARM approach suffers from problem
#2, below. It is also more complex and harder to test. I'm not sure
how to recreate some of the worst corner cases, e.g. hardirq cacheflush
interrupts a softirq cacheflush which interrupted a user cacheflush.

Dezhong's patch was largely based on the ARM scheme, minus the
kmap_high_l1_vipt() logic. I am sending an update to this patch which
attempts to imitate the PPC approach instead. There is a considerable
amount of reuse since both strategies require similar modifications to
dma-default.c, in order to get that code to pass around "struct page"
pointers rather than kseg0 addresses.


PROBLEM #2:

Consider this sequence:

mm/highmem.c:kmap_high()
mm/highmem.c:map_new_virtual()
mm/highmem.c:flush_all_zero_pkmaps()
arch/mips/kernel/smp.c:flush_tlb_kernel_range()
kernel/softirq.c:on_each_cpu()
kernel/smp.c:smp_call_function()

The first thing kmap_high() does is lock_kmap(), which disables
interrupts on kmap_high_get() architectures:

#ifdef ARCH_NEEDS_KMAP_HIGH_GET
#define lock_kmap() spin_lock_irq(&kmap_lock)
...
#else
#define lock_kmap() spin_lock(&kmap_lock)
#endif

smp_call_function() may not be called with interrupts disabled:

* You must not call this function with disabled interrupts or from a
* hardware interrupt handler or from a bottom half handler.

So, on SMP, we get warnings like:

------------[ cut here ]------------
WARNING: at kernel/smp.c:293 smp_call_function_single+0x17c/0x260()
Modules linked in:
Call Trace:
[<800160f0>] dump_stack+0x8/0x34
[<8004a544>] warn_slowpath_common+0x78/0xa4
[<8004a588>] warn_slowpath_null+0x18/0x24
[<800876e4>] smp_call_function_single+0x17c/0x260
[<80087ca8>] smp_call_function+0x28/0x38
[<800531b4>] on_each_cpu+0x1c/0x80
[<80026d20>] flush_tlb_kernel_range+0x28/0x34
[<800b95f4>] kmap_high+0x1dc/0x270
[<8002a570>] __kmap+0x60/0x7c
[<801f15b4>] do_readpage+0x68/0x540
[<801f218c>] ubifs_write_begin+0xbc/0x53c
[<8009a558>] generic_perform_write+0xd4/0x1f0
[<8009a6e4>] generic_file_buffered_write+0x70/0xbc
[<8009dd3c>] __generic_file_aio_write+0x2fc/0x600
[<8009e0b0>] generic_file_aio_write+0x70/0xf4
[<800da864>] do_sync_write+0xc4/0x13c
[<800db50c>] vfs_write+0xc0/0x168
[<800db6ac>] sys_write+0x4c/0xa4
[<80003d9c>] stack_done+0x20/0x3c
---[ end trace 8abde6adefbcc81f ]---
I did some digging and found that ARM runs into the same problem. For
processors that cannot "broadcast" TLB operations, SMP + HIGHMEM are
deemed incompatible:

http://www.spinics.net/lists/arm-kernel/msg74563.html

So, I would opt for the PPC approach in order to avoid this conflict.
Disabling SMP will result in a much nastier performance problem than
blocking interrupts during flushes.


PROBLEM #3:

Regarding David's flush_data_cache_page() concern:

http://www.linux-mips.org/archives/linux-mips/2008-03/msg00011.html


There are at least 4 reasons to flush the cache:

1) Boot time (cache contents are undefined at reset)

2) DMA coherence

3) I$/D$ coherence (self-modifying code)

4) Zap cache aliases


My interpretation of the code is that flush_dcache_page() is only called
for #3 and #4:

#define ARCH_IMPLEMENTS_FLUSH_DCACHE_PAGE 1
static inline void flush_dcache_page(struct page *page)
{
if (cpu_has_dc_aliases || !cpu_has_ic_fills_f_dc)
__flush_dcache_page(page);
}

The fact that the L2 / board cache is not flushed in c-r4k.c leads me to
believe that #2 is not an intended use of this function:

static inline void local_r4k_flush_data_cache_page(void * addr)
{
r4k_blast_dcache_page((unsigned long) addr);
}
Besides, we already have a standard, documented DMA API that should be
used instead.

Since this is not boot time, and HIGHMEM is incompatible with cache
aliases, that leaves #3 as a possible issue. Is this function ever used
for I$/D$ coherence?

I should note that ARM has special handling in __flush_dcache_page() for
high pages. IIRC, at one point they tried to support HIGHMEM on systems
with cache aliases, before deciding it was not feasible. Maybe this
code is just a relic - or maybe it's really needed for something.



\
 
 \ /
  Last update: 2010-09-07 06:23    [from the cache]
©2003-2014 Jasper Spaans. Advertise on this site