On Thu, 2009-08-13 at 23:34 +0200, Uwe Kleine-König wrote:

I think it should, since it uses kunmap_high() to undo whatever kmap_high_get() did. Now, if there'd been a kmap_high_put()... :-)

As to the patch, its not quite right.

From what I understand kmap_high_get() is used to pin a page's kmap iff it has one, whereas the result of your patch seems to be that it'll actually create one if its not found.

Something like the below ought to do I guess.

Signed-off-by: Peter Zijlstra <a.p....@chello.nl>

--- arch/arm/include/asm/highmem.h | 1 + arch/arm/mm/dma-mapping.c | 2 +- mm/highmem.c | 50 ++++++++++++++++++--------------------- 3 files changed, 25 insertions(+), 28 deletions(-)

diff --git a/arch/arm/include/asm/highmem.h b/arch/arm/include/asm/highmem.h index 7f36d00..4d9573b 100644 --- a/arch/arm/include/asm/highmem.h +++ b/arch/arm/include/asm/highmem.h @@ -19,6 +19,7 @@ extern pte_t *pkmap_page_table;

extern void *kmap_high(struct page *page); extern void *kmap_high_get(struct page *page); +extern void *kmap_high_put(struct page *page); extern void kunmap_high(struct page *page);

extern void *kmap(struct page *page); diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c index 1576176..4a166d9 100644 --- a/arch/arm/mm/dma-mapping.c +++ b/arch/arm/mm/dma-mapping.c @@ -551,7 +551,7 @@ static void dma_cache_maint_contiguous(struct page *page,
unsigned long offset, if (vaddr) { vaddr += offset; inner_op(vaddr, vaddr + size); - kunmap_high(page); + kmap_high_put(page); } }

diff --git a/mm/highmem.c b/mm/highmem.c index 66e915a..b2eaefe 100644 --- a/mm/highmem.c +++ b/mm/highmem.c @@ -75,26 +75,6 @@ pte_t * pkmap_page_table;

static DECLARE_WAIT_QUEUE_HEAD(pkmap_wait);

- -/* - * Most architectures have no use for kmap_high_get(), so let's abstract - * the disabling of IRQ out of the locking in that case to save on a - * potential useless overhead. - */ -#ifdef ARCH_NEEDS_KMAP_HIGH_GET -#define lock_kmap() spin_lock_irq(&kmap_lock) -#define unlock_kmap() spin_unlock_irq(&kmap_lock) -#define lock_kmap_any(flags) spin_lock_irqsave(&kmap_lock, flags) -#define unlock_kmap_any(flags) spin_unlock_irqrestore(&kmap_lock, flags) -#else -#define lock_kmap() spin_lock(&kmap_lock) -#define unlock_kmap() spin_unlock(&kmap_lock) -#define lock_kmap_any(flags) \ - do { spin_lock(&kmap_lock); (void)(flags); } while (0) -#define unlock_kmap_any(flags) \ - do { spin_unlock(&kmap_lock); (void)(flags); } while (0) -#endif - /* * Try to free a given kmap slot. * @@ -361,22 +341,38 @@ EXPORT_SYMBOL(kmap_high); * * Returns the page's current virtual memory address, or NULL if no mapping * exists. When and only when a non null address is returned then a - * matching call to kunmap_high() is necessary. + * matching call to kmap_high_put() is necessary. * * This can be called from any context. */ void *kmap_high_get(struct page *page) { - unsigned long vaddr, flags; + unsigned long vaddr;

- lock_kmap_any(flags); +again: vaddr = (unsigned long)page_address(page); if (vaddr) { - BUG_ON(pkmap_count[PKMAP_NR(vaddr)] < 1); - pkmap_count[PKMAP_NR(vaddr)]++; + atomic_t *counter = &pkmap_count[PKMAP_NR(vaddr)]; + if (atomic_inc_not_zero(counter)) { + unsigned long vaddr2 = (unsigned long)page_address(page); + + if (likely(vaddr == vaddr2)) + return (void *)vaddr; + + pkmap_put(counter); + goto again; + } } - unlock_kmap_any(flags); - return (void*) vaddr; + + return NULL; +} + +void kmap_high_put(struct page *page) +{ + unsigned long vaddr = (unsigned long)page_address(page); + + BUG_ON(!vaddr); + pkmap_put(&pkmap_count[PKMAP_NR(vaddr)]); } #endif