[PATCH 3/4] mm: cma: Ensure that reservations never cross the low/high mem boundary

Hi Joonsoo,

Thank you for the review.

On Friday 24 October 2014 11:53:25 Joonsoo Kim wrote:

On Thu, Oct 23, 2014 at 05:33:47PM +0300, Laurent Pinchart wrote:

quoted

Commit 95b0e655f914 ("ARM: mm: don't limit default CMA region only to
low memory") extended CMA memory reservation to allow usage of high
memory. It relied on commit f7426b983a6a ("mm: cma: adjust address limit
to avoid hitting low/high memory boundary") to ensure that the reserved
block never crossed the low/high memory boundary. While the
implementation correctly lowered the limit, it failed to consider the
case where the base..limit range crossed the low/high memory boundary
with enough space on each side to reserve the requested size on either
low or high memory.

Rework the base and limit adjustment to fix the problem. The function
now starts by rejecting the reservation altogether for fixed
reservations that cross the boundary, then adjust the limit if
reservation from high memory is impossible, and finally first try to
reserve from high memory first and then falls back to low memory.

Signed-off-by: Laurent Pinchart
[off-list ref]
---

 mm/cma.c | 58 ++++++++++++++++++++++++++++++++++++++++++++--------------
 1 file changed, 44 insertions(+), 14 deletions(-)

diff --git a/mm/cma.c b/mm/cma.c
index 6b14346..b83597b 100644
--- a/mm/cma.c
+++ b/mm/cma.c

@@ -247,23 +247,38 @@ int __init cma_declare_contiguous(phys_addr_t base,
 		return -EINVAL;
 	
 	/*
-	 * adjust limit to avoid crossing low/high memory boundary for
+	 * Adjust limit and base to avoid crossing low/high memory boundary

for
 	 * automatically allocated regions
 	 */

-	if (((limit == 0 || limit > memblock_end) &&
-	     (memblock_end - size < highmem_start &&
-	      memblock_end > highmem_start)) ||
-	    (!fixed && limit > highmem_start && limit - size <
highmem_start)) {
-		limit = highmem_start;
-	}

-	if (fixed && base < highmem_start && base+size > highmem_start) {
+	/*
+	 * If allocating at a fixed base the request region must not cross
the
+	 * low/high memory boundary.
+	 */
+	if (fixed && base < highmem_start && base + size > highmem_start) {
 		ret = -EINVAL;
 		pr_err("Region at %08lx defined on low/high memory boundary
 		(%08lx)\n",
 			(unsigned long)base, (unsigned long)highmem_start);
 		goto err;
 	}

+	/*
+	 * If the limit is unspecified or above the memblock end, its
effective
+	 * value will be the memblock end. Set it explicitly to simplify
further
+	 * checks.
+	 */
+	if (limit == 0 || limit > memblock_end)
+		limit = memblock_end;
+
+	/*
+	 * If the limit is above the highmem start by less than the reserved
+	 * size allocation in highmem won't be possible. Lower the limit to
the
+	 * lowmem end.
+	 */
+	if (limit > highmem_start && limit - size < highmem_start)
+		limit = highmem_start;
+

How about removing this check?
Without this check, memblock_alloc_range would be failed and we can
go fallback correctly. So, this is redundant, IMO.

Good point. I'll remove the check in v2.

-- 
Regards,

Laurent Pinchart