Hello Liu,
I rechecked the current code, then I lost the necessity of
1e93ee75f9d47c219e833210eb31e4a747cc3a8d...
>Sorry, I missed the point.
>
>(1)for flatmem model, kernel get the page struct after minusing ARCH_PFN_OFFSET
> 28 #if defined(CONFIG_FLATMEM)
> 29
> 30 #define __pfn_to_page(pfn) (mem_map + ((pfn) - ARCH_PFN_OFFSET))
> 31 #define __page_to_pfn(page) ((unsigned long)((page) - mem_map) + \
> 32 ARCH_PFN_OFFSET)
>
>So, 1e93ee75f9d47c219e833210eb31e4a747cc3a8d do no harm.
I understand that the base of the kernel's mem_map is ARCH_PFN_OFFSET
and makedumpfile gets the its address as:
dump_mem_map(0, info->max_mapnr, mem_map, 0);
^^^
doesn't consider the offset
so makedumpfile's mem_map[] has the offset at this point.
The filtering process(exclude_unnecessary_pages()) doesn't consider the offset
while the writing process(write_kdump_pages()) considers it.
This thing will make the gap between the page descriptor and the page itself.
>(2)for discontigmem model
>
> 18 #ifndef arch_local_page_offset
> 19 #define arch_local_page_offset(pfn, nid) \
> 20 ((pfn) - NODE_DATA(nid)->node_start_pfn)
> 21 #endif
> 22
> 23 #endif /* CONFIG_DISCONTIGMEM */
>
>.....
>
> 33 #elif defined(CONFIG_DISCONTIGMEM)
> 34
> 35 #define __pfn_to_page(pfn) \
> 36 ({ unsigned long __pfn = (pfn); \
> 37 unsigned long __nid = arch_pfn_to_nid(__pfn); \
> 38 NODE_DATA(__nid)->node_mem_map + arch_local_page_offset(__pfn, __nid);\
> 39 })
>
>
>The kernel minuses "NODE_DATA(nid)->node_start_pfn". So relative postion is got
>for the tranfser.
Hmm, I think the process above just means each NODE_DATA(nid)->node_mem_map
corresponds to its own region which starts at NODE_DATA(nid)->node_start_pfn.
It looks kernel's mem_map doesn't have the offest and makedumpfile get it
as it is, so makedumpfile's mem_map[] has no offset.
>So to get page struct
>
>kernel : paddr --> pfn -[consider ARCH_PFN_OFFSET ] -> page
>makedumpfile : paddr -[consider ARCH_PFN_OFFSET] -> pfn --> page
Why does ARCH_PFN_OFFSET appear here ? I couldn't find the code.
Anyway, the gap between the page descriptor and the page itself will appear
like flatmem.
>
>So I think commit 1e93ee75f9d47c219e833210eb31e4a747cc3a8d can fit discontigmem and flatmem.
>After applying this patch, it also fits sparsemem model. What do you think?
If my understanding is correct, I think we should fix get_mm_flatmem() instead of
1e93ee75f9d47c219e833210e and your patch like below:
...
dump_mem_map(0, ARCH_PFN_OFFSET, NOT_MEMMAP_ADDR, 0);
dump_mem_map(ARCH_PFN_OFFSET, info->max_mapnr, mem_map, 1);
...
"paddr:0x0 == pfn:0" is easy to understand.
Thanks
Atsushi Kumagai
>
>Thanks,
>Liu Hua
>
>>
>> Actually, the similar gap exists in the sparse_mem case as you described,
>> so I suspect we have to take care of it also for other memory models.
>>
>>>> }
>>>>
>>>> So there is no problem in this model since the top of mem_map corresponds to
>>>> ARCH_PFN_OFFSET, right?
>>>
>>> I don't think so. Is it clear for my words above?
>>>
>>>>
>>>>> (2) For discontigmem, it manages the mem_map with node_memblk. commit
>>>>> 1e93ee75f9d47c21 also does no harm.
>>>>
>>>> alloc_node_mem_map() allocates mem_map also for discontigmem, but I can't find
>>>> any codes to consider ARCH_PFN_OFFSET for this model.
>>>> So I suspect the mismatch between the pfn for makedumpfile and the actual content
>>>> of mem_map can exist. Could you explain why this case is OK in more detail?
>>>>
>>> Actually I did not test this memory model. I reach my conclusion via the codes.
>>>
>>> get_mm_discontigmem
>>> {
>>> ....
>>> for (i = 0; i < vt.numnodes; i++) { //loop for every node
>>> 2591 if (!readmem(VADDR, pgdat + OFFSET(pglist_data.node_start_pfn),
>>> 2592 &pfn_start, sizeof pfn_start)) { //get pfn_start for this node
>>> ....
>>> 2596 if (!readmem(VADDR,pgdat+OFFSET(pglist_data.node_spanned_pages),
>>> 2597 &node_spanned_pages, sizeof node_spanned_pages)) { //get the number of pages in this node
>>>
>>> 2603 if (SYMBOL(vmem_map) == NOT_FOUND_SYMBOL) {
>>> 2604 if (!readmem(VADDR, pgdat + OFFSET(pglist_data.node_mem_map), //get the mem_map for this
>>> node.
>>> 2605 &mem_map, sizeof mem_map)) {
>>> 2606 ERRMSG("Can't get mem_map.\n");
>>> 2607 return FALSE;
>>> 2608 }
>>> 2609 } else
>>> 2610 mem_map = vmem_map + (SIZE(page) * pfn_start);
>>> ....
>>> }
>>>
>>> So I think for discontigmem, makedumpfile can get the start address and length of mem_map from vmcore directly.
>>> And Everything can go well without ARCH_PFN_OFFSET.
>>
>> The same can be said, is it not needed to consider ARCH_PFN_OFFSET
>> to get a page struct from the mem_map?
>>
>>
>> Thanks
>> Atsushi Kumagai
>>
>>>
>>> Perhaps I need some tests on discontigmem. Did I explan my idea clearly?
>>>
>>>
>>>
>>>>
>>>> Thanks
>>>> Atsushi Kumagai
>>>>
>>>>> What do you think?
>>>>>
>>>>> Thanks,
>>>>> Liu Hua
