9:41 p.m.
Add get_value_vmcore() to get the symbol value for @name.
Also add macro GET_SYM to simplify the code.
Signed-off-by: Huang Shijie <shijie(a)os.amperecomputing.com>
---
defs.h | 1 +
kernel.c | 85 +++++++++++++++++++-------------------------------------
2 files changed, 30 insertions(+), 56 deletions(-)
diff --git a/defs.h b/defs.h
index 1fe2d0b..eec7b3e 100644
--- a/defs.h
+++ b/defs.h
@@ -6055,6 +6055,7 @@ int hide_offline_cpu(int);
int get_highest_cpu_online(void);
int get_highest_cpu_present(void);
int get_cpus_to_display(void);
+bool get_value_vmcore(const char *name, ulong *v);
void get_log_from_vmcoreinfo(char *file);
int in_cpu_map(int, int);
void paravirt_init(void);
diff --git a/kernel.c b/kernel.c
index 6dcf414..caf6149 100644
--- a/kernel.c
+++ b/kernel.c
@@ -104,6 +104,20 @@ static void check_vmcoreinfo(void);
static int is_pvops_xen(void);
static int get_linux_banner_from_vmlinux(char *, size_t);
+/* Return TRUE if we succeed, return FALSE on failure. */
+bool
+get_value_vmcore(const char *name, ulong *v)
+{
+ char *string = pc->read_vmcoreinfo(name);
+
+ if (!string)
+ return FALSE;
+
+ *v = htol(string, RETURN_ON_ERROR, NULL);
+ free(string);
+ return TRUE;
+}
+
/*
* popuplate the global kernel table (kt) with kernel version
* information parsed from UTSNAME/OSRELEASE string
@@ -10984,6 +10998,12 @@ hypervisor_init(void)
fprintf(fp, "hypervisor: %s\n", kt->hypervisor);
}
+#define GET_SYM(s,v) \
+ if (get_value_vmcore((s), &(v))) { \
+ if (CRASHDEBUG(1)) \
+ fprintf(fp, s ": %lx\n", v); \
+ }
+
/*
* Get and display the kernel log buffer using the vmcoreinfo
* data alone without the vmlinux file.
@@ -11024,62 +11044,15 @@ get_log_from_vmcoreinfo(char *file)
} else
error(FATAL, "VMCOREINFO: cannot determine page size\n");
- if ((string = pc->read_vmcoreinfo("SYMBOL(log_buf)"))) {
- vmc->log_buf_SYMBOL = htol(string, RETURN_ON_ERROR, NULL);
- if (CRASHDEBUG(1))
- fprintf(fp, "SYMBOL(log_buf): %lx\n",
- vmc->log_buf_SYMBOL);
- free(string);
- }
- if ((string = pc->read_vmcoreinfo("SYMBOL(log_end)"))) {
- vmc->log_end_SYMBOL = htol(string, RETURN_ON_ERROR, NULL);
- if (CRASHDEBUG(1))
- fprintf(fp, "SYMBOL(log_end): %lx\n",
- vmc->log_end_SYMBOL);
- free(string);
- }
- if ((string = pc->read_vmcoreinfo("SYMBOL(log_buf_len)"))) {
- vmc->log_buf_len_SYMBOL = htol(string, RETURN_ON_ERROR, NULL);
- if (CRASHDEBUG(1))
- fprintf(fp, "SYMBOL(log_buf_len): %lx\n",
- vmc->log_buf_len_SYMBOL);
- free(string);
- }
- if ((string = pc->read_vmcoreinfo("SYMBOL(logged_chars)"))) {
- vmc->logged_chars_SYMBOL = htol(string, RETURN_ON_ERROR, NULL);
- if (CRASHDEBUG(1))
- fprintf(fp, "SYMBOL(logged_chars): %lx\n",
- vmc->logged_chars_SYMBOL);
- free(string);
- }
- if ((string = pc->read_vmcoreinfo("SYMBOL(log_first_idx)"))) {
- vmc->log_first_idx_SYMBOL = htol(string, RETURN_ON_ERROR, NULL);
- if (CRASHDEBUG(1))
- fprintf(fp, "SYMBOL(log_first_idx): %lx\n",
- vmc->log_first_idx_SYMBOL);
- free(string);
- }
- if ((string = pc->read_vmcoreinfo("SYMBOL(log_next_idx)"))) {
- vmc->log_next_idx_SYMBOL = htol(string, RETURN_ON_ERROR, NULL);
- if (CRASHDEBUG(1))
- fprintf(fp, "SYMBOL(log_next_idx): %lx\n",
- vmc->log_next_idx_SYMBOL);
- free(string);
- }
- if ((string = pc->read_vmcoreinfo("SYMBOL(phys_base)"))) {
- vmc->phys_base_SYMBOL = htol(string, RETURN_ON_ERROR, NULL);
- if (CRASHDEBUG(1))
- fprintf(fp, "SYMBOL(phys_base): %lx\n",
- vmc->phys_base_SYMBOL);
- free(string);
- }
- if ((string = pc->read_vmcoreinfo("SYMBOL(_stext)"))) {
- vmc->_stext_SYMBOL = htol(string, RETURN_ON_ERROR, NULL);
- if (CRASHDEBUG(1))
- fprintf(fp, "SYMBOL(_stext): %lx\n",
- vmc->_stext_SYMBOL);
- free(string);
- }
+ GET_SYM("SYMBOL(log_buf)", vmc->log_buf_SYMBOL);
+ GET_SYM("SYMBOL(log_end)", vmc->log_end_SYMBOL);
+ GET_SYM("SYMBOL(log_buf_len)", vmc->log_buf_len_SYMBOL);
+ GET_SYM("SYMBOL(logged_chars)", vmc->logged_chars_SYMBOL);
+ GET_SYM("SYMBOL(log_first_idx)", vmc->log_first_idx_SYMBOL);
+ GET_SYM("SYMBOL(log_next_idx)", vmc->log_next_idx_SYMBOL);
+ GET_SYM("SYMBOL(phys_base)", vmc->phys_base_SYMBOL);
+ GET_SYM("SYMBOL(_stext)", vmc->_stext_SYMBOL);
+
if ((string = pc->read_vmcoreinfo("OFFSET(log.ts_nsec)"))) {
vmc->log_ts_nsec_OFFSET = dtol(string, RETURN_ON_ERROR, NULL);
if (CRASHDEBUG(1))
--
2.40.1
9:41 p.m.
New subject: [PATCH 2/2] Add vmemmap support
If the kernel exports the vmmemap then we can use that symbol in
crash to optimize access. vmmemap is just an array of page structs
after all.
This patch tries to get the "vmemmap" from the vmcore file.
If we can use the "vmemmap", use the vmemmap_is_page_ptr to replace
the generic_is_page_ptr().
If we have vmemmap then we can implement fast page_to_pfn code in
vmemmap_is_page_ptr
Test result:
Test the patch: "[PATCH v3] add "files -n" command for an inode"
https://lists.crash-utility.osci.io/archives/list/
devel(a)lists.crash-utility.osci.io/thread/CRLOQP534YKEQPTCXIWVDYK4NA7BOSUK/
Without the this patch:
#files -n xxx (xxx is the inode of vmlinux which is 441M)
This costed about 162 seconds.
With the this patch:
#files -n xxx (xxx is the inode of vmlinux which is 441M)
This costed less then 1 second.
Signed-off-by: Huang Shijie <shijie(a)os.amperecomputing.com>
---
defs.h | 3 +++
memory.c | 35 +++++++++++++++++++++++++++++++++++
2 files changed, 38 insertions(+)
diff --git a/defs.h b/defs.h
index eec7b3e..25bb455 100644
--- a/defs.h
+++ b/defs.h
@@ -2615,6 +2615,7 @@ struct vm_table { /* kernel VM-related data */
ulong vma_cache_fills;
void *mem_sec;
char *mem_section;
+ ulong vmemmap;
int ZONE_HIGHMEM;
ulong *node_online_map;
int node_online_map_len;
@@ -2665,10 +2666,12 @@ struct vm_table { /* kernel VM-related data */
#define SLAB_OVERLOAD_PAGE (0x8000000)
#define SLAB_CPU_CACHE (0x10000000)
#define SLAB_ROOT_CACHES (0x20000000)
+#define SPARSEMEM_VMEMMAP (0x40000000)
#define IS_FLATMEM() (vt->flags & FLATMEM)
#define IS_DISCONTIGMEM() (vt->flags & DISCONTIGMEM)
#define IS_SPARSEMEM() (vt->flags & SPARSEMEM)
+#define IS_SPARSEMEM_VMEMMAP() (vt->flags & SPARSEMEM_VMEMMAP)
#define IS_SPARSEMEM_EX() (vt->flags & SPARSEMEM_EX)
#define COMMON_VADDR_SPACE() (vt->flags & COMMON_VADDR)
diff --git a/memory.c b/memory.c
index ed1a4fb..668567f 100644
--- a/memory.c
+++ b/memory.c
@@ -17462,6 +17462,39 @@ sparse_decode_mem_map(ulong coded_mem_map, ulong section_nr)
(section_nr_to_pfn(section_nr) * SIZE(page));
}
+static int
+vmemmap_is_page_ptr(ulong addr, physaddr_t *phys)
+{
+ ulong pfn;
+
+ if (IS_SPARSEMEM_VMEMMAP() &&
+ (VMEMMAP_VADDR <= addr && addr < VMEMMAP_END)) {
+ ulong size = SIZE(page);
+
+ /* Not aligned with the page structure */
+ if ((addr - vt->vmemmap) % size)
+ return FALSE;
+
+ pfn = (addr - vt->vmemmap) / size;
+ if (phys)
+ *phys = PTOB(pfn);
+ return TRUE;
+ }
+ return FALSE;
+}
+
+static bool
+check_sparsemem_vmemmap(void)
+{
+ if (!get_value_vmcore("SYMBOL(vmemmap)", &vt->vmemmap))
+ return FALSE;
+
+ vt->flags |= SPARSEMEM_VMEMMAP;
+ if (machdep->is_page_ptr == generic_is_page_ptr)
+ machdep->is_page_ptr = vmemmap_is_page_ptr;
+ return TRUE;
+}
+
void
sparse_mem_init(void)
{
@@ -17472,6 +17505,8 @@ sparse_mem_init(void)
if (!IS_SPARSEMEM())
return;
+ check_sparsemem_vmemmap();
+
MEMBER_OFFSET_INIT(mem_section_section_mem_map, "mem_section",
"section_mem_map");
--
2.40.1
1:21 a.m.
New subject: [PATCH 2/2] Add vmemmap support
thanks for the patch.
On 2023/11/28 12:41, Huang Shijie wrote:
If the kernel exports the vmmemap then we can use that symbol in
crash to optimize access. vmmemap is just an array of page structs
after all.
This patch tries to get the "vmemmap" from the vmcore file.
If we can use the "vmemmap", use the vmemmap_is_page_ptr to replace
the generic_is_page_ptr().
I recalled that I also did a similar thing for x86_64 [1] in the past..
so it's not so slow on x86_64 and it has valid section check too.
[1]
https://github.com/crash-utility/crash/commit/4141373d9de3fea29f5d2b58f60...
I don't object to add an entry to vmcoreinfo as a way of getting the
vmemmap value. Seems like arm64 has another value from VMEMMAP_START.
but there are some concerns in crash:
- machdep->flags already has VMEMMAP flag. It's used like "if
(IS_SPARSEMEM() && (machdep->flags & VMEMMAP)" e.g. [1]. Adding the
new
flag is very confusing and inconsistent.
- Do all architectures have VMEMMAP_VADDR/END values in crash with
CONFIG_SPARSEMEM_VMEMMAP configured?
we cannot test all archs, so this kind of arch-independent change makes
me nervous. I would like to suggest an arch-specific use first. e.g.
how about making x86_64_is_page_ptr more generic and use it?
What happens if you use it on arm64? maybe the valid section can check
it with VMEMMAP_VADDR without the vmemmap value. Just an idea though.
Thanks,
Kazu
>
> If we have vmemmap then we can implement fast page_to_pfn code in
> vmemmap_is_page_ptr
>
> Test result:
> Test the patch: "[PATCH v3] add "files -n" command for an
inode"
> https://lists.crash-utility.osci.io/archives/list/
> devel(a)lists.crash-utility.osci.io/thread/CRLOQP534YKEQPTCXIWVDYK4NA7BOSUK/
>
> Without the this patch:
> #files -n xxx (xxx is the inode of vmlinux which is 441M)
> This costed about 162 seconds.
>
> With the this patch:
> #files -n xxx (xxx is the inode of vmlinux which is 441M)
> This costed less then 1 second.
>
> Signed-off-by: Huang Shijie <shijie(a)os.amperecomputing.com>
> ---
> defs.h | 3 +++
> memory.c | 35 +++++++++++++++++++++++++++++++++++
> 2 files changed, 38 insertions(+)
>
> diff --git a/defs.h b/defs.h
> index eec7b3e..25bb455 100644
> --- a/defs.h
> +++ b/defs.h
> @@ -2615,6 +2615,7 @@ struct vm_table { /* kernel VM-related data */
> ulong vma_cache_fills;
> void *mem_sec;
> char *mem_section;
> + ulong vmemmap;
> int ZONE_HIGHMEM;
> ulong *node_online_map;
> int node_online_map_len;
> @@ -2665,10 +2666,12 @@ struct vm_table { /* kernel VM-related data
*/
> #define SLAB_OVERLOAD_PAGE (0x8000000)
> #define SLAB_CPU_CACHE (0x10000000)
> #define SLAB_ROOT_CACHES (0x20000000)
> +#define SPARSEMEM_VMEMMAP (0x40000000)
>
> #define IS_FLATMEM() (vt->flags & FLATMEM)
> #define IS_DISCONTIGMEM() (vt->flags & DISCONTIGMEM)
> #define IS_SPARSEMEM() (vt->flags & SPARSEMEM)
> +#define IS_SPARSEMEM_VMEMMAP() (vt->flags & SPARSEMEM_VMEMMAP)
> #define IS_SPARSEMEM_EX() (vt->flags & SPARSEMEM_EX)
>
> #define COMMON_VADDR_SPACE() (vt->flags & COMMON_VADDR)
> diff --git a/memory.c b/memory.c
> index ed1a4fb..668567f 100644
> --- a/memory.c
> +++ b/memory.c
> @@ -17462,6 +17462,39 @@ sparse_decode_mem_map(ulong coded_mem_map, ulong
section_nr)
> (section_nr_to_pfn(section_nr) * SIZE(page));
> }
>
> +static int
> +vmemmap_is_page_ptr(ulong addr, physaddr_t *phys)
> +{
> + ulong pfn;
> +
> + if (IS_SPARSEMEM_VMEMMAP() &&
> + (VMEMMAP_VADDR <= addr && addr < VMEMMAP_END)) {
> + ulong size = SIZE(page);
> +
> + /* Not aligned with the page structure */
> + if ((addr - vt->vmemmap) % size)
> + return FALSE;
> +
> + pfn = (addr - vt->vmemmap) / size;
> + if (phys)
> + *phys = PTOB(pfn);
> + return TRUE;
> + }
> + return FALSE;
> +}
> +
> +static bool
> +check_sparsemem_vmemmap(void)
> +{
> + if (!get_value_vmcore("SYMBOL(vmemmap)", &vt->vmemmap))
> + return FALSE;
> +
> + vt->flags |= SPARSEMEM_VMEMMAP;
> + if (machdep->is_page_ptr == generic_is_page_ptr)
> + machdep->is_page_ptr = vmemmap_is_page_ptr;
> + return TRUE;
> +}
> +
> void
> sparse_mem_init(void)
> {
> @@ -17472,6 +17505,8 @@ sparse_mem_init(void)
> if (!IS_SPARSEMEM())
> return;
>
> + check_sparsemem_vmemmap();
> +
> MEMBER_OFFSET_INIT(mem_section_section_mem_map, "mem_section",
> "section_mem_map");
>
1:29 a.m.
New subject: [PATCH 2/2] Add vmemmap support
On 2023/11/30 16:21, HAGIO KAZUHITO(萩尾 一仁) wrote:
thanks for the patch.
On 2023/11/28 12:41, Huang Shijie wrote:
> If the kernel exports the vmmemap then we can use that symbol in
> crash to optimize access. vmmemap is just an array of page structs
> after all.
>
> This patch tries to get the "vmemmap" from the vmcore file.
> If we can use the "vmemmap", use the vmemmap_is_page_ptr to replace
> the generic_is_page_ptr().
I recalled that I also did a similar thing for x86_64 [1] in the past..
so it's not so slow on x86_64 and it has valid section check too.
[1]
https://github.com/crash-utility/crash/commit/4141373d9de3fea29f5d2b58f60...
I don't object to add an entry to vmcoreinfo as a way of getting the
vmemmap value. Seems like arm64 has another value from VMEMMAP_START.
but there are some concerns in crash:
- machdep->flags already has VMEMMAP flag. It's used like "if
(IS_SPARSEMEM() && (machdep->flags & VMEMMAP)" e.g. [1]. Adding the
new
flag is very confusing and inconsistent.
- Do all architectures have VMEMMAP_VADDR/END values in crash with
CONFIG_SPARSEMEM_VMEMMAP configured?
we cannot test all archs, so this kind of arch-independent change makes
me nervous. I would like to suggest an arch-specific use first. e.g.
how about making x86_64_is_page_ptr more generic and use it?
What happens if you use it on arm64? maybe the valid section can
check
it with VMEMMAP_VADDR without the vmemmap value. Just an idea though.
hmm sorry, probably this does not work, I missed this.
+ pfn = (addr - vt->vmemmap) / size;
but can VMEMMAP_VADDR and phys_offset can be used to get vmemmap value
on arm64?
Thanks,
Kazu
>
> Thanks,
> Kazu
>
>>
>> If we have vmemmap then we can implement fast page_to_pfn code in
>> vmemmap_is_page_ptr
>>
>> Test result:
>> Test the patch: "[PATCH v3] add "files -n" command for an
inode"
>> https://lists.crash-utility.osci.io/archives/list/
>> devel(a)lists.crash-utility.osci.io/thread/CRLOQP534YKEQPTCXIWVDYK4NA7BOSUK/
>>
>> Without the this patch:
>> #files -n xxx (xxx is the inode of vmlinux which is 441M)
>> This costed about 162 seconds.
>>
>> With the this patch:
>> #files -n xxx (xxx is the inode of vmlinux which is 441M)
>> This costed less then 1 second.
>>
>> Signed-off-by: Huang Shijie <shijie(a)os.amperecomputing.com>
>> ---
>> defs.h | 3 +++
>> memory.c | 35 +++++++++++++++++++++++++++++++++++
>> 2 files changed, 38 insertions(+)
>>
>> diff --git a/defs.h b/defs.h
>> index eec7b3e..25bb455 100644
>> --- a/defs.h
>> +++ b/defs.h
>> @@ -2615,6 +2615,7 @@ struct vm_table { /* kernel VM-related data
*/
>> ulong vma_cache_fills;
>> void *mem_sec;
>> char *mem_section;
>> + ulong vmemmap;
>> int ZONE_HIGHMEM;
>> ulong *node_online_map;
>> int node_online_map_len;
>> @@ -2665,10 +2666,12 @@ struct vm_table { /* kernel VM-related
data */
>> #define SLAB_OVERLOAD_PAGE (0x8000000)
>> #define SLAB_CPU_CACHE (0x10000000)
>> #define SLAB_ROOT_CACHES (0x20000000)
>> +#define SPARSEMEM_VMEMMAP (0x40000000)
>>
>> #define IS_FLATMEM() (vt->flags & FLATMEM)
>> #define IS_DISCONTIGMEM() (vt->flags & DISCONTIGMEM)
>> #define IS_SPARSEMEM() (vt->flags & SPARSEMEM)
>> +#define IS_SPARSEMEM_VMEMMAP() (vt->flags & SPARSEMEM_VMEMMAP)
>> #define IS_SPARSEMEM_EX() (vt->flags & SPARSEMEM_EX)
>>
>> #define COMMON_VADDR_SPACE() (vt->flags & COMMON_VADDR)
>> diff --git a/memory.c b/memory.c
>> index ed1a4fb..668567f 100644
>> --- a/memory.c
>> +++ b/memory.c
>> @@ -17462,6 +17462,39 @@ sparse_decode_mem_map(ulong coded_mem_map, ulong
section_nr)
>> (section_nr_to_pfn(section_nr) * SIZE(page));
>> }
>>
>> +static int
>> +vmemmap_is_page_ptr(ulong addr, physaddr_t *phys)
>> +{
>> + ulong pfn;
>> +
>> + if (IS_SPARSEMEM_VMEMMAP() &&
>> + (VMEMMAP_VADDR <= addr && addr < VMEMMAP_END)) {
>> + ulong size = SIZE(page);
>> +
>> + /* Not aligned with the page structure */
>> + if ((addr - vt->vmemmap) % size)
>> + return FALSE;
>> +
>> + pfn = (addr - vt->vmemmap) / size;
>> + if (phys)
>> + *phys = PTOB(pfn);
>> + return TRUE;
>> + }
>> + return FALSE;
>> +}
>> +
>> +static bool
>> +check_sparsemem_vmemmap(void)
>> +{
>> + if (!get_value_vmcore("SYMBOL(vmemmap)", &vt->vmemmap))
>> + return FALSE;
>> +
>> + vt->flags |= SPARSEMEM_VMEMMAP;
>> + if (machdep->is_page_ptr == generic_is_page_ptr)
>> + machdep->is_page_ptr = vmemmap_is_page_ptr;
>> + return TRUE;
>> +}
>> +
>> void
>> sparse_mem_init(void)
>> {
>> @@ -17472,6 +17505,8 @@ sparse_mem_init(void)
>> if (!IS_SPARSEMEM())
>> return;
>>
>> + check_sparsemem_vmemmap();
>> +
>> MEMBER_OFFSET_INIT(mem_section_section_mem_map, "mem_section",
>> "section_mem_map");
>>
> --
> Crash-utility mailing list -- devel(a)lists.crash-utility.osci.io
> To unsubscribe send an email to devel-leave(a)lists.crash-utility.osci.io
> https://${domain_name}/admin/lists/devel.lists.crash-utility.osci.io/
> Contribution Guidelines: https://github.com/crash-utility/crash/wiki
2:26 a.m.
New subject: [PATCH 2/2] Add vmemmap support
Hi Kazu,
在 2023/11/30 15:29, HAGIO KAZUHITO(萩尾 一仁) 写道:
On 2023/11/30 16:21, HAGIO KAZUHITO(萩尾 一仁) wrote:
> thanks for the patch.
>
> On 2023/11/28 12:41, Huang Shijie wrote:
>> If the kernel exports the vmmemap then we can use that symbol in
>> crash to optimize access. vmmemap is just an array of page structs
>> after all.
>>
>> This patch tries to get the "vmemmap" from the vmcore file.
>> If we can use the "vmemmap", use the vmemmap_is_page_ptr to replace
>> the generic_is_page_ptr().
> I recalled that I also did a similar thing for x86_64 [1] in the past..
> so it's not so slow on x86_64 and it has valid section check too.
Maybe
your vmlinux is not so big as mine(441M)...
> [1]
>
https://github.com/crash-utility/crash/commit/4141373d9de3fea29f5d2b58f60...
>
> I don't object to add an entry to vmcoreinfo as a way of getting the
> vmemmap value. Seems like arm64 has another value from VMEMMAP_START.
>
> but there are some concerns in crash:
>
> - machdep->flags already has VMEMMAP flag. It's used like "if
> (IS_SPARSEMEM() && (machdep->flags & VMEMMAP)" e.g. [1]. Adding
the new
> flag is very confusing and inconsistent.
>
> - Do all architectures have VMEMMAP_VADDR/END values in crash with
> CONFIG_SPARSEMEM_VMEMMAP configured?
yes. your concern are right. I found some arch does not support the
VMEMMAP_VADDR/END.
So it is better to add this into ARM64 first. (I use the arm64 machine
all the time..)
>
> we cannot test all archs, so this kind of arch-independent change makes
> me nervous. I would like to suggest an arch-specific use first. e.g.
> how about making x86_64_is_page_ptr more generic and use it?
>
I will add arm64 version code. I am not familar with x86, and I do not
have x86 machine for the testing.
so it's better to not change it.
> What happens if you use it on arm64? maybe the valid section can
check
> it with VMEMMAP_VADDR without the vmemmap value. Just an idea though.
hmm sorry, probably this does not work, I missed this.
+ pfn = (addr - vt->vmemmap) / size;
The code is converted by the linux code:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/i...
#define __page_to_pfn(page) (unsigned long)((page) - vmemmap)
So it's correct.
but can VMEMMAP_VADDR and phys_offset can be used to get vmemmap value
on arm64?
On arm64, I can get vmemmap by parsing "memstart_addr".
But I cannot know if CONFIG_SPARSEMEM_VMEMMAP is enabled.
So if the kernel can export "vmemmap", everything become easy.
All the archs supporting the VMEMMAP can benifit from it, such as arm64,
ppc,loongarch,riscv..
Thanks
Huang Shijie
7:11 p.m.
New subject: [PATCH 2/2] Add vmemmap support
On Thu, 30 Nov 2023, Shijie Huang wrote:
> but can VMEMMAP_VADDR and phys_offset can be used to get vmemmap
value
> on arm64?
On arm64, I can get vmemmap by parsing "memstart_addr".
But I cannot know if CONFIG_SPARSEMEM_VMEMMAP is enabled.
You can get to the struct page pointer through the VMEMMAP and the frame
number. So you would need to get the pfn from the phys_offset.
memmory_model.h:
/* memmap is virtually contiguous. */
#define __pfn_to_page(pfn) (vmemmap + (pfn))
#define __page_to_pfn(page) (unsigned long)((page) - vmemmap)
/*
* Convert a physical address to a Page Frame Number and back
*/
#define __phys_to_pfn(paddr) PHYS_PFN(paddr)
#define __pfn_to_phys(pfn) PFN_PHYS(pfn)
So if the kernel can export "vmemmap", everything become easy.
All the archs supporting the VMEMMAP can benifit from it, such as arm64,
ppc,loongarch,riscv..
And x86. CONFIG_SPARSEMEM_VMEMMAP was developed for x86 initially.
2:08 a.m.
New subject: [PATCH 2/2] Add vmemmap support
On 2023/11/30 17:26, Shijie Huang wrote:
Hi Kazu,
在 2023/11/30 15:29, HAGIO KAZUHITO(萩尾 一仁) 写道:
> On 2023/11/30 16:21, HAGIO KAZUHITO(萩尾 一仁) wrote:
>> thanks for the patch.
>>
>> On 2023/11/28 12:41, Huang Shijie wrote:
>>> If the kernel exports the vmmemap then we can use that symbol in
>>> crash to optimize access. vmmemap is just an array of page structs
>>> after all.
>>>
>>> This patch tries to get the "vmemmap" from the vmcore file.
>>> If we can use the "vmemmap", use the vmemmap_is_page_ptr to
replace
>>> the generic_is_page_ptr().
>> I recalled that I also did a similar thing for x86_64 [1] in the past..
>> so it's not so slow on x86_64 and it has valid section check too.
Maybe your vmlinux is not so big as mine(441M)...
It's not so small :)
crash> files | grep -m1 vmlinux
5 ffff97c21fa2cf00 ffff97c2400e5200 ffff97c0f5cf5ec8 REG
/usr/lib/debug/usr/lib/modules/4.18.0-305.el8.x86_64/vmlinux
crash> !ls -lh /usr/lib/debug/usr/lib/modules/4.18.0-305.el8.x86_64/vmlinux
-rwxr-xr-x. 3 root root 826M Apr 29 2021
/usr/lib/debug/usr/lib/modules/4.18.0-305.el8.x86_64/vmlinux
crash> files -n ffff97c0f5cf5ec8 | ts.py
16:42:30.902 START
16:42:30.902 INODE NRPAGES
16:42:30.902 ffff97c0f5cf5ec8 211283
16:42:30.902
16:42:31.106 NODE PAGES
16:42:31.106 0 211283
16:42:31.106 END (ELAPS 0:00:00.204)
(This crash has your "files -n" v2 patch and is_page_ptr() check at the
beginning of summary_inode_page)
>> [1]
>>
https://github.com/crash-utility/crash/commit/4141373d9de3fea29f5d2b58f60...
>>
>> I don't object to add an entry to vmcoreinfo as a way of getting the
>> vmemmap value. Seems like arm64 has another value from VMEMMAP_START.
>>
>> but there are some concerns in crash:
>>
>> - machdep->flags already has VMEMMAP flag. It's used like "if
>> (IS_SPARSEMEM() && (machdep->flags & VMEMMAP)" e.g. [1].
Adding the new
>> flag is very confusing and inconsistent.
>>
>> - Do all architectures have VMEMMAP_VADDR/END values in crash with
>> CONFIG_SPARSEMEM_VMEMMAP configured?
yes. your concern are right. I found some arch does not support the
VMEMMAP_VADDR/END.
So it is better to add this into ARM64 first. (I use the arm64 machine
all the time..)
>>
>> we cannot test all archs, so this kind of arch-independent change makes
>> me nervous. I would like to suggest an arch-specific use first. e.g.
>> how about making x86_64_is_page_ptr more generic and use it?
>>
I will add arm64 version code. I am not familar with x86, and I do not
have x86 machine for the testing.
so it's better to not change it.
OK, I see.
>> What happens if you use it on arm64? maybe the valid section can check
>> it with VMEMMAP_VADDR without the vmemmap value. Just an idea though.
> hmm sorry, probably this does not work, I missed this.
>
> + pfn = (addr - vt->vmemmap) / size;
The code is converted by the linux code:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/i...
#define __page_to_pfn(page) (unsigned long)((page) - vmemmap)
So it's correct.
>
> but can VMEMMAP_VADDR and phys_offset can be used to get vmemmap value
> on arm64?
On arm64, I can get vmemmap by parsing "memstart_addr".
But I cannot know if CONFIG_SPARSEMEM_VMEMMAP is enabled.
on x86_64, vmemmap_populate is used:
if (symbol_exists("vmemmap_populate"))
machdep->flags |= VMEMMAP;
on arm64, it's set unconditionally, is it ok?
arm64_init(int when)
{
...
case PRE_GDB:
...
machdep->stacksize = ARM64_STACK_SIZE;
machdep->flags |= VMEMMAP;
So if the kernel can export "vmemmap", everything become easy.
All the archs supporting the VMEMMAP can benifit from it, such as arm64,
ppc,loongarch,riscv..
Yes but we're not sure whether the users of the other archs have the
same issue (e.g. x86_64 is fast enough) and it's hard to test all archs.
Also, crash cannot always access vmcoreinfo (e.g. ramdumps), basically
it's better to implement without depending on vmcoreinfo first if
possible. If there is a case where crash cannot work without
vmcoreinfo, then we will add a entry though.
Thanks,
Kazu
3:56 a.m.
New subject: [PATCH 2/2] Add vmemmap support
在 2023/12/1 16:08, HAGIO KAZUHITO(萩尾 一仁) 写道:
On 2023/11/30 17:26, Shijie Huang wrote:
> Hi Kazu,
>
> 在 2023/11/30 15:29, HAGIO KAZUHITO(萩尾 一仁) 写道:
>> On 2023/11/30 16:21, HAGIO KAZUHITO(萩尾 一仁) wrote:
>>> thanks for the patch.
>>>
>>> On 2023/11/28 12:41, Huang Shijie wrote:
>>>> If the kernel exports the vmmemap then we can use that symbol in
>>>> crash to optimize access. vmmemap is just an array of page structs
>>>> after all.
>>>>
>>>> This patch tries to get the "vmemmap" from the vmcore file.
>>>> If we can use the "vmemmap", use the vmemmap_is_page_ptr to
replace
>>>> the generic_is_page_ptr().
>>> I recalled that I also did a similar thing for x86_64 [1] in the past..
>>> so it's not so slow on x86_64 and it has valid section check too.
> Maybe your vmlinux is not so big as mine(441M)...
It's not so small :)
crash> files | grep -m1 vmlinux
5 ffff97c21fa2cf00 ffff97c2400e5200 ffff97c0f5cf5ec8 REG
/usr/lib/debug/usr/lib/modules/4.18.0-305.el8.x86_64/vmlinux
crash> !ls -lh /usr/lib/debug/usr/lib/modules/4.18.0-305.el8.x86_64/vmlinux
-rwxr-xr-x. 3 root root 826M Apr 29 2021
/usr/lib/debug/usr/lib/modules/4.18.0-305.el8.x86_64/vmlinux
crash> files -n ffff97c0f5cf5ec8 | ts.py
16:42:30.902 START
16:42:30.902 INODE NRPAGES
16:42:30.902 ffff97c0f5cf5ec8 211283
16:42:30.902
16:42:31.106 NODE PAGES
16:42:31.106 0 211283
16:42:31.106 END (ELAPS 0:00:00.204)
Really fast.
(This crash has your "files -n" v2 patch and is_page_ptr() check at the
beginning of summary_inode_page)
>>> [1]
>>>
https://github.com/crash-utility/crash/commit/4141373d9de3fea29f5d2b58f60...
>>>
>>> I don't object to add an entry to vmcoreinfo as a way of getting the
>>> vmemmap value. Seems like arm64 has another value from VMEMMAP_START.
>>>
>>> but there are some concerns in crash:
>>>
>>> - machdep->flags already has VMEMMAP flag. It's used like "if
>>> (IS_SPARSEMEM() && (machdep->flags & VMEMMAP)" e.g. [1].
Adding the new
>>> flag is very confusing and inconsistent.
>>>
>>> - Do all architectures have VMEMMAP_VADDR/END values in crash with
>>> CONFIG_SPARSEMEM_VMEMMAP configured?
> yes. your concern are right. I found some arch does not support the
> VMEMMAP_VADDR/END.
>
>
> So it is better to add this into ARM64 first. (I use the arm64 machine
> all the time..)
>
>
>>> we cannot test all archs, so this kind of arch-independent change makes
>>> me nervous. I would like to suggest an arch-specific use first. e.g.
>>> how about making x86_64_is_page_ptr more generic and use it?
>>>
> I will add arm64 version code. I am not familar with x86, and I do not
> have x86 machine for the testing.
>
> so it's better to not change it.
OK, I see.
>
>>> What happens if you use it on arm64? maybe the valid section can check
>>> it with VMEMMAP_VADDR without the vmemmap value. Just an idea though.
>> hmm sorry, probably this does not work, I missed this.
>>
>> + pfn = (addr - vt->vmemmap) / size;
> The code is converted by the linux code:
>
>
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/i...
>
> #define __page_to_pfn(page) (unsigned long)((page) - vmemmap)
>
>
> So it's correct.
>
>
>> but can VMEMMAP_VADDR and phys_offset can be used to get vmemmap value
>> on arm64?
> On arm64, I can get vmemmap by parsing "memstart_addr".
>
> But I cannot know if CONFIG_SPARSEMEM_VMEMMAP is enabled.
on x86_64, vmemmap_populate is used:
if (symbol_exists("vmemmap_populate"))
machdep->flags |= VMEMMAP;
on arm64, it's set unconditionally, is it ok?
It is really set unconditionally. I do not know why?
arm64_init(int when)
{
...
case PRE_GDB:
...
machdep->stacksize = ARM64_STACK_SIZE;
machdep->flags |= VMEMMAP;
>
> So if the kernel can export "vmemmap", everything become easy.
>
> All the archs supporting the VMEMMAP can benifit from it, such as arm64,
> ppc,loongarch,riscv..
Yes but we're not sure whether the users of the other archs have the
same issue (e.g. x86_64 is fast enough) and it's hard to test all archs.
Also, crash cannot always access vmcoreinfo (e.g. ramdumps), basically
it's better to implement without depending on vmcoreinfo first if
okay. But I use crash with vmcoreinfo(/proc/kcore) all the time:
1.) Test the performance
2.) Debug the linux kernel.
So I improve the vmcoreinfo to run the crash faster..
possible. If there is a case where crash cannot work without
vmcoreinfo, then we will add a entry though.
Thanks
Huang Shijie
>
> Thanks,
> Kazu
8:35 a.m.
New subject: [PATCH 2/2] Add vmemmap support
在 2023/12/1 16:08, HAGIO KAZUHITO(萩尾 一仁) 写道:
On 2023/11/30 17:26, Shijie Huang wrote:
> Hi Kazu,
>
> 在 2023/11/30 15:29, HAGIO KAZUHITO(萩尾 一仁) 写道:
>> On 2023/11/30 16:21, HAGIO KAZUHITO(萩尾 一仁) wrote:
>>> thanks for the patch.
>>>
>>> On 2023/11/28 12:41, Huang Shijie wrote:
>>>> If the kernel exports the vmmemap then we can use that symbol in
>>>> crash to optimize access. vmmemap is just an array of page structs
>>>> after all.
>>>>
>>>> This patch tries to get the "vmemmap" from the vmcore file.
>>>> If we can use the "vmemmap", use the vmemmap_is_page_ptr to
replace
>>>> the generic_is_page_ptr().
>>> I recalled that I also did a similar thing for x86_64 [1] in the past..
>>> so it's not so slow on x86_64 and it has valid section check too.
> Maybe your vmlinux is not so big as mine(441M)...
It's not so small :)
crash> files | grep -m1 vmlinux
5 ffff97c21fa2cf00 ffff97c2400e5200 ffff97c0f5cf5ec8 REG
/usr/lib/debug/usr/lib/modules/4.18.0-305.el8.x86_64/vmlinux
crash> !ls -lh /usr/lib/debug/usr/lib/modules/4.18.0-305.el8.x86_64/vmlinux
-rwxr-xr-x. 3 root root 826M Apr 29 2021
/usr/lib/debug/usr/lib/modules/4.18.0-305.el8.x86_64/vmlinux
crash> files -n ffff97c0f5cf5ec8 | ts.py
16:42:30.902 START
16:42:30.902 INODE NRPAGES
16:42:30.902 ffff97c0f5cf5ec8 211283
16:42:30.902
16:42:31.106 NODE PAGES
16:42:31.106 0 211283
16:42:31.106 END (ELAPS 0:00:00.204)
(This crash has your "files -n" v2 patch and is_page_ptr() check at the
beginning of summary_inode_page)
>>> [1]
>>>
https://github.com/crash-utility/crash/commit/4141373d9de3fea29f5d2b58f60...
>>>
>>> I don't object to add an entry to vmcoreinfo as a way of getting the
>>> vmemmap value. Seems like arm64 has another value from VMEMMAP_START.
>>>
>>> but there are some concerns in crash:
>>>
>>> - machdep->flags already has VMEMMAP flag. It's used like "if
>>> (IS_SPARSEMEM() && (machdep->flags & VMEMMAP)" e.g. [1].
Adding the new
>>> flag is very confusing and inconsistent.
>>>
>>> - Do all architectures have VMEMMAP_VADDR/END values in crash with
>>> CONFIG_SPARSEMEM_VMEMMAP configured?
> yes. your concern are right. I found some arch does not support the
> VMEMMAP_VADDR/END.
>
>
> So it is better to add this into ARM64 first. (I use the arm64 machine
> all the time..)
>
>
>>> we cannot test all archs, so this kind of arch-independent change makes
>>> me nervous. I would like to suggest an arch-specific use first. e.g.
>>> how about making x86_64_is_page_ptr more generic and use it?
>>>
> I will add arm64 version code. I am not familar with x86, and I do not
> have x86 machine for the testing.
>
> so it's better to not change it.
OK, I see.
>
>>> What happens if you use it on arm64? maybe the valid section can check
>>> it with VMEMMAP_VADDR without the vmemmap value. Just an idea though.
>> hmm sorry, probably this does not work, I missed this.
>>
>> + pfn = (addr - vt->vmemmap) / size;
> The code is converted by the linux code:
>
>
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/i...
>
> #define __page_to_pfn(page) (unsigned long)((page) - vmemmap)
>
>
> So it's correct.
>
>
>> but can VMEMMAP_VADDR and phys_offset can be used to get vmemmap value
>> on arm64?
> On arm64, I can get vmemmap by parsing "memstart_addr".
>
> But I cannot know if CONFIG_SPARSEMEM_VMEMMAP is enabled.
on x86_64, vmemmap_populate is used:
if (symbol_exists("vmemmap_populate"))
machdep->flags |= VMEMMAP;
on arm64, it's set unconditionally, is it ok?
arm64_init(int when)
{
...
case PRE_GDB:
...
machdep->stacksize = ARM64_STACK_SIZE;
machdep->flags |= VMEMMAP;
>
> So if the kernel can export "vmemmap", everything become easy.
>
> All the archs supporting the VMEMMAP can benifit from it, such as arm64,
> ppc,loongarch,riscv..
Yes but we're not sure whether the users of the other archs have the
same issue (e.g. x86_64 is fast enough) and it's hard to test all archs.
I suddenly realised that the reason why x86_64 is fast is because the
x86 has already implement
the vmemmap (or part of the vmemmap).
Thanks
Huang Shijie
6:16 p.m.
New subject: [PATCH 2/2] Add vmemmap support
在 2023/12/1 22:34, Shijie Huang 写道:
在 2023/12/1 16:08, HAGIO KAZUHITO(萩尾 一仁) 写道:
> On 2023/11/30 17:26, Shijie Huang wrote:
>> Hi Kazu,
>>
>> 在 2023/11/30 15:29, HAGIO KAZUHITO(萩尾 一仁) 写道:
>>> On 2023/11/30 16:21, HAGIO KAZUHITO(萩尾 一仁) wrote:
>>>> thanks for the patch.
>>>>
>>>> On 2023/11/28 12:41, Huang Shijie wrote:
>>>>> If the kernel exports the vmmemap then we can use that symbol in
>>>>> crash to optimize access. vmmemap is just an array of page structs
>>>>> after all.
>>>>>
>>>>> This patch tries to get the "vmemmap" from the vmcore
file.
>>>>> If we can use the "vmemmap", use the vmemmap_is_page_ptr to
replace
>>>>> the generic_is_page_ptr().
>>>> I recalled that I also did a similar thing for x86_64 [1] in the
>>>> past..
>>>> so it's not so slow on x86_64 and it has valid section check too.
>> Maybe your vmlinux is not so big as mine(441M)...
> It's not so small :)
>
> crash> files | grep -m1 vmlinux
> 5 ffff97c21fa2cf00 ffff97c2400e5200 ffff97c0f5cf5ec8 REG
> /usr/lib/debug/usr/lib/modules/4.18.0-305.el8.x86_64/vmlinux
> crash> !ls -lh
> /usr/lib/debug/usr/lib/modules/4.18.0-305.el8.x86_64/vmlinux
> -rwxr-xr-x. 3 root root 826M Apr 29 2021
> /usr/lib/debug/usr/lib/modules/4.18.0-305.el8.x86_64/vmlinux
> crash> files -n ffff97c0f5cf5ec8 | ts.py
> 16:42:30.902 START
> 16:42:30.902 INODE NRPAGES
> 16:42:30.902 ffff97c0f5cf5ec8 211283
> 16:42:30.902
> 16:42:31.106 NODE PAGES
> 16:42:31.106 0 211283
> 16:42:31.106 END (ELAPS 0:00:00.204)
>
> (This crash has your "files -n" v2 patch and is_page_ptr() check at the
> beginning of summary_inode_page)
>
>>>> [1]
>>>>
https://github.com/crash-utility/crash/commit/4141373d9de3fea29f5d2b58f60...
>>>>
>>>>
>>>> I don't object to add an entry to vmcoreinfo as a way of getting the
>>>> vmemmap value. Seems like arm64 has another value from
>>>> VMEMMAP_START.
>>>>
>>>> but there are some concerns in crash:
>>>>
>>>> - machdep->flags already has VMEMMAP flag. It's used like
"if
>>>> (IS_SPARSEMEM() && (machdep->flags & VMEMMAP)" e.g.
[1]. Adding
>>>> the new
>>>> flag is very confusing and inconsistent.
>>>>
>>>> - Do all architectures have VMEMMAP_VADDR/END values in crash with
>>>> CONFIG_SPARSEMEM_VMEMMAP configured?
>> yes. your concern are right. I found some arch does not support the
>> VMEMMAP_VADDR/END.
>>
>>
>> So it is better to add this into ARM64 first. (I use the arm64 machine
>> all the time..)
>>
>>
>>>> we cannot test all archs, so this kind of arch-independent change
>>>> makes
>>>> me nervous. I would like to suggest an arch-specific use first.
>>>> e.g.
>>>> how about making x86_64_is_page_ptr more generic and use it?
>>>>
>> I will add arm64 version code. I am not familar with x86, and I do not
>> have x86 machine for the testing.
>>
>> so it's better to not change it.
> OK, I see.
>
>>
>>>> What happens if you use it on arm64? maybe the valid section can
>>>> check
>>>> it with VMEMMAP_VADDR without the vmemmap value. Just an idea
>>>> though.
>>> hmm sorry, probably this does not work, I missed this.
>>>
>>> + pfn = (addr - vt->vmemmap) / size;
>> The code is converted by the linux code:
>>
>>
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/i...
>>
>>
>> #define __page_to_pfn(page) (unsigned long)((page) - vmemmap)
>>
>>
>> So it's correct.
>>
>>
>>> but can VMEMMAP_VADDR and phys_offset can be used to get vmemmap value
>>> on arm64?
>> On arm64, I can get vmemmap by parsing "memstart_addr".
>>
>> But I cannot know if CONFIG_SPARSEMEM_VMEMMAP is enabled.
> on x86_64, vmemmap_populate is used:
>
> if (symbol_exists("vmemmap_populate"))
> machdep->flags |= VMEMMAP;
>
> on arm64, it's set unconditionally, is it ok?
>
> arm64_init(int when)
> {
> ...
> case PRE_GDB:
> ...
> machdep->stacksize = ARM64_STACK_SIZE;
> machdep->flags |= VMEMMAP;
>
>>
>> So if the kernel can export "vmemmap", everything become easy.
>>
>> All the archs supporting the VMEMMAP can benifit from it, such as
>> arm64,
>> ppc,loongarch,riscv..
> Yes but we're not sure whether the users of the other archs have the
> same issue (e.g. x86_64 is fast enough) and it's hard to test all archs.
I suddenly realised that the reason why x86_64 is fast is because the
x86 has already implement
the vmemmap (or part of the vmemmap).
I read the x86 code, and find this:
https://github.com/torvalds/linux/blob/master/arch/x86/include/asm/pgtabl...
#define vmemmap ((struct page *)VMEMMAP_START)
In crash code, the x86_64_is_page_ptr():
--
1812:x86_64_is_page_ptr(ulong addr, physaddr_t *phys)
1813-{
1814- ulong pfn, nr;
1815-
1816- if (IS_SPARSEMEM() && (machdep->flags & VMEMMAP) &&
1817- (addr >= VMEMMAP_VADDR && addr <= VMEMMAP_END) &&
1818- !((addr - VMEMMAP_VADDR) % SIZE(page))) {
1819-
1820- pfn = (addr - VMEMMAP_VADDR) / SIZE(page);
1821- nr = pfn_to_section_nr(pfn);
1822- if (valid_section_nr(nr)) {
1823- if (phys)
1824- *phys = PTOB(pfn);
1825- return TRUE;
1826- }
1827- }
1828- return FALSE;
1829-}
In x86, the VMEMMAP_VADDR is equal to "vmemmap". Since the current
kernel does not export "vmemmap",
x86 cannot use the vmemmap too, so it uses the VMEMMAP_VADDR in the code.
In logic, the x86_64_is_page_ptr() is wrong, it gets the correct result
by accident.
If the "vmemmap" is exported by kernel, the x86_64_is_page_ptr() and
arm64_vmemmap_is_page_ptr() can merge into
one function, and rename it to "vmemmap_is_page_ptr()", and move it to a
common place, such as memory.c.
Thanks
Huang Shijie
1:22 a.m.
On 2023/11/28 12:41, Huang Shijie wrote:
Add get_value_vmcore() to get the symbol value for @name.
Also add macro GET_SYM to simplify the code.
Signed-off-by: Huang Shijie <shijie(a)os.amperecomputing.com>
---
defs.h | 1 +
kernel.c | 85 +++++++++++++++++++-------------------------------------
2 files changed, 30 insertions(+), 56 deletions(-)
diff --git a/defs.h b/defs.h
index 1fe2d0b..eec7b3e 100644
--- a/defs.h
+++ b/defs.h
@@ -6055,6 +6055,7 @@ int hide_offline_cpu(int);
int get_highest_cpu_online(void);
int get_highest_cpu_present(void);
int get_cpus_to_display(void);
+bool get_value_vmcore(const char *name, ulong *v);
void get_log_from_vmcoreinfo(char *file);
int in_cpu_map(int, int);
void paravirt_init(void);
diff --git a/kernel.c b/kernel.c
index 6dcf414..caf6149 100644
--- a/kernel.c
+++ b/kernel.c
@@ -104,6 +104,20 @@ static void check_vmcoreinfo(void);
static int is_pvops_xen(void);
static int get_linux_banner_from_vmlinux(char *, size_t);
+/* Return TRUE if we succeed, return FALSE on failure. */
+bool
+get_value_vmcore(const char *name, ulong *v)
"get_value_vmcoreinfo" is better to clarify.
+{
+ char *string = pc->read_vmcoreinfo(name);
+
+ if (!string)
+ return FALSE;
+
+ *v = htol(string, RETURN_ON_ERROR, NULL);
+ free(string);
+ return TRUE;
+}
+
/*
* popuplate the global kernel table (kt) with kernel version
* information parsed from UTSNAME/OSRELEASE string
@@ -10984,6 +10998,12 @@ hypervisor_init(void)
fprintf(fp, "hypervisor: %s\n", kt->hypervisor);
}
+#define GET_SYM(s,v) \
How about "GET_SYMBOL" and
+ if (get_value_vmcore((s), &(v))) { \
adding "SYMBOL(" and ")" here?
+ if (CRASHDEBUG(1)) \
+ fprintf(fp, s ": %lx\n", v); \
+ }
+
/*
* Get and display the kernel log buffer using the vmcoreinfo
* data alone without the vmlinux file.
@@ -11024,62 +11044,15 @@ get_log_from_vmcoreinfo(char *file)
} else
error(FATAL, "VMCOREINFO: cannot determine page size\n");
- if ((string = pc->read_vmcoreinfo("SYMBOL(log_buf)"))) {
- vmc->log_buf_SYMBOL = htol(string, RETURN_ON_ERROR, NULL);
- if (CRASHDEBUG(1))
- fprintf(fp, "SYMBOL(log_buf): %lx\n",
- vmc->log_buf_SYMBOL);
- free(string);
- }
- if ((string = pc->read_vmcoreinfo("SYMBOL(log_end)"))) {
- vmc->log_end_SYMBOL = htol(string, RETURN_ON_ERROR, NULL);
- if (CRASHDEBUG(1))
- fprintf(fp, "SYMBOL(log_end): %lx\n",
- vmc->log_end_SYMBOL);
- free(string);
- }
- if ((string = pc->read_vmcoreinfo("SYMBOL(log_buf_len)"))) {
- vmc->log_buf_len_SYMBOL = htol(string, RETURN_ON_ERROR, NULL);
- if (CRASHDEBUG(1))
- fprintf(fp, "SYMBOL(log_buf_len): %lx\n",
- vmc->log_buf_len_SYMBOL);
- free(string);
- }
- if ((string = pc->read_vmcoreinfo("SYMBOL(logged_chars)"))) {
- vmc->logged_chars_SYMBOL = htol(string, RETURN_ON_ERROR, NULL);
- if (CRASHDEBUG(1))
- fprintf(fp, "SYMBOL(logged_chars): %lx\n",
- vmc->logged_chars_SYMBOL);
- free(string);
- }
- if ((string = pc->read_vmcoreinfo("SYMBOL(log_first_idx)"))) {
- vmc->log_first_idx_SYMBOL = htol(string, RETURN_ON_ERROR, NULL);
- if (CRASHDEBUG(1))
- fprintf(fp, "SYMBOL(log_first_idx): %lx\n",
- vmc->log_first_idx_SYMBOL);
- free(string);
- }
- if ((string = pc->read_vmcoreinfo("SYMBOL(log_next_idx)"))) {
- vmc->log_next_idx_SYMBOL = htol(string, RETURN_ON_ERROR, NULL);
- if (CRASHDEBUG(1))
- fprintf(fp, "SYMBOL(log_next_idx): %lx\n",
- vmc->log_next_idx_SYMBOL);
- free(string);
- }
- if ((string = pc->read_vmcoreinfo("SYMBOL(phys_base)"))) {
- vmc->phys_base_SYMBOL = htol(string, RETURN_ON_ERROR, NULL);
- if (CRASHDEBUG(1))
- fprintf(fp, "SYMBOL(phys_base): %lx\n",
- vmc->phys_base_SYMBOL);
- free(string);
- }
- if ((string = pc->read_vmcoreinfo("SYMBOL(_stext)"))) {
- vmc->_stext_SYMBOL = htol(string, RETURN_ON_ERROR, NULL);
- if (CRASHDEBUG(1))
- fprintf(fp, "SYMBOL(_stext): %lx\n",
- vmc->_stext_SYMBOL);
- free(string);
- }
+ GET_SYM("SYMBOL(log_buf)", vmc->log_buf_SYMBOL);
then GET_SYMBOL("log_buf", vmc->log_buf_SYMBOL);
Thanks,
Kazu
> + GET_SYM("SYMBOL(log_end)", vmc->log_end_SYMBOL);
> + GET_SYM("SYMBOL(log_buf_len)", vmc->log_buf_len_SYMBOL);
> + GET_SYM("SYMBOL(logged_chars)", vmc->logged_chars_SYMBOL);
> + GET_SYM("SYMBOL(log_first_idx)", vmc->log_first_idx_SYMBOL);
> + GET_SYM("SYMBOL(log_next_idx)", vmc->log_next_idx_SYMBOL);
> + GET_SYM("SYMBOL(phys_base)", vmc->phys_base_SYMBOL);
> + GET_SYM("SYMBOL(_stext)", vmc->_stext_SYMBOL);
> +
> if ((string = pc->read_vmcoreinfo("OFFSET(log.ts_nsec)"))) {
> vmc->log_ts_nsec_OFFSET = dtol(string, RETURN_ON_ERROR, NULL);
> if (CRASHDEBUG(1))
2:37 a.m.
在 2023/11/30 15:22, HAGIO KAZUHITO(萩尾 一仁) 写道:
On 2023/11/28 12:41, Huang Shijie wrote:
> Add get_value_vmcore() to get the symbol value for @name.
>
> Also add macro GET_SYM to simplify the code.
>
> Signed-off-by: Huang Shijie <shijie(a)os.amperecomputing.com>
> ---
> defs.h | 1 +
> kernel.c | 85 +++++++++++++++++++-------------------------------------
> 2 files changed, 30 insertions(+), 56 deletions(-)
>
> diff --git a/defs.h b/defs.h
> index 1fe2d0b..eec7b3e 100644
> --- a/defs.h
> +++ b/defs.h
> @@ -6055,6 +6055,7 @@ int hide_offline_cpu(int);
> int get_highest_cpu_online(void);
> int get_highest_cpu_present(void);
> int get_cpus_to_display(void);
> +bool get_value_vmcore(const char *name, ulong *v);
> void get_log_from_vmcoreinfo(char *file);
> int in_cpu_map(int, int);
> void paravirt_init(void);
> diff --git a/kernel.c b/kernel.c
> index 6dcf414..caf6149 100644
> --- a/kernel.c
> +++ b/kernel.c
> @@ -104,6 +104,20 @@ static void check_vmcoreinfo(void);
> static int is_pvops_xen(void);
> static int get_linux_banner_from_vmlinux(char *, size_t);
>
> +/* Return TRUE if we succeed, return FALSE on failure. */
> +bool
> +get_value_vmcore(const char *name, ulong *v)
"get_value_vmcoreinfo" is better to clarify.
What about get_symbol_value_vmcoreinfo()?
We can use "htol" to do the convert for symbol, but htol does not work
for "NUMBER()"..
> +{
> + char *string = pc->read_vmcoreinfo(name);
> +
> + if (!string)
> + return FALSE;
> +
> + *v = htol(string, RETURN_ON_ERROR, NULL);
> + return TRUE;
> +}
> +
> /*
> * popuplate the global kernel table (kt) with kernel version
> * information parsed from UTSNAME/OSRELEASE string
> @@ -10984,6 +10998,12 @@ hypervisor_init(void)
> fprintf(fp, "hypervisor: %s\n", kt->hypervisor);
> }
>
> +#define GET_SYM(s,v) \
How about "GET_SYMBOL" and
no problem.
> + if (get_value_vmcore((s), &(v))) { \
adding "SYMBOL(" and ")" here?
it is not easy to add "SYMBOL" here.
I prefer to add to get_symbol_value_vmcoreinfo().
> + if (CRASHDEBUG(1)) \
> + fprintf(fp, s ": %lx\n", v); \
> + }
> +
> /*
> * Get and display the kernel log buffer using the vmcoreinfo
> * data alone without the vmlinux file.
> @@ -11024,62 +11044,15 @@ get_log_from_vmcoreinfo(char *file)
> } else
> error(FATAL, "VMCOREINFO: cannot determine page size\n");
>
> - if ((string = pc->read_vmcoreinfo("SYMBOL(log_buf)"))) {
> - vmc->log_buf_SYMBOL = htol(string, RETURN_ON_ERROR, NULL);
> - if (CRASHDEBUG(1))
> - fprintf(fp, "SYMBOL(log_buf): %lx\n",
> - vmc->log_buf_SYMBOL);
> - free(string);
> - }
> - if ((string = pc->read_vmcoreinfo("SYMBOL(log_end)"))) {
> - vmc->log_end_SYMBOL = htol(string, RETURN_ON_ERROR, NULL);
> - if (CRASHDEBUG(1))
> - fprintf(fp, "SYMBOL(log_end): %lx\n",
> - vmc->log_end_SYMBOL);
> - free(string);
> - }
> - if ((string = pc->read_vmcoreinfo("SYMBOL(log_buf_len)"))) {
> - vmc->log_buf_len_SYMBOL = htol(string, RETURN_ON_ERROR, NULL);
> - if (CRASHDEBUG(1))
> - fprintf(fp, "SYMBOL(log_buf_len): %lx\n",
> - vmc->log_buf_len_SYMBOL);
> - free(string);
> - }
> - if ((string = pc->read_vmcoreinfo("SYMBOL(logged_chars)"))) {
> - vmc->logged_chars_SYMBOL = htol(string, RETURN_ON_ERROR, NULL);
> - if (CRASHDEBUG(1))
> - fprintf(fp, "SYMBOL(logged_chars): %lx\n",
> - vmc->logged_chars_SYMBOL);
> - free(string);
> - }
> - if ((string = pc->read_vmcoreinfo("SYMBOL(log_first_idx)"))) {
> - vmc->log_first_idx_SYMBOL = htol(string, RETURN_ON_ERROR, NULL);
> - if (CRASHDEBUG(1))
> - fprintf(fp, "SYMBOL(log_first_idx): %lx\n",
> - vmc->log_first_idx_SYMBOL);
> - free(string);
> - }
> - if ((string = pc->read_vmcoreinfo("SYMBOL(log_next_idx)"))) {
> - vmc->log_next_idx_SYMBOL = htol(string, RETURN_ON_ERROR, NULL);
> - if (CRASHDEBUG(1))
> - fprintf(fp, "SYMBOL(log_next_idx): %lx\n",
> - vmc->log_next_idx_SYMBOL);
> - free(string);
> - }
> - if ((string = pc->read_vmcoreinfo("SYMBOL(phys_base)"))) {
> - vmc->phys_base_SYMBOL = htol(string, RETURN_ON_ERROR, NULL);
> - if (CRASHDEBUG(1))
> - fprintf(fp, "SYMBOL(phys_base): %lx\n",
> - vmc->phys_base_SYMBOL);
> - free(string);
> - }
> - if ((string = pc->read_vmcoreinfo("SYMBOL(_stext)"))) {
> - vmc->_stext_SYMBOL = htol(string, RETURN_ON_ERROR, NULL);
> - if (CRASHDEBUG(1))
> - fprintf(fp, "SYMBOL(_stext): %lx\n",
> - vmc->_stext_SYMBOL);
> - free(string);
> - }
> + GET_SYM("SYMBOL(log_buf)", vmc->log_buf_SYMBOL);
then GET_SYMBOL("log_buf", vmc->log_buf_SYMBOL);
okay.
Thanks
Huang Shijie
1:37 a.m.
On 2023/11/30 17:37, Shijie Huang wrote:
>> @@ -104,6 +104,20 @@ static void check_vmcoreinfo(void);
>> static int is_pvops_xen(void);
>> static int get_linux_banner_from_vmlinux(char *, size_t);
>> +/* Return TRUE if we succeed, return FALSE on failure. */
>> +bool
>> +get_value_vmcore(const char *name, ulong *v)
> "get_value_vmcoreinfo" is better to clarify.
What about get_symbol_value_vmcoreinfo()?
Agree, it's better.
We can use "htol" to do the convert for symbol, but htol does not work
for "NUMBER()"..
>
>> +{
>> + char *string = pc->read_vmcoreinfo(name);
>> +
>> + if (!string)
>> + return FALSE;
>> +
>> + *v = htol(string, RETURN_ON_ERROR, NULL);
>> + return TRUE;
>> +}
>> +
>> /*
>> * popuplate the global kernel table (kt) with kernel version
>> * information parsed from UTSNAME/OSRELEASE string
>> @@ -10984,6 +10998,12 @@ hypervisor_init(void)
>> fprintf(fp, "hypervisor: %s\n", kt->hypervisor);
>> }
>> +#define GET_SYM(s,v) \
> How about "GET_SYMBOL" and
no problem.
>> + if (get_value_vmcore((s), &(v))) { \
> adding "SYMBOL(" and ")" here?
it is not easy to add "SYMBOL" here.
I prefer to add to get_symbol_value_vmcoreinfo().
I mean
if (get_symbol_value_vmcoreinfo("SYMBOL(" s ")", &(v))) ...
>
>> + if (CRASHDEBUG(1)) \
>> + fprintf(fp, s ": %lx\n", v); \
like this. possible?
Thanks,
Kazu
3:35 a.m.
在 2023/12/1 15:37, HAGIO KAZUHITO(萩尾 一仁) 写道:
[EXTERNAL EMAIL NOTICE: This email originated from an external
sender. Please be mindful of safe email handling and proprietary information protection
practices.]
On 2023/11/30 17:37, Shijie Huang wrote:
>>> @@ -104,6 +104,20 @@ static void check_vmcoreinfo(void);
>>> static int is_pvops_xen(void);
>>> static int get_linux_banner_from_vmlinux(char *, size_t);
>>> +/* Return TRUE if we succeed, return FALSE on failure. */
>>> +bool
>>> +get_value_vmcore(const char *name, ulong *v)
>> "get_value_vmcoreinfo" is better to clarify.
> What about get_symbol_value_vmcoreinfo()?
Agree, it's better.
> We can use "htol" to do the convert for symbol, but htol does not work
> for "NUMBER()"..
>
>
>>> +{
>>> + char *string = pc->read_vmcoreinfo(name);
>>> +
>>> + if (!string)
>>> + return FALSE;
>>> +
>>> + *v = htol(string, RETURN_ON_ERROR, NULL);
>>> + return TRUE;
>>> +}
>>> +
>>> /*
>>> * popuplate the global kernel table (kt) with kernel version
>>> * information parsed from UTSNAME/OSRELEASE string
>>> @@ -10984,6 +10998,12 @@ hypervisor_init(void)
>>> fprintf(fp, "hypervisor: %s\n", kt->hypervisor);
>>> }
>>> +#define GET_SYM(s,v) \
>> How about "GET_SYMBOL" and
> no problem.
>
>
>>> + if (get_value_vmcore((s), &(v))) { \
>> adding "SYMBOL(" and ")" here?
> it is not easy to add "SYMBOL" here.
>
> I prefer to add to get_symbol_value_vmcoreinfo().
I mean
if (get_symbol_value_vmcoreinfo("SYMBOL(" s ")", &(v))) ...
thanks, I have better solution now :)
Thanks
Huang Shijie
210
days inactive
214
days old
devel@lists.crash-utility.osci.io
14 comments
4 participants
participants (4)
-
HAGIO KAZUHITO(萩尾 一仁) -
Huang Shijie -
Lameter, Christopher -
Shijie Huang