On Sat, Jan 28, 2017 at 12:42:20AM +0900, AKASHI Takahiro wrote:

On Fri, Jan 27, 2017 at 01:59:05PM +0000, James Morse wrote:

quoted

On 24/01/17 08:49, AKASHI Takahiro wrote:

quoted

+	/*
+	 * While crash dump kernel memory is contained in a single memblock
+	 * for now, it should appear in an isolated mapping so that we can
+	 * independently unmap the region later.
+	 */
+	if (crashk_res.end && crashk_res.start >= start &&
+	    crashk_res.end <= end) {
+		if (crashk_res.start != start)
+			__create_pgd_mapping(pgd, start, __phys_to_virt(start),
+					     crashk_res.start - start,
+					     PAGE_KERNEL,
+					     early_pgtable_alloc,
+					     debug_pagealloc_enabled());
+
+		/* before kexec_load(), the region can be read-writable. */
+		__create_pgd_mapping(pgd, crashk_res.start,
+				     __phys_to_virt(crashk_res.start),
+				     crashk_res.end - crashk_res.start + 1,
+				     PAGE_KERNEL, early_pgtable_alloc,
+				     debug_pagealloc_enabled());
+
+		if (crashk_res.end != end)
+			__create_pgd_mapping(pgd, crashk_res.end + 1,
+					     __phys_to_virt(crashk_res.end + 1),
+					     end - crashk_res.end - 1,
+					     PAGE_KERNEL,
+					     early_pgtable_alloc,
+					     debug_pagealloc_enabled());

quoted

+		return;

Doesn't this mean we skip all the 'does this overlap with the kernel text' tests
that happen further down in this file?

You're right. We should still ckeck the overlap against
[start..crashk_res.start] and [crashk_res.end+1..end].

(Using memblock_isolate_range() could simplify the code.)

My key concern here was that we handle both of these in the same way, so
using memblock_isolate_range() for both generally sounds fine to me.

One concern I had with using memblock_isolate_range() is that it does
not guarantee that the region remains isolated. So if there was a
subsequent memblock_add() call, memblock_merge_regions() might end up
merging the region with an adjacent region.

If we isolate the regions at the start of map_mem(), and have a comment
explaining that we must avoid subsequent memblock merging, then I think
this would be ok.

Thanks,
Mark.