Hi Pavel,
On Fri, Oct 13, 2017 at 10:10:09AM -0400, Pavel Tatashin wrote:
> I have a couple concerns about your patch:
>
> One of the reasons (and actually, the main reason) why I preferred to
> keep vmemmap_populate() instead of implementing kasan's own variant,
> which btw can be done in common code similarly to
> vmemmap_populate_basepages() is that vmemmap_populate() uses large
> pages when available. I think it is a considerable downgrade to go
> back to base pages, when we already have large page support available
> to us.
It shouldn't be difficult to use section mappings with my patch, I just
don't really see the need to try to optimise TLB pressure when you're
running with KASAN enabled which already has something like a 3x slowdown
afaik. If it ends up being a big deal, we can always do that later, but
my main aim here is to divorce kasan from vmemmap because they should be
completely unrelated.
> The kasan shadow tree is large, it is up-to 1/8th of system memory, so
> even on moderate size servers, shadow tree is going to be multiple
> gigabytes.
>
> The second concern is that there is an existing bug associated with
> your patch that I am not sure how to solve:
>
> Try building your patch with CONFIG_DEBUG_VM. This config makes
> memblock_virt_alloc_try_nid_raw() to do memset(0xff) on all allocated
> memory.
>
> I am getting the following panic during boot:
>
> [ 0.012637] pid_max: default: 32768 minimum: 301
> [ 0.016037] Security Framework initialized
> [ 0.018389] Dentry cache hash table entries: 16384 (order: 5, 131072 bytes)
> [ 0.019559] Inode-cache hash table entries: 8192 (order: 4, 65536 bytes)
> [ 0.020409] Mount-cache hash table entries: 512 (order: 0, 4096 bytes)
> [ 0.020721] Mountpoint-cache hash table entries: 512 (order: 0, 4096 bytes)
> [ 0.055337] Unable to handle kernel paging request at virtual
> address ffff0400010065af
> [ 0.055422] Mem abort info:
> [ 0.055518] Exception class = DABT (current EL), IL = 32 bits
> [ 0.055579] SET = 0, FnV = 0
> [ 0.055640] EA = 0, S1PTW = 0
> [ 0.055699] Data abort info:
> [ 0.055762] ISV = 0, ISS = 0x00000007
> [ 0.055822] CM = 0, WnR = 0
> [ 0.055966] swapper pgtable: 4k pages, 48-bit VAs, pgd = ffff20000a8f4000
> [ 0.056047] [ffff0400010065af] *pgd=0000000046fe7003,
> *pud=0000000046fe6003, *pmd=0000000046fe5003, *pte=0000000000000000
> [ 0.056436] Internal error: Oops: 96000007 [#1] PREEMPT SMP
> [ 0.056701] Modules linked in:
> [ 0.056939] CPU: 0 PID: 0 Comm: swapper/0 Not tainted
> 4.14.0-rc4_pt_memset12-00096-gfca5985f860e-dirty #16
> [ 0.057001] Hardware name: linux,dummy-virt (DT)
> [ 0.057084] task: ffff2000099d9000 task.stack: ffff2000099c0000
> [ 0.057275] PC is at __asan_load8+0x34/0xb0
> [ 0.057375] LR is at __d_rehash+0xf0/0x240
[...]
> So, I've been trying to root cause it, and here is what I've got:
>
> First, I went back to my version of kasan_map_populate() and replaced
> vmemmap_populate() with vmemmap_populate_basepages(), which
> behavior-vise made it very similar to your patch. After doing this I
> got the same panic. So, I figured there must be something to do with
> the differences that regular vmemmap allocated with granularity of
> SWAPPER_BLOCK_SIZE while kasan with granularity of PAGE_SIZE.
>
> So, I made the following modification to your patch:
>
> static void __init kasan_map_populate(unsigned long start, unsigned long end,
> int node)
> {
> + start = round_down(start, SWAPPER_BLOCK_SIZE);
> + end = round_up(end, SWAPPER_BLOCK_SIZE);
> kasan_pgd_populate(start & PAGE_MASK, PAGE_ALIGN(end), node, false);
> }
>
> This is basically makes shadow tree ranges to be SWAPPER_BLOCK_SIZE
> aligned. After, this modification everything is working. However, I
> am not sure if this is a proper fix.
This certainly doesn't sound right; mapping the shadow with pages shouldn't
lead to problems. I also can't seem to reproduce this myself -- could you
share your full .config and a pointer to the git tree that you're using,
please?
> I feel, this patch requires more work, and I am troubled with using
> base pages instead of large pages.
I'm happy to try fixing this, because I think splitting up kasan and vmemmap
is the right thing to do here.
Will
--
To unsubscribe from this list: send the line "unsubscribe linux-s390" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
