11:02 a.m.
Hi Dave,
I noticed that the "timer" command prints somewhat wrongly for RHEL7.6 or
later kernels (not including RHEL8 / upstream) and found that it is because
struct tvec and tvec_root have an array of list_head, instead of hlist_head
in upstream kernels.
So the following patch looks good only for RHEL7, but I don't know how we
should determine the list type in order to switch the size, etc in this case.
What do you think?
Thanks,
Kazu
---
diff --git a/kernel.c b/kernel.c
index a79e5a3010ea..419cca3ac7ae 100644
--- a/kernel.c
+++ b/kernel.c
@@ -8376,7 +8376,7 @@ dump_timer_data_tvec_bases_v3(const ulong *cpus)
char buf4[BUFSIZE];
vec_root_size = vec_size = 0;
- head_size = SIZE(hlist_head);
+ head_size = SIZE(list_head);
if ((i = get_array_length("tvec_root.vec", NULL, head_size)))
vec_root_size = i;
@@ -8804,19 +8804,19 @@ do_timer_list_v3(ulong vec_kvaddr,
tdx++;
}
- readmem(vec_kvaddr, KVADDR, vec, SIZE(hlist_head) * size,
+ readmem(vec_kvaddr, KVADDR, vec, SIZE(list_head) * size,
"timer_list vec array", FAULT_ON_ERROR);
ld = &list_data;
timer_list_buf = GETBUF(SIZE(timer_list));
- for (i = count = 0; i < size; i++, vec_kvaddr += SIZE(hlist_head)) {
+ for (i = count = 0; i < size; i++, vec_kvaddr += SIZE(list_head)) {
- if (vec[i] == 0)
+ if (vec[i*2] == vec_kvaddr)
continue;
BZERO(ld, sizeof(struct list_data));
- ld->start = vec[i];
+ ld->start = vec[i*2];
ld->list_head_offset = OFFSET(timer_list_entry);
ld->end = vec_kvaddr;
ld->flags = RETURN_ON_LIST_ERROR;
12:22 p.m.
----- Original Message -----
Hi Dave,
I noticed that the "timer" command prints somewhat wrongly for RHEL7.6 or
later kernels (not including RHEL8 / upstream) and found that it is because
struct tvec and tvec_root have an array of list_head, instead of hlist_head
in upstream kernels.
So the following patch looks good only for RHEL7, but I don't know how we
should determine the list type in order to switch the size, etc in this case.
What do you think?
Try using the MEMBER_TYPE_NAME() macro. For example, adding this to cmd_test():
--- a/test.c 2011-09-01 14:06:56.000000000 -0400
+++ b/test.c 2019-08-01 13:16:13.783615151 -0400
@@ -58,6 +58,7 @@ cmd_test(void)
;
optind++;
}
+fprintf(fp, "%s\n", MEMBER_TYPE_NAME("tvec_root", "vec"));
}
shows this on a RHEL7 kernel:
crash> struct tvec_root
struct tvec_root {
struct list_head vec[256];
}
SIZE: 409
crash> test
list_head
crash> s
Dave
Thanks,
Kazu
---
diff --git a/kernel.c b/kernel.c
index a79e5a3010ea..419cca3ac7ae 100644
--- a/kernel.c
+++ b/kernel.c
@@ -8376,7 +8376,7 @@ dump_timer_data_tvec_bases_v3(const ulong *cpus)
char buf4[BUFSIZE];
vec_root_size = vec_size = 0;
- head_size = SIZE(hlist_head);
+ head_size = SIZE(list_head);
if ((i = get_array_length("tvec_root.vec", NULL, head_size)))
vec_root_size = i;
@@ -8804,19 +8804,19 @@ do_timer_list_v3(ulong vec_kvaddr,
tdx++;
}
- readmem(vec_kvaddr, KVADDR, vec, SIZE(hlist_head) * size,
+ readmem(vec_kvaddr, KVADDR, vec, SIZE(list_head) * size,
"timer_list vec array", FAULT_ON_ERROR);
ld = &list_data;
timer_list_buf = GETBUF(SIZE(timer_list));
- for (i = count = 0; i < size; i++, vec_kvaddr += SIZE(hlist_head)) {
+ for (i = count = 0; i < size; i++, vec_kvaddr += SIZE(list_head)) {
- if (vec[i] == 0)
+ if (vec[i*2] == vec_kvaddr)
continue;
BZERO(ld, sizeof(struct list_data));
- ld->start = vec[i];
+ ld->start = vec[i*2];
ld->list_head_offset = OFFSET(timer_list_entry);
ld->end = vec_kvaddr;
ld->flags = RETURN_ON_LIST_ERROR;
1:31 p.m.
On 8/1/2019 1:22 PM, Dave Anderson wrote:
----- Original Message -----
> Hi Dave,
>
> I noticed that the "timer" command prints somewhat wrongly for RHEL7.6 or
> later kernels (not including RHEL8 / upstream) and found that it is because
> struct tvec and tvec_root have an array of list_head, instead of hlist_head
> in upstream kernels.
>
> So the following patch looks good only for RHEL7, but I don't know how we
> should determine the list type in order to switch the size, etc in this case.
> What do you think?
Try using the MEMBER_TYPE_NAME() macro. For example, adding this to cmd_test():
Nice. I'll try to rewrite the patch with it.
Thanks!
Kazu
--- a/test.c 2011-09-01 14:06:56.000000000 -0400
+++ b/test.c 2019-08-01 13:16:13.783615151 -0400
@@ -58,6 +58,7 @@ cmd_test(void)
;
optind++;
}
+fprintf(fp, "%s\n", MEMBER_TYPE_NAME("tvec_root",
"vec"));
}
shows this on a RHEL7 kernel:
crash> struct tvec_root
struct tvec_root {
struct list_head vec[256];
}
SIZE: 409
crash> test
list_head
crash> s
Dave
>
> Thanks,
> Kazu
>
> ---
> diff --git a/kernel.c b/kernel.c
> index a79e5a3010ea..419cca3ac7ae 100644
> --- a/kernel.c
> +++ b/kernel.c
> @@ -8376,7 +8376,7 @@ dump_timer_data_tvec_bases_v3(const ulong *cpus)
> char buf4[BUFSIZE];
>
> vec_root_size = vec_size = 0;
> - head_size = SIZE(hlist_head);
> + head_size = SIZE(list_head);
>
> if ((i = get_array_length("tvec_root.vec", NULL, head_size)))
> vec_root_size = i;
> @@ -8804,19 +8804,19 @@ do_timer_list_v3(ulong vec_kvaddr,
> tdx++;
> }
>
> - readmem(vec_kvaddr, KVADDR, vec, SIZE(hlist_head) * size,
> + readmem(vec_kvaddr, KVADDR, vec, SIZE(list_head) * size,
> "timer_list vec array", FAULT_ON_ERROR);
>
> ld = &list_data;
> timer_list_buf = GETBUF(SIZE(timer_list));
>
> - for (i = count = 0; i < size; i++, vec_kvaddr += SIZE(hlist_head)) {
> + for (i = count = 0; i < size; i++, vec_kvaddr += SIZE(list_head)) {
>
> - if (vec[i] == 0)
> + if (vec[i*2] == vec_kvaddr)
> continue;
>
> BZERO(ld, sizeof(struct list_data));
> - ld->start = vec[i];
> + ld->start = vec[i*2];
> ld->list_head_offset = OFFSET(timer_list_entry);
> ld->end = vec_kvaddr;
> ld->flags = RETURN_ON_LIST_ERROR;
>
--
Crash-utility mailing list
Crash-utility(a)redhat.com
https://www.redhat.com/mailman/listinfo/crash-utility
3:06 p.m.
On 8/1/2019 2:31 PM, Kazuhito Hagio wrote:
On 8/1/2019 1:22 PM, Dave Anderson wrote:
>
>
> ----- Original Message -----
>> Hi Dave,
>>
>> I noticed that the "timer" command prints somewhat wrongly for RHEL7.6
or
>> later kernels (not including RHEL8 / upstream) and found that it is because
>> struct tvec and tvec_root have an array of list_head, instead of hlist_head
>> in upstream kernels.
>>
>> So the following patch looks good only for RHEL7, but I don't know how we
>> should determine the list type in order to switch the size, etc in this case.
>> What do you think?
>
> Try using the MEMBER_TYPE_NAME() macro. For example, adding this to cmd_test():
Nice. I'll try to rewrite the patch with it.
I choose to add head_size arg to do_timer_list_v3().
This tested OK on RHEL7.6 and kernel 4.2.1.
Signed-off-by: Kazuhito Hagio <k-hagio(a)ab.jp.nec.com>
---
diff --git a/kernel.c b/kernel.c
index a79e5a3010ea..7804aef2074b 100644
--- a/kernel.c
+++ b/kernel.c
@@ -53,7 +53,7 @@ static void dump_timer_data_timer_bases(const ulong *cpus);
struct tv_range;
static void init_tv_ranges(struct tv_range *, int, int, int);
static int do_timer_list(ulong,int, ulong *, void *,ulong *, ulong *, struct tv_range *,
ulong);
-static int do_timer_list_v3(ulong, int, ulong *, void *,ulong *, ulong *, ulong);
+static int do_timer_list_v3(ulong, int, ulong *, void *,ulong *, ulong *, ulong, long);
struct timer_bases_data;
static int do_timer_list_v4(struct timer_bases_data *, ulong);
static int compare_timer_data(const void *, const void *);
@@ -8376,7 +8376,12 @@ dump_timer_data_tvec_bases_v3(const ulong *cpus)
char buf4[BUFSIZE];
vec_root_size = vec_size = 0;
- head_size = SIZE(hlist_head);
+
+ if (STREQ(MEMBER_TYPE_NAME("tvec_root", "vec"),
"list_head"))
+ /* for RHEL7.6 or later */
+ head_size = SIZE(list_head);
+ else
+ head_size = SIZE(hlist_head);
if ((i = get_array_length("tvec_root.vec", NULL, head_size)))
vec_root_size = i;
@@ -8414,15 +8419,15 @@ next_cpu:
init_tv_ranges(tv, vec_root_size, vec_size, cpu);
count += do_timer_list_v3(tv[1].base + OFFSET(tvec_root_s_vec),
- vec_root_size, vec, NULL, NULL, NULL, 0);
+ vec_root_size, vec, NULL, NULL, NULL, 0, head_size);
count += do_timer_list_v3(tv[2].base + OFFSET(tvec_s_vec),
- vec_size, vec, NULL, NULL, NULL, 0);
+ vec_size, vec, NULL, NULL, NULL, 0, head_size);
count += do_timer_list_v3(tv[3].base + OFFSET(tvec_s_vec),
- vec_size, vec, NULL, NULL, NULL, 0);
+ vec_size, vec, NULL, NULL, NULL, 0, head_size);
count += do_timer_list_v3(tv[4].base + OFFSET(tvec_s_vec),
- vec_size, vec, NULL, NULL, NULL, 0);
+ vec_size, vec, NULL, NULL, NULL, 0, head_size);
count += do_timer_list_v3(tv[5].base + OFFSET(tvec_s_vec),
- vec_size, vec, NULL, NULL, NULL, 0);
+ vec_size, vec, NULL, NULL, NULL, 0, head_size);
if (count)
td = (struct timer_data *)
@@ -8433,16 +8438,16 @@ next_cpu:
get_symbol_data("jiffies", sizeof(ulong), &jiffies);
- do_timer_list_v3(tv[1].base + OFFSET(tvec_root_s_vec),
- vec_root_size, vec, (void *)td, &highest, &highest_tte, jiffies);
- do_timer_list_v3(tv[2].base + OFFSET(tvec_s_vec),
- vec_size, vec, (void *)td, &highest, &highest_tte, jiffies);
- do_timer_list_v3(tv[3].base + OFFSET(tvec_s_vec),
- vec_size, vec, (void *)td, &highest, &highest_tte, jiffies);
- do_timer_list_v3(tv[4].base + OFFSET(tvec_s_vec),
- vec_size, vec, (void *)td, &highest, &highest_tte, jiffies);
- tdx = do_timer_list_v3(tv[5].base + OFFSET(tvec_s_vec),
- vec_size, vec, (void *)td, &highest, &highest_tte, jiffies);
+ do_timer_list_v3(tv[1].base + OFFSET(tvec_root_s_vec), vec_root_size,
+ vec, (void *)td, &highest, &highest_tte, jiffies, head_size);
+ do_timer_list_v3(tv[2].base + OFFSET(tvec_s_vec), vec_size,
+ vec, (void *)td, &highest, &highest_tte, jiffies, head_size);
+ do_timer_list_v3(tv[3].base + OFFSET(tvec_s_vec), vec_size,
+ vec, (void *)td, &highest, &highest_tte, jiffies, head_size);
+ do_timer_list_v3(tv[4].base + OFFSET(tvec_s_vec), vec_size,
+ vec, (void *)td, &highest, &highest_tte, jiffies, head_size);
+ tdx = do_timer_list_v3(tv[5].base + OFFSET(tvec_s_vec), vec_size,
+ vec, (void *)td, &highest, &highest_tte, jiffies, head_size);
qsort(td, tdx, sizeof(struct timer_data), compare_timer_data);
@@ -8786,7 +8791,8 @@ do_timer_list_v3(ulong vec_kvaddr,
void *option,
ulong *highest,
ulong *highest_tte,
- ulong jiffies)
+ ulong jiffies,
+ long head_size)
{
int i, t;
int count, tdx;
@@ -8804,19 +8810,24 @@ do_timer_list_v3(ulong vec_kvaddr,
tdx++;
}
- readmem(vec_kvaddr, KVADDR, vec, SIZE(hlist_head) * size,
+ readmem(vec_kvaddr, KVADDR, vec, head_size * size,
"timer_list vec array", FAULT_ON_ERROR);
ld = &list_data;
timer_list_buf = GETBUF(SIZE(timer_list));
- for (i = count = 0; i < size; i++, vec_kvaddr += SIZE(hlist_head)) {
+ for (i = count = 0; i < size; i++, vec_kvaddr += head_size) {
- if (vec[i] == 0)
- continue;
+ if (head_size == SIZE(list_head)) {
+ if (vec[i*2] == vec_kvaddr)
+ continue;
+ } else {
+ if (vec[i] == 0)
+ continue;
+ }
BZERO(ld, sizeof(struct list_data));
- ld->start = vec[i];
+ ld->start = (head_size == SIZE(list_head)) ? vec[i*2] : vec[i];
ld->list_head_offset = OFFSET(timer_list_entry);
ld->end = vec_kvaddr;
ld->flags = RETURN_ON_LIST_ERROR;
11:05 a.m.
----- Original Message -----
On 8/1/2019 2:31 PM, Kazuhito Hagio wrote:
> On 8/1/2019 1:22 PM, Dave Anderson wrote:
>>
>>
>> ----- Original Message -----
>>> Hi Dave,
>>>
>>> I noticed that the "timer" command prints somewhat wrongly for
RHEL7.6 or
>>> later kernels (not including RHEL8 / upstream) and found that it is
>>> because
>>> struct tvec and tvec_root have an array of list_head, instead of
>>> hlist_head
>>> in upstream kernels.
>>>
>>> So the following patch looks good only for RHEL7, but I don't know how
we
>>> should determine the list type in order to switch the size, etc in this
case.
>>> What do you think?
>>
>> Try using the MEMBER_TYPE_NAME() macro. For example, adding this to
>> cmd_test():
>
> Nice. I'll try to rewrite the patch with it.
I choose to add head_size arg to do_timer_list_v3().
This tested OK on RHEL7.6 and kernel 4.2.1.
Thanks Kazu, it looks good -- queued for crash-7.2.7:
https://github.com/crash-utility/crash/commit/a812e59a175c8c6197ae3dc78bd...
Dave
Signed-off-by: Kazuhito Hagio <k-hagio(a)ab.jp.nec.com>
---
diff --git a/kernel.c b/kernel.c
index a79e5a3010ea..7804aef2074b 100644
--- a/kernel.c
+++ b/kernel.c
@@ -53,7 +53,7 @@ static void dump_timer_data_timer_bases(const ulong *cpus);
struct tv_range;
static void init_tv_ranges(struct tv_range *, int, int, int);
static int do_timer_list(ulong,int, ulong *, void *,ulong *, ulong *, struct
tv_range *, ulong);
-static int do_timer_list_v3(ulong, int, ulong *, void *,ulong *, ulong *,
ulong);
+static int do_timer_list_v3(ulong, int, ulong *, void *,ulong *, ulong *,
ulong, long);
struct timer_bases_data;
static int do_timer_list_v4(struct timer_bases_data *, ulong);
static int compare_timer_data(const void *, const void *);
@@ -8376,7 +8376,12 @@ dump_timer_data_tvec_bases_v3(const ulong *cpus)
char buf4[BUFSIZE];
vec_root_size = vec_size = 0;
- head_size = SIZE(hlist_head);
+
+ if (STREQ(MEMBER_TYPE_NAME("tvec_root", "vec"),
"list_head"))
+ /* for RHEL7.6 or later */
+ head_size = SIZE(list_head);
+ else
+ head_size = SIZE(hlist_head);
if ((i = get_array_length("tvec_root.vec", NULL, head_size)))
vec_root_size = i;
@@ -8414,15 +8419,15 @@ next_cpu:
init_tv_ranges(tv, vec_root_size, vec_size, cpu);
count += do_timer_list_v3(tv[1].base + OFFSET(tvec_root_s_vec),
- vec_root_size, vec, NULL, NULL, NULL, 0);
+ vec_root_size, vec, NULL, NULL, NULL, 0, head_size);
count += do_timer_list_v3(tv[2].base + OFFSET(tvec_s_vec),
- vec_size, vec, NULL, NULL, NULL, 0);
+ vec_size, vec, NULL, NULL, NULL, 0, head_size);
count += do_timer_list_v3(tv[3].base + OFFSET(tvec_s_vec),
- vec_size, vec, NULL, NULL, NULL, 0);
+ vec_size, vec, NULL, NULL, NULL, 0, head_size);
count += do_timer_list_v3(tv[4].base + OFFSET(tvec_s_vec),
- vec_size, vec, NULL, NULL, NULL, 0);
+ vec_size, vec, NULL, NULL, NULL, 0, head_size);
count += do_timer_list_v3(tv[5].base + OFFSET(tvec_s_vec),
- vec_size, vec, NULL, NULL, NULL, 0);
+ vec_size, vec, NULL, NULL, NULL, 0, head_size);
if (count)
td = (struct timer_data *)
@@ -8433,16 +8438,16 @@ next_cpu:
get_symbol_data("jiffies", sizeof(ulong), &jiffies);
- do_timer_list_v3(tv[1].base + OFFSET(tvec_root_s_vec),
- vec_root_size, vec, (void *)td, &highest, &highest_tte, jiffies);
- do_timer_list_v3(tv[2].base + OFFSET(tvec_s_vec),
- vec_size, vec, (void *)td, &highest, &highest_tte, jiffies);
- do_timer_list_v3(tv[3].base + OFFSET(tvec_s_vec),
- vec_size, vec, (void *)td, &highest, &highest_tte, jiffies);
- do_timer_list_v3(tv[4].base + OFFSET(tvec_s_vec),
- vec_size, vec, (void *)td, &highest, &highest_tte, jiffies);
- tdx = do_timer_list_v3(tv[5].base + OFFSET(tvec_s_vec),
- vec_size, vec, (void *)td, &highest, &highest_tte, jiffies);
+ do_timer_list_v3(tv[1].base + OFFSET(tvec_root_s_vec), vec_root_size,
+ vec, (void *)td, &highest, &highest_tte, jiffies, head_size);
+ do_timer_list_v3(tv[2].base + OFFSET(tvec_s_vec), vec_size,
+ vec, (void *)td, &highest, &highest_tte, jiffies, head_size);
+ do_timer_list_v3(tv[3].base + OFFSET(tvec_s_vec), vec_size,
+ vec, (void *)td, &highest, &highest_tte, jiffies, head_size);
+ do_timer_list_v3(tv[4].base + OFFSET(tvec_s_vec), vec_size,
+ vec, (void *)td, &highest, &highest_tte, jiffies, head_size);
+ tdx = do_timer_list_v3(tv[5].base + OFFSET(tvec_s_vec), vec_size,
+ vec, (void *)td, &highest, &highest_tte, jiffies, head_size);
qsort(td, tdx, sizeof(struct timer_data), compare_timer_data);
@@ -8786,7 +8791,8 @@ do_timer_list_v3(ulong vec_kvaddr,
void *option,
ulong *highest,
ulong *highest_tte,
- ulong jiffies)
+ ulong jiffies,
+ long head_size)
{
int i, t;
int count, tdx;
@@ -8804,19 +8810,24 @@ do_timer_list_v3(ulong vec_kvaddr,
tdx++;
}
- readmem(vec_kvaddr, KVADDR, vec, SIZE(hlist_head) * size,
+ readmem(vec_kvaddr, KVADDR, vec, head_size * size,
"timer_list vec array", FAULT_ON_ERROR);
ld = &list_data;
timer_list_buf = GETBUF(SIZE(timer_list));
- for (i = count = 0; i < size; i++, vec_kvaddr += SIZE(hlist_head)) {
+ for (i = count = 0; i < size; i++, vec_kvaddr += head_size) {
- if (vec[i] == 0)
- continue;
+ if (head_size == SIZE(list_head)) {
+ if (vec[i*2] == vec_kvaddr)
+ continue;
+ } else {
+ if (vec[i] == 0)
+ continue;
+ }
BZERO(ld, sizeof(struct list_data));
- ld->start = vec[i];
+ ld->start = (head_size == SIZE(list_head)) ? vec[i*2] : vec[i];
ld->list_head_offset = OFFSET(timer_list_entry);
ld->end = vec_kvaddr;
ld->flags = RETURN_ON_LIST_ERROR;
6:54 p.m.
New subject: Fix fill_mem_map_cache() to read page-size-or-less size correctly
Hi Kazu,
I will be out of the office until Monday September 16th. I will check
out your patch when I get back.
Thanks,
Dave
----- Original Message -----
fill_mem_map_cache() intends to read page-size-or-less size if it cannot
read the whole cache size, but it seems it doesn't correctly, and shows
just zero for existing data.
crash> kmem -p 1000
PAGE PHYSICAL MAPPING INDEX CNT FLAGS
ffffea0000000040 1000 0 0 0 0
crash> rd ffffea0000000040
ffffea0000000040: 000fffff00000400 ........
I think the size below should be cnt, and confirmed that the patch works
well with a dump by makedumpfile -e option.
Signed-off-by: Kazuhito Hagio <k-hagio(a)ab.jp.nec.com>
---
memory.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/memory.c b/memory.c
index ac7e73679405..4584673ed0ae 100644
--- a/memory.c
+++ b/memory.c
@@ -6348,13 +6348,13 @@ fill_mem_map_cache(ulong pp, ulong ppend, char *page_cache)
if (cnt > size)
cnt = size;
- if (!readmem(addr, KVADDR, bufptr, size,
+ if (!readmem(addr, KVADDR, bufptr, cnt,
"virtual page struct cache", RETURN_ON_ERROR|QUIET)) {
- BZERO(bufptr, size);
- if (!(vt->flags & V_MEM_MAP) && ((addr+size) <
ppend))
+ BZERO(bufptr, cnt);
+ if (!(vt->flags & V_MEM_MAP) && ((addr+cnt) <
ppend))
error(WARNING,
"mem_map[] from %lx to %lx not
accessible\n",
- addr, addr+size);
+ addr, addr+cnt);
}
addr += cnt;
--
2.18.1
1:40 p.m.
New subject: Fix fill_mem_map_cache() to read page-size-or-less size correctly
Hi Kazu,
This seems to be an extrememly rare condition, but in any case, your
patch is the correct thing to do.
However, I do have two dumpfiles where the change causes the warning message
to be displayed unnecessarily, which was only meant to be displayed if the
mem_map cache is *not* virtually-mapped. But the single check for
!(vt->flags & V_MEM_MAP) is not always sufficient, so it should also
check for !(machdep->flags & VMEMMAP) like so:
--- a/memory.c 2019-09-17 14:20:24.586069004 -0400
+++ b/memory.c 2019-09-17 14:36:36.765286602 -0400
@@ -6348,13 +6348,13 @@ fill_mem_map_cache(ulong pp, ulong ppend
if (cnt > size)
cnt = size;
- if (!readmem(addr, KVADDR, bufptr, size,
+ if (!readmem(addr, KVADDR, bufptr, cnt,
"virtual page struct cache", RETURN_ON_ERROR|QUIET)) {
- BZERO(bufptr, size);
- if (!(vt->flags & V_MEM_MAP) && ((addr+size) < ppend))
+ BZERO(bufptr, cnt);
+ if (!((vt->flags & V_MEM_MAP) || (machdep->flags & VMEMMAP)) &&
((addr+cnt) < ppend))
error(WARNING,
"mem_map[] from %lx to %lx not accessible\n",
- addr, addr+size);
+ addr, addr+cnt);
}
addr += cnt;
You OK with that?
Thanks,
Dave
----- Original Message -----
Hi Kazu,
I will be out of the office until Monday September 16th. I will check
out your patch when I get back.
Thanks,
Dave
----- Original Message -----
>
> fill_mem_map_cache() intends to read page-size-or-less size if it cannot
> read the whole cache size, but it seems it doesn't correctly, and shows
> just zero for existing data.
>
> crash> kmem -p 1000
> PAGE PHYSICAL MAPPING INDEX CNT FLAGS
> ffffea0000000040 1000 0 0 0 0
> crash> rd ffffea0000000040
> ffffea0000000040: 000fffff00000400 ........
>
> I think the size below should be cnt, and confirmed that the patch works
> well with a dump by makedumpfile -e option.
>
> Signed-off-by: Kazuhito Hagio <k-hagio(a)ab.jp.nec.com>
> ---
> memory.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/memory.c b/memory.c
> index ac7e73679405..4584673ed0ae 100644
> --- a/memory.c
> +++ b/memory.c
> @@ -6348,13 +6348,13 @@ fill_mem_map_cache(ulong pp, ulong ppend, char
> *page_cache)
> if (cnt > size)
> cnt = size;
>
> - if (!readmem(addr, KVADDR, bufptr, size,
> + if (!readmem(addr, KVADDR, bufptr, cnt,
> "virtual page struct cache", RETURN_ON_ERROR|QUIET))
{
> - BZERO(bufptr, size);
> - if (!(vt->flags & V_MEM_MAP) && ((addr+size)
<
> ppend))
> + BZERO(bufptr, cnt);
> + if (!(vt->flags & V_MEM_MAP) && ((addr+cnt)
<
> ppend))
> error(WARNING,
> "mem_map[] from %lx to %lx not
> accessible\n",
> - addr, addr+size);
> + addr, addr+cnt);
> }
>
> addr += cnt;
> --
> 2.18.1
>
--
Crash-utility mailing list
Crash-utility(a)redhat.com
https://www.redhat.com/mailman/listinfo/crash-utility
3:16 p.m.
New subject: Fix fill_mem_map_cache() to read page-size-or-less size correctly
Hi Dave,
On 9/17/2019 2:40 PM, Dave Anderson wrote:
Hi Kazu,
This seems to be an extrememly rare condition, but in any case, your
patch is the correct thing to do.
However, I do have two dumpfiles where the change causes the warning message
to be displayed unnecessarily, which was only meant to be displayed if the
mem_map cache is *not* virtually-mapped. But the single check for
!(vt->flags & V_MEM_MAP) is not always sufficient, so it should also
check for !(machdep->flags & VMEMMAP) like so:
--- a/memory.c 2019-09-17 14:20:24.586069004 -0400
+++ b/memory.c 2019-09-17 14:36:36.765286602 -0400
@@ -6348,13 +6348,13 @@ fill_mem_map_cache(ulong pp, ulong ppend
if (cnt > size)
cnt = size;
- if (!readmem(addr, KVADDR, bufptr, size,
+ if (!readmem(addr, KVADDR, bufptr, cnt,
"virtual page struct cache", RETURN_ON_ERROR|QUIET)) {
- BZERO(bufptr, size);
- if (!(vt->flags & V_MEM_MAP) && ((addr+size) < ppend))
+ BZERO(bufptr, cnt);
+ if (!((vt->flags & V_MEM_MAP) || (machdep->flags & VMEMMAP)) &&
((addr+cnt) < ppend))
error(WARNING,
"mem_map[] from %lx to %lx not accessible\n",
- addr, addr+size);
+ addr, addr+cnt);
}
addr += cnt;
You OK with that?
I'm OK, thanks!
Kazu
Thanks,
Dave
----- Original Message -----
> Hi Kazu,
>
> I will be out of the office until Monday September 16th. I will check
> out your patch when I get back.
>
> Thanks,
> Dave
>
>
> ----- Original Message -----
>>
>> fill_mem_map_cache() intends to read page-size-or-less size if it cannot
>> read the whole cache size, but it seems it doesn't correctly, and shows
>> just zero for existing data.
>>
>> crash> kmem -p 1000
>> PAGE PHYSICAL MAPPING INDEX CNT FLAGS
>> ffffea0000000040 1000 0 0 0 0
>> crash> rd ffffea0000000040
>> ffffea0000000040: 000fffff00000400 ........
>>
>> I think the size below should be cnt, and confirmed that the patch works
>> well with a dump by makedumpfile -e option.
>>
>> Signed-off-by: Kazuhito Hagio <k-hagio(a)ab.jp.nec.com>
>> ---
>> memory.c | 8 ++++----
>> 1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/memory.c b/memory.c
>> index ac7e73679405..4584673ed0ae 100644
>> --- a/memory.c
>> +++ b/memory.c
>> @@ -6348,13 +6348,13 @@ fill_mem_map_cache(ulong pp, ulong ppend, char
>> *page_cache)
>> if (cnt > size)
>> cnt = size;
>>
>> - if (!readmem(addr, KVADDR, bufptr, size,
>> + if (!readmem(addr, KVADDR, bufptr, cnt,
>> "virtual page struct cache",
RETURN_ON_ERROR|QUIET)) {
>> - BZERO(bufptr, size);
>> - if (!(vt->flags & V_MEM_MAP) &&
((addr+size) <
>> ppend))
>> + BZERO(bufptr, cnt);
>> + if (!(vt->flags & V_MEM_MAP) &&
((addr+cnt) <
>> ppend))
>> error(WARNING,
>> "mem_map[] from %lx to %lx not
>> accessible\n",
>> - addr, addr+size);
>> + addr, addr+cnt);
>> }
>>
>> addr += cnt;
>> --
>> 2.18.1
>>
>
> --
> Crash-utility mailing list
> Crash-utility(a)redhat.com
> https://www.redhat.com/mailman/listinfo/crash-utility
>
--
Crash-utility mailing list
Crash-utility(a)redhat.com
https://www.redhat.com/mailman/listinfo/crash-utility
3:29 p.m.
New subject: Fix fill_mem_map_cache() to read page-size-or-less size correctly
----- Original Message -----
Hi Dave,
On 9/17/2019 2:40 PM, Dave Anderson wrote:
>
> Hi Kazu,
>
> This seems to be an extrememly rare condition, but in any case, your
> patch is the correct thing to do.
>
> However, I do have two dumpfiles where the change causes the warning message
> to be displayed unnecessarily, which was only meant to be displayed if the
> mem_map cache is *not* virtually-mapped. But the single check for
> !(vt->flags & V_MEM_MAP) is not always sufficient, so it should also
> check for !(machdep->flags & VMEMMAP) like so:
>
> --- a/memory.c 2019-09-17 14:20:24.586069004 -0400
> +++ b/memory.c 2019-09-17 14:36:36.765286602 -0400
> @@ -6348,13 +6348,13 @@ fill_mem_map_cache(ulong pp, ulong ppend
> if (cnt > size)
> cnt = size;
>
> - if (!readmem(addr, KVADDR, bufptr, size,
> + if (!readmem(addr, KVADDR, bufptr, cnt,
> "virtual page struct cache", RETURN_ON_ERROR|QUIET))
{
> - BZERO(bufptr, size);
> - if (!(vt->flags & V_MEM_MAP) && ((addr+size) < ppend))
> + BZERO(bufptr, cnt);
> + if (!((vt->flags & V_MEM_MAP) || (machdep->flags & VMEMMAP))
&& ((addr+cnt) < ppend))
> error(WARNING,
> "mem_map[] from %lx to %lx not accessible\n",
> - addr, addr+size);
> + addr, addr+cnt);
> }
>
> addr += cnt;
>
> You OK with that?
I'm OK, thanks!
Kazu
OK good -- it's queued for crash-7.2.7:
https://github.com/crash-utility/crash/commit/5d43b10b6e12a931ad6e469fe7f...
I'm hoping to release 7.2.7 by the end of this week.
Thanks,
Dave
2089
days inactive
2136
days old
devel@lists.crash-utility.osci.io
8 comments
2 participants
participants (2)
-
Dave Anderson -
Kazuhito Hagio