Re: [PATCH] ion: Handle the memory mapping correctly on x86

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 08/03/2015 05:21 PM, Radhakrishna, Pradeep wrote:
 From b4069e7fe1b2aa1660b3f944e246c13b4947db21 Mon Sep 17 00:00:00 2001
From: Zhebin Jin <zhebin.jin@xxxxxxxxx>
Date: Wed, 8 Jul 2015 10:35:06 +0800
Subject: [PATCH] ion: Handle the memory mapping correctly on x86

This patch modifies the ion page pool code to address
limitation in x86 PAT. That is, when one physical page is
mapped to multiple virtual pages, the same cache policy
should be used. So there is a need to add set_memory_wc/uc
call to avoid aliases. Otherwise, all mappings will be
cached(write back), no matter which cache policy is choosed
to do the mapping.


I haven't looked at other x86 code much before, how is this case
handled elsewhere? Can you also cite a specification or document
where this is necessary? I know alias mappings are documented in
the ARM ARM.
Signed-off-by: Zhebin Jin <zhebin.jin@xxxxxxxxx>
Signed-off-by: Pradeep Radhakrishna <pradeep.radhakrishna@xxxxxxxxx>
---
  drivers/staging/android/ion/ion_page_pool.c   |   33 ++++++++++++++++++++++---
  drivers/staging/android/ion/ion_priv.h        |    2 +-
  drivers/staging/android/ion/ion_system_heap.c |    5 ++--
  3 files changed, 34 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/android/ion/ion_page_pool.c b/drivers/staging/android/ion/ion_page_pool.c
index 0e20e62..566946b 100644
--- a/drivers/staging/android/ion/ion_page_pool.c
+++ b/drivers/staging/android/ion/ion_page_pool.c
@@ -21,6 +21,11 @@
  #include <linux/list.h>
  #include <linux/module.h>
  #include <linux/slab.h>
+
+#if defined(CONFIG_X86)
+#include <asm/cacheflush.h>
+#endif
+
  #include "ion_priv.h"

  struct ion_page_pool_item {
@@ -34,6 +39,16 @@ static void *ion_page_pool_alloc_pages(struct ion_page_pool *pool)

  	if (!page)
  		return NULL;
+
+#if defined(CONFIG_X86)
+	{
+		void *va = page_address(page);
+
+		if (va)
+			set_memory_wc((unsigned long)va, 1 << pool->order);
+	}
+#endif
+
  	ion_pages_sync_for_device(NULL, page, PAGE_SIZE << pool->order,
  						DMA_BIDIRECTIONAL);
  	return page;
@@ -42,6 +57,15 @@ static void *ion_page_pool_alloc_pages(struct ion_page_pool *pool)
  static void ion_page_pool_free_pages(struct ion_page_pool *pool,
  				     struct page *page)
  {
+#if defined(CONFIG_X86)
+	{
+		void *va = page_address(page);
+
+		if (va)
+			set_memory_wb((unsigned long)va, 1 << pool->order);
+	}
+#endif
+
  	__free_pages(page, pool->order);
  }

@@ -108,11 +132,14 @@ void *ion_page_pool_alloc(struct ion_page_pool *pool)
  	return page;
  }

-void ion_page_pool_free(struct ion_page_pool *pool, struct page *page)
+void ion_page_pool_free(struct ion_page_pool *pool, struct page *page,
+			bool skip_pool)
  {
-	int ret;
+	int ret = -ENOMEM;
+
+	if (!skip_pool)
+		ret = ion_page_pool_add(pool, page);

-	ret = ion_page_pool_add(pool, page);
  	if (ret)
  		ion_page_pool_free_pages(pool, page);
  }
diff --git a/drivers/staging/android/ion/ion_priv.h b/drivers/staging/android/ion/ion_priv.h
index 7c06e2e..ac635bcb 100644
--- a/drivers/staging/android/ion/ion_priv.h
+++ b/drivers/staging/android/ion/ion_priv.h
@@ -379,7 +379,7 @@ struct ion_page_pool {
  struct ion_page_pool *ion_page_pool_create(gfp_t gfp_mask, unsigned int order);
  void ion_page_pool_destroy(struct ion_page_pool *);
  void *ion_page_pool_alloc(struct ion_page_pool *);
-void ion_page_pool_free(struct ion_page_pool *, struct page *);
+void ion_page_pool_free(struct ion_page_pool *, struct page *, bool skip_pool);

  /** ion_page_pool_shrink - shrinks the size of the memory cached in the pool
   * @pool:		the pool
diff --git a/drivers/staging/android/ion/ion_system_heap.c b/drivers/staging/android/ion/ion_system_heap.c
index 5945445..7bef39d 100644
--- a/drivers/staging/android/ion/ion_system_heap.c
+++ b/drivers/staging/android/ion/ion_system_heap.c
@@ -91,10 +91,11 @@ static void free_buffer_page(struct ion_system_heap *heap,
  {
  	bool cached = ion_buffer_cached(buffer);

-	if (!cached && !(buffer->private_flags & ION_PRIV_FLAG_SHRINKER_FREE)) {
+	if (!cached) {
  		struct ion_page_pool *pool = heap->pools[order_to_index(order)];

-		ion_page_pool_free(pool, page);
+		ion_page_pool_free(pool, page, buffer->private_flags &
+				ION_PRIV_FLAG_SHRINKER_FREE);
  	} else {
  		__free_pages(page, order);
  	}


Can you split the change to ion_page_pool_free into a separate patch? It's not
clear what that has to do with correct memory mapping.

Thanks,
Laura

_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel



[Index of Archives]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux