11:28 a.m.
Hi Dave,
On s390 we will have a dump method that creates live dumps, similar to
the snap.so crash plugin. Because Linux is not stopped while the dump
is created, the resulting dump is not consistent. Therefore it is
important that the crash tool informs the user about this issue.
The dump tool writes a magic number (ASCII "LIVEDUMP") into the first 8
bytes of the dump memory. With this patch this is checked in POST_INIT
by the s390x backend crash code. If the magic is found, the LIVE_SYSTEM
flag is set. This ensures that commands that do not work with /dev/mem
also will fail with s390x live dumps.
Example:
crash> bt -a
bt: -a option not supported on a live system
In addition to that with this patch crash prints a "[LIVE DUMP]" info
tag for live dump files at startup (similar to [PARTIAL DUMP]):
$ crash livedump vmlinux
KERNEL: /boot/vmlinux
DUMPFILE: dump.s390 [LIVE DUMP]
Michael
---
kernel.c | 8 ++++++--
s390x.c | 12 ++++++++++++
2 files changed, 18 insertions(+), 2 deletions(-)
--- a/kernel.c
+++ b/kernel.c
@@ -975,7 +975,9 @@ non_matching_kernel(void)
else
fprintf(fp, " DUMPFILE: ");
if (ACTIVE()) {
- if (REMOTE_ACTIVE())
+ if (pc->dumpfile)
+ fprintf(fp, "%s [LIVE DUMP]\n", pc->dumpfile);
+ else if (REMOTE_ACTIVE())
fprintf(fp, "%s@%s (remote live system)\n",
pc->server_memsrc, pc->server);
else
@@ -4080,7 +4082,9 @@ display_sys_stats(void)
else
fprintf(fp, " DUMPFILE: ");
if (ACTIVE()) {
- if (REMOTE_ACTIVE())
+ if (pc->dumpfile)
+ fprintf(fp, "%s [LIVE DUMP]\n", pc->dumpfile);
+ else if (REMOTE_ACTIVE())
fprintf(fp, "%s@%s (remote live system)\n",
pc->server_memsrc, pc->server);
else
--- a/s390x.c
+++ b/s390x.c
@@ -328,6 +328,17 @@ static void s390x_process_elf_notes(void
}
}
+static void s390x_check_live(void)
+{
+ unsigned long long live_magic;
+
+ readmem(0, KVADDR, &live_magic, sizeof(live_magic),
"live_magic",
+ RETURN_ON_ERROR | QUIET);
+
+ if (live_magic == 0x4c49564544554d50ULL)
+ pc->flags |= LIVE_SYSTEM;
+}
+
/*
* Do all necessary machine-specific setup here. This is called
several
* times during initialization.
@@ -402,6 +413,7 @@ s390x_init(int when)
break;
case POST_INIT:
+ s390x_check_live();
break;
}
}
1:44 p.m.
----- Original Message -----
Hi Dave,
On s390 we will have a dump method that creates live dumps, similar to
the snap.so crash plugin. Because Linux is not stopped while the dump
is created, the resulting dump is not consistent. Therefore it is
important that the crash tool informs the user about this issue.
The dump tool writes a magic number (ASCII "LIVEDUMP") into the first 8
bytes of the dump memory. With this patch this is checked in POST_INIT
by the s390x backend crash code. If the magic is found, the LIVE_SYSTEM
flag is set. This ensures that commands that do not work with /dev/mem
also will fail with s390x live dumps.
Example:
crash> bt -a
bt: -a option not supported on a live system
In addition to that with this patch crash prints a "[LIVE DUMP]" info
tag for live dump files at startup (similar to [PARTIAL DUMP]):
$ crash livedump vmlinux
KERNEL: /boot/vmlinux
DUMPFILE: dump.s390 [LIVE DUMP]
Michael
Interesting -- I'm amazed that even doing such a thing works!
Anyway, if you look at all the places where ACTIVE() is called,
this patch, at a minimum, would be kind of inefficient.
For example, consider that -- after each command -- the complete
task table would get re-initialized (as opposed to doing a single
invocation-time initialization from a dumpfile). That's not a big
deal when reading RAM, but it could be costly when having to
read from the dumpfile each time. But there are numerous places
where a similar tact is taken to avoid unnecessary dumpfile accesses
if the data can be cached.
I would have no problem with adding a new LIVE_DUMP flag to
pc->flags2, and just checking it in display_sys_stats() and
non_matching_kernel() as you've done below. In dealing with
dumpfiles generated from snap.so, the "bt" command is pretty
much the only command that probably should be restricted.
However, I don't restrict "bt" with snap.so vmcores because
currently there's no magic/signature/whatever that indicates
what kind of dump it is. But if you implement a new LIVE_DUMP
flag, I might do it there as well so we've got some consistency.
What do you think about that?
Dave
---
kernel.c | 8 ++++++--
s390x.c | 12 ++++++++++++
2 files changed, 18 insertions(+), 2 deletions(-)
--- a/kernel.c
+++ b/kernel.c
@@ -975,7 +975,9 @@ non_matching_kernel(void)
else
fprintf(fp, " DUMPFILE: ");
if (ACTIVE()) {
- if (REMOTE_ACTIVE())
+ if (pc->dumpfile)
+ fprintf(fp, "%s [LIVE DUMP]\n", pc->dumpfile);
+ else if (REMOTE_ACTIVE())
fprintf(fp, "%s@%s (remote live system)\n",
pc->server_memsrc, pc->server);
else
@@ -4080,7 +4082,9 @@ display_sys_stats(void)
else
fprintf(fp, " DUMPFILE: ");
if (ACTIVE()) {
- if (REMOTE_ACTIVE())
+ if (pc->dumpfile)
+ fprintf(fp, "%s [LIVE DUMP]\n", pc->dumpfile);
+ else if (REMOTE_ACTIVE())
fprintf(fp, "%s@%s (remote live system)\n",
pc->server_memsrc, pc->server);
else
--- a/s390x.c
+++ b/s390x.c
@@ -328,6 +328,17 @@ static void s390x_process_elf_notes(void
}
}
+static void s390x_check_live(void)
j +{
+ unsigned long long live_magic;
+
+ readmem(0, KVADDR, &live_magic, sizeof(live_magic),
"live_magic",
+ RETURN_ON_ERROR | QUIET);
+
+ if (live_magic == 0x4c49564544554d50ULL)
+ pc->flags |= LIVE_SYSTEM;
+}
+
/*
* Do all necessary machine-specific setup here. This is called
several
* times during initialization.
@@ -402,6 +413,7 @@ s390x_init(int when)
break;
case POST_INIT:
+ s390x_check_live();
break;
}
}
4:02 a.m.
Hello Dave,
On Thu, 19 Apr 2012 14:44:43 -0400 (EDT)
Dave Anderson <anderson(a)redhat.com> wrote:
[snip]
I would have no problem with adding a new LIVE_DUMP flag to
pc->flags2, and just checking it in display_sys_stats() and
non_matching_kernel() as you've done below. In dealing with
dumpfiles generated from snap.so, the "bt" command is pretty
much the only command that probably should be restricted.
However, I don't restrict "bt" with snap.so vmcores because
currently there's no magic/signature/whatever that indicates
what kind of dump it is. But if you implement a new LIVE_DUMP
flag, I might do it there as well so we've got some consistency.
What do you think about that?
Sounds good. So what about introducing a new macro LIVE() that
indicates that the dump or live system is inconsistent.
Something like the following:
---
defs.h | 2 ++
kernel.c | 9 +++++++--
main.c | 2 ++
s390x.c | 12 ++++++++++++
4 files changed, 23 insertions(+), 2 deletions(-)
--- a/defs.h
+++ b/defs.h
@@ -203,6 +203,7 @@ struct number_option {
#define ACTIVE() (pc->flags & LIVE_SYSTEM)
#define DUMPFILE() (!(pc->flags & LIVE_SYSTEM))
+#define LIVE() (pc->flags2 & LIVE_DUMP || pc->flags &
LIVE_SYSTEM)
#define MEMORY_SOURCES
(NETDUMP|KDUMP|MCLXCD|LKCD|DEVMEM|S390D|MEMMOD|DISKDUMP|XENDUMP|CRASHBUILTIN|KVMDUMP|PROC_KCORE|SADUMP)
#define DUMPFILE_TYPES
(DISKDUMP|NETDUMP|KDUMP|MCLXCD|LKCD|S390D|XENDUMP|KVMDUMP|SADUMP)
#define REMOTE() (pc->flags2 & REMOTE_DAEMON)
@@ -446,6 +447,7 @@ struct program_context {
#define REMOTE_DAEMON (0x08ULL)
#define ERASEINFO_DATA (0x10ULL)
#define GDB_CMD_MODE (0x20ULL)
+#define LIVE_DUMP (0x40ULL)
#define FLAT_FORMAT() (pc->flags2 & FLAT)
#define ELF_NOTES_VALID() (pc->flags2 & ELF_NOTES)
char *cleanup;
--- a/kernel.c
+++ b/kernel.c
@@ -992,6 +992,8 @@ non_matching_kernel(void)
else
fprintf(fp, "%s", pc->dumpfile);
}
+ if (LIVE())
+ fprintf(fp, " [LIVE DUMP]");
}
fprintf(fp, "\n\n");
@@ -2072,9 +2074,9 @@ else
}
if (active) {
- if (ACTIVE())
+ if (LIVE())
error(FATAL,
- "-a option not supported on a live system\n");
+ "-a option not supported on a live system or live dump\n");
if (bt->flags & BT_THREAD_GROUP)
error(FATAL,
@@ -4098,6 +4100,9 @@ display_sys_stats(void)
fprintf(fp, "%s", pc->dumpfile);
}
+ if (LIVE())
+ fprintf(fp, " [LIVE DUMP]");
+
if (NETDUMP_DUMPFILE() && is_partial_netdump())
fprintf(fp, " [PARTIAL DUMP]");
--- a/main.c
+++ b/main.c
@@ -1110,6 +1110,8 @@ dump_program_context(void)
if (pc->flags & LIVE_SYSTEM)
sprintf(&buf[strlen(buf)], "%sLIVE_SYSTEM",
others++ ? "|" : "");
+ if (pc->flags2 & LIVE_DUMP)
+ sprintf(&buf[strlen(buf)], "%sLIVE_DUMP", others++ ? "|" :
"");
if (pc->flags & TTY)
sprintf(&buf[strlen(buf)], "%sTTY", others++ ? "|" :
"");
if (pc->flags & IN_FOREACH)
--- a/s390x.c
+++ b/s390x.c
@@ -328,6 +328,17 @@ static void s390x_process_elf_notes(void
}
}
+static void s390x_check_live(void)
+{
+ unsigned long long live_magic;
+
+ readmem(0, KVADDR, &live_magic, sizeof(live_magic), "live_magic",
+ RETURN_ON_ERROR | QUIET);
+
+ if (live_magic == 0x4c49564544554d50ULL)
+ pc->flags2 |= LIVE_DUMP;
+}
+
/*
* Do all necessary machine-specific setup here. This is called several
* times during initialization.
@@ -402,6 +413,7 @@ s390x_init(int when)
break;
case POST_INIT:
+ s390x_check_live();
break;
}
}
9:03 a.m.
----- Original Message -----
Hello Dave,
On Thu, 19 Apr 2012 14:44:43 -0400 (EDT)
Dave Anderson <anderson(a)redhat.com> wrote:
[snip]
> I would have no problem with adding a new LIVE_DUMP flag to
> pc->flags2, and just checking it in display_sys_stats() and
> non_matching_kernel() as you've done below. In dealing with
> dumpfiles generated from snap.so, the "bt" command is pretty
> much the only command that probably should be restricted.
> However, I don't restrict "bt" with snap.so vmcores because
> currently there's no magic/signature/whatever that indicates
> what kind of dump it is. But if you implement a new LIVE_DUMP
> flag, I might do it there as well so we've got some consistency.
>
> What do you think about that?
Sounds good. So what about introducing a new macro LIVE() that
indicates that the dump or live system is inconsistent.
Good idea -- queued for crash-6.0.6.
I'll also put some kind of indicator in the snap.so ELF header.
Thanks,
Dave
Something like the following:
---
defs.h | 2 ++
kernel.c | 9 +++++++--
main.c | 2 ++
s390x.c | 12 ++++++++++++
4 files changed, 23 insertions(+), 2 deletions(-)
--- a/defs.h
+++ b/defs.h
@@ -203,6 +203,7 @@ struct number_option {
#define ACTIVE() (pc->flags & LIVE_SYSTEM)
#define DUMPFILE() (!(pc->flags & LIVE_SYSTEM))
+#define LIVE() (pc->flags2 & LIVE_DUMP || pc->flags &
LIVE_SYSTEM)
#define MEMORY_SOURCES
(NETDUMP|KDUMP|MCLXCD|LKCD|DEVMEM|S390D|MEMMOD|DISKDUMP|XENDUMP|CRASHBUILTIN|KVMDUMP|PROC_KCORE|SADUMP)
#define DUMPFILE_TYPES
(DISKDUMP|NETDUMP|KDUMP|MCLXCD|LKCD|S390D|XENDUMP|KVMDUMP|SADUMP)
#define REMOTE() (pc->flags2 & REMOTE_DAEMON)
@@ -446,6 +447,7 @@ struct program_context {
#define REMOTE_DAEMON (0x08ULL)
#define ERASEINFO_DATA (0x10ULL)
#define GDB_CMD_MODE (0x20ULL)
+#define LIVE_DUMP (0x40ULL)
#define FLAT_FORMAT() (pc->flags2 & FLAT)
#define ELF_NOTES_VALID() (pc->flags2 & ELF_NOTES)
char *cleanup;
--- a/kernel.c
+++ b/kernel.c
@@ -992,6 +992,8 @@ non_matching_kernel(void)
else
fprintf(fp, "%s", pc->dumpfile);
}
+ if (LIVE())
+ fprintf(fp, " [LIVE DUMP]");
}
fprintf(fp, "\n\n");
@@ -2072,9 +2074,9 @@ else
}
if (active) {
- if (ACTIVE())
+ if (LIVE())
error(FATAL,
- "-a option not supported on a live system\n");
+ "-a option not supported on a live system or live dump\n");
if (bt->flags & BT_THREAD_GROUP)
error(FATAL,
@@ -4098,6 +4100,9 @@ display_sys_stats(void)
fprintf(fp, "%s", pc->dumpfile);
}
+ if (LIVE())
+ fprintf(fp, " [LIVE DUMP]");
+
if (NETDUMP_DUMPFILE() && is_partial_netdump())
fprintf(fp, " [PARTIAL DUMP]");
--- a/main.c
+++ b/main.c
@@ -1110,6 +1110,8 @@ dump_program_context(void)
if (pc->flags & LIVE_SYSTEM)
sprintf(&buf[strlen(buf)], "%sLIVE_SYSTEM",
others++ ? "|" : "");
+ if (pc->flags2 & LIVE_DUMP)
+ sprintf(&buf[strlen(buf)], "%sLIVE_DUMP", others++ ? "|" :
"");
if (pc->flags & TTY)
sprintf(&buf[strlen(buf)], "%sTTY", others++ ? "|" :
"");
if (pc->flags & IN_FOREACH)
--- a/s390x.c
+++ b/s390x.c
@@ -328,6 +328,17 @@ static void s390x_process_elf_notes(void
}
}
+static void s390x_check_live(void)
+{
+ unsigned long long live_magic;
+
+ readmem(0, KVADDR, &live_magic, sizeof(live_magic), "live_magic",
+ RETURN_ON_ERROR | QUIET);
+
+ if (live_magic == 0x4c49564544554d50ULL)
+ pc->flags2 |= LIVE_DUMP;
+}
+
/*
* Do all necessary machine-specific setup here. This is called
several
* times during initialization.
@@ -402,6 +413,7 @@ s390x_init(int when)
break;
case POST_INIT:
+ s390x_check_live();
break;
}
}
3:54 a.m.
Hi Dave,
On Fri, 20 Apr 2012 10:03:23 -0400 (EDT)
Dave Anderson <anderson(a)redhat.com> wrote:
----- Original Message -----
> Hello Dave,
>
> On Thu, 19 Apr 2012 14:44:43 -0400 (EDT)
> Dave Anderson <anderson(a)redhat.com> wrote:
>
> [snip]
>
> > I would have no problem with adding a new LIVE_DUMP flag to
> > pc->flags2, and just checking it in display_sys_stats() and
> > non_matching_kernel() as you've done below. In dealing with
> > dumpfiles generated from snap.so, the "bt" command is pretty
> > much the only command that probably should be restricted.
> > However, I don't restrict "bt" with snap.so vmcores because
> > currently there's no magic/signature/whatever that indicates
> > what kind of dump it is. But if you implement a new LIVE_DUMP
> > flag, I might do it there as well so we've got some consistency.
> >
> > What do you think about that?
>
> Sounds good. So what about introducing a new macro LIVE() that
> indicates that the dump or live system is inconsistent.
Good idea -- queued for crash-6.0.6.
Thanks!
Michael
4:19 a.m.
Hi Dave,
On Fri, 20 Apr 2012 10:03:23 -0400 (EDT)
Dave Anderson <anderson(a)redhat.com> wrote:
> Sounds good. So what about introducing a new macro LIVE() that
> indicates that the dump or live system is inconsistent.
Good idea -- queued for crash-6.0.6.
There is another ACTIVE() call in s390x.c that should be changed to
LIVE(). When issuing "bt" on a running task on a live system, we
currently write "(active)".
This should also be done for live dumps:
---
s390x.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
--- a/s390x.c
+++ b/s390x.c
@@ -1092,7 +1092,7 @@ static void s390x_back_trace_cmd(struct
* Print lowcore and print interrupt stacks when task has cpu
*/
if (s390x_has_cpu(bt)) {
- if (ACTIVE()) {
+ if (LIVE()) {
fprintf(fp,"(active)\n");
return;
}
7:50 a.m.
----- Original Message -----
Hi Dave,
On Fri, 20 Apr 2012 10:03:23 -0400 (EDT)
Dave Anderson <anderson(a)redhat.com> wrote:
> > Sounds good. So what about introducing a new macro LIVE() that
> > indicates that the dump or live system is inconsistent.
>
> Good idea -- queued for crash-6.0.6.
There is another ACTIVE() call in s390x.c that should be changed to
LIVE(). When issuing "bt" on a running task on a live system, we
currently write "(active)".
This should also be done for live dumps:
Question: how does that code path ever get run?
If you try a "bt" on an active task on a live system, it would
print the "(active)" here in back_trace(), prior to calling into
the machine-specific backtrace function:
if (ACTIVE() && !(bt->flags & BT_EFRAME_SEARCH) &&
((bt->task == tt->this_task) || is_task_active(bt->task))) {
if (BT_REFERENCE_CHECK(bt) ||
bt->flags & (BT_TEXT_SYMBOLS_PRINT|BT_TEXT_SYMBOLS_NOPRINT))
return;
if (!(bt->flags &
(BT_KSTACKP|BT_TEXT_SYMBOLS|BT_TEXT_SYMBOLS_ALL)))
fprintf(fp, "(active)\n");
if (!(bt->flags & (BT_TEXT_SYMBOLS|BT_TEXT_SYMBOLS_ALL)))
return;
}
Note the "bt -[tT]" options should be allowed even if the task is active.
Dave
---
s390x.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
--- a/s390x.c
+++ b/s390x.c
@@ -1092,7 +1092,7 @@ static void s390x_back_trace_cmd(struct
* Print lowcore and print interrupt stacks when task has cpu
*/
if (s390x_has_cpu(bt)) {
- if (ACTIVE()) {
+ if (LIVE()) {
fprintf(fp,"(active)\n");
return;
}
10:55 a.m.
Hello Dave,
On Mon, 23 Apr 2012 08:50:02 -0400 (EDT)
Dave Anderson <anderson(a)redhat.com> wrote:
[snip]
Question: how does that code path ever get run?
Interesting...
I only tested the patch on a live dump and not on the live system
with /dev/mem. Therefore I did not see that. On a live system, as you
described it, the s390x.c code is not used.
If you try a "bt" on an active task on a live system, it would
print the "(active)" here in back_trace(), prior to calling into
the machine-specific backtrace function:
if (ACTIVE() && !(bt->flags & BT_EFRAME_SEARCH) &&
((bt->task == tt->this_task) ||
is_task_active(bt->task))) {
if (BT_REFERENCE_CHECK(bt) ||
bt->flags &
(BT_TEXT_SYMBOLS_PRINT|BT_TEXT_SYMBOLS_NOPRINT)) return;
if (!(bt->flags &
(BT_KSTACKP|BT_TEXT_SYMBOLS|BT_TEXT_SYMBOLS_ALL)))
fprintf(fp, "(active)\n");
if (!(bt->flags &
(BT_TEXT_SYMBOLS|BT_TEXT_SYMBOLS_ALL))) return;
}
Note the "bt -[tT]" options should be allowed even if the task is
active.
So the check should be done in kernel.c and not in s390x.c.
Would the following patch be better?
---
kernel.c | 2 +-
s390x.c | 4 ----
2 files changed, 1 insertion(+), 5 deletions(-)
--- a/kernel.c
+++ b/kernel.c
@@ -2245,7 +2245,7 @@ back_trace(struct bt_info *bt)
return;
}
- if (ACTIVE() && !(bt->flags & BT_EFRAME_SEARCH) &&
+ if (LIVE() && !(bt->flags & BT_EFRAME_SEARCH) &&
((bt->task == tt->this_task) || is_task_active(bt->task))) {
if (BT_REFERENCE_CHECK(bt) ||
--- a/s390x.c
+++ b/s390x.c
@@ -1092,10 +1092,6 @@ static void s390x_back_trace_cmd(struct
* Print lowcore and print interrupt stacks when task has cpu
*/
if (s390x_has_cpu(bt)) {
- if (ACTIVE()) {
- fprintf(fp,"(active)\n");
- return;
- }
s390x_get_lowcore(bt, lowcore);
psw_flags = ULONG(lowcore + OFFSET(s390_lowcore_psw_save_area));
11:06 a.m.
----- Original Message -----
Hello Dave,
On Mon, 23 Apr 2012 08:50:02 -0400 (EDT)
Dave Anderson <anderson(a)redhat.com> wrote:
[snip]
>
>
> Question: how does that code path ever get run?
Interesting...
I only tested the patch on a live dump and not on the live system
with /dev/mem. Therefore I did not see that. On a live system, as you
described it, the s390x.c code is not used.
>
> If you try a "bt" on an active task on a live system, it would
> print the "(active)" here in back_trace(), prior to calling into
> the machine-specific backtrace function:
>
> if (ACTIVE() && !(bt->flags & BT_EFRAME_SEARCH) &&
> ((bt->task == tt->this_task) || is_task_active(bt->task))) {
>
> if (BT_REFERENCE_CHECK(bt) ||
> bt->flags &
(BT_TEXT_SYMBOLS_PRINT|BT_TEXT_SYMBOLS_NOPRINT))
> return;
>
> if (!(bt->flags &
> (BT_KSTACKP|BT_TEXT_SYMBOLS|BT_TEXT_SYMBOLS_ALL)))
> fprintf(fp, "(active)\n");
>
> if (!(bt->flags & (BT_TEXT_SYMBOLS|BT_TEXT_SYMBOLS_ALL)))
> return;
> }
>
> Note the "bt -[tT]" options should be allowed even if the task is active.
So the check should be done in kernel.c and not in s390x.c.
Would the following patch be better?
Right, that's better -- queued for crash-6.0.6.
Thanks,
Dave
---
kernel.c | 2 +-
s390x.c | 4 ----
2 files changed, 1 insertion(+), 5 deletions(-)
--- a/kernel.c
+++ b/kernel.c
@@ -2245,7 +2245,7 @@ back_trace(struct bt_info *bt)
return;
}
- if (ACTIVE() && !(bt->flags & BT_EFRAME_SEARCH) &&
+ if (LIVE() && !(bt->flags & BT_EFRAME_SEARCH) &&
((bt->task == tt->this_task) ||
is_task_active(bt->task))) {
if (BT_REFERENCE_CHECK(bt) ||
--- a/s390x.c
+++ b/s390x.c
@@ -1092,10 +1092,6 @@ static void s390x_back_trace_cmd(struct
* Print lowcore and print interrupt stacks when task has cpu
*/
if (s390x_has_cpu(bt)) {
- if (ACTIVE()) {
- fprintf(fp,"(active)\n");
- return;
- }
s390x_get_lowcore(bt, lowcore);
psw_flags = ULONG(lowcore + OFFSET(s390_lowcore_psw_save_area));
4943
days inactive
4947
days old
devel@lists.crash-utility.osci.io
8 comments
2 participants
participants (2)
-
Dave Anderson -
Michael Holzheu