Message-ID: <20190128214408.25442-1-afd@ti.com>
Date:   Mon, 28 Jan 2019 15:44:08 -0600
From:   "Andrew F. Davis" <afd@...com>
To:     Laura Abbott <labbott@...hat.com>,
        Sumit Semwal <sumit.semwal@...aro.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Arve Hjønnevåg <arve@...roid.com>,
        Christoph Hellwig <hch@...radead.org>,
        Liam Mark <lmark@...eaurora.org>,
        Brian Starkey <Brian.Starkey@....com>
CC:     <devel@...verdev.osuosl.org>, <dri-devel@...ts.freedesktop.org>,
        <linux-kernel@...r.kernel.org>, "Andrew F . Davis" <afd@...com>
Subject: [PATCH v2] staging: android: ion: Allocate from heap ID directly without mask

Previously the heap to allocate from was selected by a mask of allowed
heap types. This may have been done as a primitive form of constraint
solving, the first heap type that matched any set bit of the heap mask
was allocated from, unless that heap was excluded by having its heap
ID bit not set in the separate passed in heap ID mask.

The heap type does not really represent constraints that should be
matched against to begin with. So the patch [0] removed the the heap type
mask matching but unfortunately left the heap ID mask check (possibly by
mistake or to preserve API). Therefor we now only have a mask of heap
IDs, but heap IDs are unique identifiers and have nothing to do with the
underlying heap, so mask matching is not useful here. This also limits us
to 32 heaps total in a system.

With the heap query API users can find the right heap based on type or
name themselves then just supply the ID for that heap. Remove heap ID
mask and allow users to specify heap ID directly by its number.

I know this is an ABI break, but we are in staging so lets get this over
with now rather than limit ourselves later.

[0] commit 2bb9f5034ec7 ("gpu: ion: Remove heapmask from client")

Signed-off-by: Andrew F. Davis <afd@...com>
---

This also means we don't need to store the available heaps in a plist,
we only operation we care about is lookup, so a better data structure
should be chosen at some point, regular list or xarray maybe?

This is base on -next as to be on top of the other taken Ion patches.

Changes from v1:
 - Fix spelling in commit message
 - Cleanup logic per Brian's suggestion

 drivers/staging/android/ion/ion.c  | 28 +++++++++++++---------------
 drivers/staging/android/uapi/ion.h |  6 ++----
 2 files changed, 15 insertions(+), 19 deletions(-)

diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c
index 92c2914239e3..b0b0d0b587c2 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -387,7 +387,7 @@ static const struct dma_buf_ops dma_buf_ops = {
 	.unmap = ion_dma_buf_kunmap,
 };
 
-static int ion_alloc(size_t len, unsigned int heap_id_mask, unsigned int flags)
+static int ion_alloc(size_t len, unsigned int heap_id, unsigned int flags)
 {
 	struct ion_device *dev = internal_dev;
 	struct ion_buffer *buffer = NULL;
@@ -396,27 +396,25 @@ static int ion_alloc(size_t len, unsigned int heap_id_mask, unsigned int flags)
 	int fd;
 	struct dma_buf *dmabuf;
 
-	pr_debug("%s: len %zu heap_id_mask %u flags %x\n", __func__,
-		 len, heap_id_mask, flags);
-	/*
-	 * traverse the list of heaps available in this system in priority
-	 * order.  If the heap type is supported by the client, and matches the
-	 * request of the caller allocate from it.  Repeat until allocate has
-	 * succeeded or all heaps have been tried
-	 */
+	pr_debug("%s: len %zu heap_id %u flags %x\n", __func__,
+		 len, heap_id, flags);
+
 	len = PAGE_ALIGN(len);
 
 	if (!len)
 		return -EINVAL;
 
+	/*
+	 * Traverse the list of heaps available in this system.  If the
+	 * heap id matches the request of the caller allocate from it.
+	 */
 	down_read(&dev->lock);
 	plist_for_each_entry(heap, &dev->heaps, node) {
-		/* if the caller didn't specify this heap id */
-		if (!((1 << heap->id) & heap_id_mask))
-			continue;
-		buffer = ion_buffer_create(heap, dev, len, flags);
-		if (!IS_ERR(buffer))
+		/* if the caller specified this heap id */
+		if (heap->id == heap_id) {
+			buffer = ion_buffer_create(heap, dev, len, flags);
 			break;
+		}
 	}
 	up_read(&dev->lock);
 
@@ -541,7 +539,7 @@ static long ion_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 		int fd;
 
 		fd = ion_alloc(data.allocation.len,
-			       data.allocation.heap_id_mask,
+			       data.allocation.heap_id,
 			       data.allocation.flags);
 		if (fd < 0)
 			return fd;
diff --git a/drivers/staging/android/uapi/ion.h b/drivers/staging/android/uapi/ion.h
index 5d7009884c13..6a78a1e23251 100644
--- a/drivers/staging/android/uapi/ion.h
+++ b/drivers/staging/android/uapi/ion.h
@@ -35,8 +35,6 @@ enum ion_heap_type {
 			       */
 };
 
-#define ION_NUM_HEAP_IDS		(sizeof(unsigned int) * 8)
-
 /**
  * allocation flags - the lower 16 bits are used by core ion, the upper 16
  * bits are reserved for use by the heaps themselves.
@@ -59,7 +57,7 @@ enum ion_heap_type {
 /**
  * struct ion_allocation_data - metadata passed from userspace for allocations
  * @len:		size of the allocation
- * @heap_id_mask:	mask of heap ids to allocate from
+ * @heap_id:		heap id to allocate from
  * @flags:		flags passed to heap
  * @handle:		pointer that will be populated with a cookie to use to
  *			refer to this allocation
@@ -68,7 +66,7 @@ enum ion_heap_type {
  */
 struct ion_allocation_data {
 	__u64 len;
-	__u32 heap_id_mask;
+	__u32 heap_id;
 	__u32 flags;
 	__u32 fd;
 	__u32 unused;
-- 
2.19.1

Powered by blists - more mailing lists