3:52 a.m.
Hello Dave,
New kernel has moved some elements of struct vfsmount to a new struct
mount. So crash will not act normally on new kernel. The patch is used
to fix the problem.
Please check.
--
--
Regards
Qiao Nuohan
--------------------------------------------------
Qiao Nuohan
Development Dept.I
Nanjing Fujitsu Nanda Software Tech. Co., Ltd.(FNST)
No. 6 Wenzhu Road, Nanjing, Nanjing, 210012, China
TEL: +86+25-86630566-8526 FUJITSU
INTERNAL: 7998-8526 FAX: +86+25-83317685
EMail: qiaonuohan(a)cn.fujitsu.com
--------------------------------------------------
This communication is for use by the intended
recipient(s) only and may contain information that
is privileged, confidential and exempt from
disclosure under applicable law. If you are not an
intended recipient of this communication, you are
hereby notified that any dissemination,
distribution or copying hereof is strictly
prohibited. If you have received this communication
in error, please notify me by reply e-mail,
permanently delete this communication from your
system, and destroy any hard copies you may have
printed
10:50 a.m.
----- Original Message -----
Hello Dave,
New kernel has moved some elements of struct vfsmount to a new struct
mount. So crash will not act normally on new kernel. The patch is used
to fix the problem.
Please check.
Hi Qiao,
This patch-set is very much appreciated. I saw Al Viro's original LKML patch
posted back in December (?), and had been waiting to see an actual kernel with
it in place. The most recent in-house RHEL/Fedora test kernels I have on-hand
are all 3.2.x-era kernels, which don't have the patch, so I'm presuming
you're
running on a 3.3.x kernel?
Anyway, I'll check this patch out for backwards-compatibility and try to
get a more recent kernel to test with. It looks good on paper...
And again, thanks for taking on this task.
Dave
3:50 p.m.
----- Original Message -----
----- Original Message -----
> Hello Dave,
>
> New kernel has moved some elements of struct vfsmount to a new struct
> mount. So crash will not act normally on new kernel. The patch is used
> to fix the problem.
>
> Please check.
Hi Qiao,
This patch-set is very much appreciated. I saw Al Viro's original LKML patch
posted back in December (?), and had been waiting to see an actual kernel with
it in place. The most recent in-house RHEL/Fedora test kernels I have on-hand
are all 3.2.x-era kernels, which don't have the patch, so I'm presuming
you're
running on a 3.3.x kernel?
Anyway, I'll check this patch out for backwards-compatibility and try to
get a more recent kernel to test with. It looks good on paper...
And again, thanks for taking on this task.
Dave
I provisioned a system with a Fedora 3.3.0-0.rc6.git2.1.fc17 kernel, which
has the vfsmount->mount patch. The session comes up cleanly, but the
following commands fail:
crash> mount
... [ cut ] ...
mount: invalid structure member offset: vfsmount_mnt_list
...
crash> files
... [cut] ...
files: invalid structure member offset: dentry_d_covers
...
The "fuser" command generates the above error because it uses the
"files" command behind the scenes.
crash> vm
... [cut] ...
vm: invalid structure member offset: dentry_d_covers
...
crash> swap
... [cut] ...
swap: invalid structure member offset: dentry_d_covers
...
With your patch applied, "mount", "files", "fuser" and
"vm" all
work OK.
But "swap" now fails in a different manner:
crash> swap
FILENAME TYPE SIZE USED PCT PRIORITY
swap: invalid kernel virtual address: 1d8ec8 type: "fill_dentry_cache"
crash>
It's failing in its call to get_pathname(). I haven't had a chance to
fully check into why it's getting a bogus parent dentry address
while walking through the swap file's
"/dev/mapper/vg_dellpec610002-lv_swap"
pathname -- and I won't be able to until next week. But presumably it's
related, and so if you (or anybody) gets a chance this weekend, please
take a look.
Thanks,
Dave
9:28 p.m.
At 2012-3-10 5:50, Dave Anderson wrote:
----- Original Message -----
>
>
> ----- Original Message -----
>> Hello Dave,
>>
>> New kernel has moved some elements of struct vfsmount to a new struct
>> mount. So crash will not act normally on new kernel. The patch is used
>> to fix the problem.
>>
>> Please check.
>
> Hi Qiao,
>
> This patch-set is very much appreciated. I saw Al Viro's original LKML patch
> posted back in December (?), and had been waiting to see an actual kernel with
> it in place. The most recent in-house RHEL/Fedora test kernels I have on-hand
> are all 3.2.x-era kernels, which don't have the patch, so I'm presuming
you're
> running on a 3.3.x kernel?
>
> Anyway, I'll check this patch out for backwards-compatibility and try to
> get a more recent kernel to test with. It looks good on paper...
>
> And again, thanks for taking on this task.
>
> Dave
I provisioned a system with a Fedora 3.3.0-0.rc6.git2.1.fc17 kernel, which
has the vfsmount->mount patch. The session comes up cleanly, but the
following commands fail:
crash> mount
... [ cut ] ...
mount: invalid structure member offset: vfsmount_mnt_list
...
crash> files
... [cut] ...
files: invalid structure member offset: dentry_d_covers
...
The "fuser" command generates the above error because it uses the
"files" command behind the scenes.
crash> vm
... [cut] ...
vm: invalid structure member offset: dentry_d_covers
...
crash> swap
... [cut] ...
swap: invalid structure member offset: dentry_d_covers
...
With your patch applied, "mount", "files", "fuser" and
"vm" all
work OK.
But "swap" now fails in a different manner:
crash> swap
FILENAME TYPE SIZE USED PCT PRIORITY
swap: invalid kernel virtual address: 1d8ec8 type: "fill_dentry_cache"
crash>
It's failing in its call to get_pathname(). I haven't had a chance to
You are right. I forgot to check every call to get_pathname. The new
patch has fixed the problem of swap.
fully check into why it's getting a bogus parent dentry address
while walking through the swap file's
"/dev/mapper/vg_dellpec610002-lv_swap"
pathname -- and I won't be able to until next week. But presumably it's
related, and so if you (or anybody) gets a chance this weekend, please
take a look.
Thanks,
Dave
--
Crash-utility mailing list
Crash-utility(a)redhat.com
https://www.redhat.com/mailman/listinfo/crash-utility
--
--
Regards
Qiao Nuohan
--------------------------------------------------
Qiao Nuohan
Development Dept.I
Nanjing Fujitsu Nanda Software Tech. Co., Ltd.(FNST)
No. 6 Wenzhu Road, Nanjing, Nanjing, 210012, China
TEL: +86+25-86630566-8526 FUJITSU
INTERNAL: 7998-8526 FAX: +86+25-83317685
EMail: qiaonuohan(a)cn.fujitsu.com
--------------------------------------------------
This communication is for use by the intended
recipient(s) only and may contain information that
is privileged, confidential and exempt from
disclosure under applicable law. If you are not an
intended recipient of this communication, you are
hereby notified that any dissemination,
distribution or copying hereof is strictly
prohibited. If you have received this communication
in error, please notify me by reply e-mail,
permanently delete this communication from your
system, and destroy any hard copies you may have
printed
2:27 p.m.
----- Original Message -----
At 2012-3-10 5:50, Dave Anderson wrote:
>
> It's failing in its call to get_pathname(). I haven't had a chance to
You are right. I forgot to check every call to get_pathname. The new
patch has fixed the problem of swap.
Although the patch appears to work in the simple cases that I have tested,
I have a few issues with it.
First, the get_pathname() function prototype is -- and has always been -- this:
void get_pathname(ulong dentry, char *pathname, int length, int full, ulong vfsmnt)
where the "vfsmnt" argument has always been a struct vfsmount pointer.
With your patch, prior to making get_pathname() calls, you typically
make a VALID_STRUCT(mount) call first, and if true, you modify the
vfsmnt argument to be a struct mount pointer.
It would make more sense maintenance-wise, and would simplify the patch,
if the get_pathname() "vfsmnt" argument would continue to be a struct
vfsmount pointer. And at the top of get_pathname(), you could make the
adjustments for when it is embedded inside a struct mount.
For example, taking these patches to open_files_dump() and file_dump():
@@ -2226,12 +2299,16 @@ open_files_dump(ulong task, int flags, struct reference *ref)
if (VALID_MEMBER(fs_struct_rootmnt)) {
vfsmnt = ULONG(fs_struct_buf +
OFFSET(fs_struct_rootmnt));
+ if (VALID_STRUCT(mount))
+ vfsmnt = vfsmnt - OFFSET(mount_mnt);
get_pathname(root_dentry, root_pathname,
BUFSIZE, 1, vfsmnt);
} else if (use_path) {
vfsmnt = ULONG(fs_struct_buf +
OFFSET(fs_struct_root) +
OFFSET(path_mnt));
+ if (VALID_STRUCT(mount))
+ vfsmnt = vfsmnt - OFFSET(mount_mnt);
get_pathname(root_dentry, root_pathname,
BUFSIZE, 1, vfsmnt);
} else {
@@ -2250,12 +2327,16 @@ open_files_dump(ulong task, int flags, struct reference *ref)
if (VALID_MEMBER(fs_struct_pwdmnt)) {
vfsmnt = ULONG(fs_struct_buf +
OFFSET(fs_struct_pwdmnt));
+ if (VALID_STRUCT(mount))
+ vfsmnt = vfsmnt - OFFSET(mount_mnt);
get_pathname(pwd_dentry, pwd_pathname,
BUFSIZE, 1, vfsmnt);
} else if (use_path) {
vfsmnt = ULONG(fs_struct_buf +
OFFSET(fs_struct_pwd) +
OFFSET(path_mnt));
+ if (VALID_STRUCT(mount))
+ vfsmnt = vfsmnt - OFFSET(mount_mnt);
get_pathname(pwd_dentry, pwd_pathname,
BUFSIZE, 1, vfsmnt);
@@ -2686,7 +2767,12 @@ file_dump(ulong file, ulong dentry, ulong inode, int fd, int
flags)
if (flags & DUMP_FULL_NAME) {
if (VALID_MEMBER(file_f_vfsmnt)) {
vfsmnt = get_root_vfsmount(file_buf);
- get_pathname(dentry, pathname, BUFSIZE, 1, vfsmnt);
+ if (VALID_STRUCT(mount))
+ get_pathname(dentry, pathname, BUFSIZE, 1,
+ vfsmnt - OFFSET(mount_mnt));
+ else
+ get_pathname(dentry, pathname, BUFSIZE, 1,
+ vfsmnt);
if (STRNEQ(pathname, "/pts/") &&
STREQ(vfsmount_devname(vfsmnt, buf1, BUFSIZE),
"devpts"))
If the vfsmount-to-mount calculation were always done in get_pathname() itself,
then the patches above would be unnecessary. The same is true for a few other
parts of the patch.
Did you consider doing it that way?
And consider the following patched version of display_dentry_info() below.
The two get_pathname() calls below would generate fatal errors on older
kernels because OFFSET(mount_mnt) will be invalid:
if (!found && symbol_exists("pipe_mnt")) {
get_symbol_data("pipe_mnt", sizeof(long), &vfs);
if (VALID_STRUCT(mount))
readmem(vfs - OFFSET(mount_mnt), KVADDR, mount_buf,
SIZE(mount),
"mount buffer", FAULT_ON_ERROR);
else
readmem(vfs, KVADDR, vfsmount_buf, SIZE(vfsmount),
"vfsmount buffer", FAULT_ON_ERROR);
sb = ULONG(vfsmount_buf + OFFSET(vfsmount_mnt_sb));
if (superblock && (sb == superblock)) {
get_pathname(dentry, pathname, BUFSIZE, 1,
vfs - OFFSET(mount_mnt));
found = TRUE;
}
}
if (!found && symbol_exists("sock_mnt")) {
get_symbol_data("sock_mnt", sizeof(long), &vfs);
if (VALID_STRUCT(mount))
readmem(vfs - OFFSET(mount_mnt), KVADDR, mount_buf,
SIZE(mount),
"mount buffer", FAULT_ON_ERROR);
else
readmem(vfs, KVADDR, vfsmount_buf, SIZE(vfsmount),
"vfsmount buffer", FAULT_ON_ERROR);
sb = ULONG(vfsmount_buf + OFFSET(vfsmount_mnt_sb));
if (superblock && (sb == superblock)) {
get_pathname(dentry, pathname, BUFSIZE, 1,
vfs - OFFSET(mount_mnt));
found = TRUE;
}
}
If we can continue to have get_pathname() receive a vfsmount pointer,
it reduces the chance of introducing new bugs like the above, and future
maintenance will be easier.
Comments?
Dave
2:50 p.m.
New subject: [PATCH] fix command vm/files -d/mount on new kernel
----- Original Message -----
...
If we can continue to have get_pathname() receive a vfsmount pointer,
it reduces the chance of introducing new bugs like the above, and future
maintenance will be easier.
Qiao,
Another thing I forgot to mention w/respect to simplifying your
patch. You do things like this in a number of places:
@@ -1336,9 +1336,15 @@ show_mounts(ulong one_vfsmount, int flags, struct task_context
*namespace_contex
devlen = strlen("DEVNAME");
for (m = 0, vfsmnt = mntlist; m < mount_cnt; m++, vfsmnt++) {
- readmem(*vfsmnt + OFFSET(vfsmount_mnt_devname),
- KVADDR, &devp, sizeof(void *),
- "vfsmount mnt_devname", FAULT_ON_ERROR);
+ if (VALID_STRUCT(mount)) {
+ readmem(*vfsmnt+OFFSET(mount_mnt_devname),
+ KVADDR, &devp, sizeof(void *),
+ "mount mnt_devname", FAULT_ON_ERROR);
+ } else {
+ readmem(*vfsmnt+OFFSET(vfsmount_mnt_devname),
+ KVADDR, &devp, sizeof(void *),
+ "vfsmount mnt_devname",
FAULT_ON_ERROR);
+ }
if (read_string(devp, buf1, BUFSIZE-1)) {
if (strlen(buf1) > devlen)
which could be simplified by a single readmem() call using OFFSET_OPTION():
readmem(*vfsmnt + OFFSET_OPTION(vfsmount_mnt_devname, mount_mnt_devname),
KVADDR, &devp, sizeof(void *), "[vfs]mount mnt_devname", FAULT_ON_ERROR);
If the first (older-kernel) option fails, then the second one is used.
Dave
10:45 p.m.
At 2012-3-13 3:27, Dave Anderson wrote:
----- Original Message -----
> At 2012-3-10 5:50, Dave Anderson wrote:
>>
>> It's failing in its call to get_pathname(). I haven't had a chance to
>
> You are right. I forgot to check every call to get_pathname. The new
> patch has fixed the problem of swap.
Although the patch appears to work in the simple cases that I have tested,
I have a few issues with it.
First, the get_pathname() function prototype is -- and has always been -- this:
void get_pathname(ulong dentry, char *pathname, int length, int full, ulong vfsmnt)
where the "vfsmnt" argument has always been a struct vfsmount pointer.
With your patch, prior to making get_pathname() calls, you typically
make a VALID_STRUCT(mount) call first, and if true, you modify the
vfsmnt argument to be a struct mount pointer.
It would make more sense maintenance-wise, and would simplify the patch,
if the get_pathname() "vfsmnt" argument would continue to be a struct
vfsmount pointer. And at the top of get_pathname(), you could make the
adjustments for when it is embedded inside a struct mount.
For example, taking these patches to open_files_dump() and file_dump():
@@ -2226,12 +2299,16 @@ open_files_dump(ulong task, int flags, struct reference *ref)
if (VALID_MEMBER(fs_struct_rootmnt)) {
vfsmnt = ULONG(fs_struct_buf +
OFFSET(fs_struct_rootmnt));
+ if (VALID_STRUCT(mount))
+ vfsmnt = vfsmnt - OFFSET(mount_mnt);
get_pathname(root_dentry, root_pathname,
BUFSIZE, 1, vfsmnt);
} else if (use_path) {
vfsmnt = ULONG(fs_struct_buf +
OFFSET(fs_struct_root) +
OFFSET(path_mnt));
+ if (VALID_STRUCT(mount))
+ vfsmnt = vfsmnt - OFFSET(mount_mnt);
get_pathname(root_dentry, root_pathname,
BUFSIZE, 1, vfsmnt);
} else {
@@ -2250,12 +2327,16 @@ open_files_dump(ulong task, int flags, struct reference *ref)
if (VALID_MEMBER(fs_struct_pwdmnt)) {
vfsmnt = ULONG(fs_struct_buf +
OFFSET(fs_struct_pwdmnt));
+ if (VALID_STRUCT(mount))
+ vfsmnt = vfsmnt - OFFSET(mount_mnt);
get_pathname(pwd_dentry, pwd_pathname,
BUFSIZE, 1, vfsmnt);
} else if (use_path) {
vfsmnt = ULONG(fs_struct_buf +
OFFSET(fs_struct_pwd) +
OFFSET(path_mnt));
+ if (VALID_STRUCT(mount))
+ vfsmnt = vfsmnt - OFFSET(mount_mnt);
get_pathname(pwd_dentry, pwd_pathname,
BUFSIZE, 1, vfsmnt);
@@ -2686,7 +2767,12 @@ file_dump(ulong file, ulong dentry, ulong inode, int fd, int
flags)
if (flags& DUMP_FULL_NAME) {
if (VALID_MEMBER(file_f_vfsmnt)) {
vfsmnt = get_root_vfsmount(file_buf);
- get_pathname(dentry, pathname, BUFSIZE, 1, vfsmnt);
+ if (VALID_STRUCT(mount))
+ get_pathname(dentry, pathname, BUFSIZE, 1,
+ vfsmnt - OFFSET(mount_mnt));
+ else
+ get_pathname(dentry, pathname, BUFSIZE, 1,
+ vfsmnt);
if (STRNEQ(pathname, "/pts/")&&
STREQ(vfsmount_devname(vfsmnt, buf1, BUFSIZE),
"devpts"))
If the vfsmount-to-mount calculation were always done in get_pathname() itself,
then the patches above would be unnecessary. The same is true for a few other
parts of the patch.
Did you consider doing it that way?
And consider the following patched version of display_dentry_info() below.
The two get_pathname() calls below would generate fatal errors on older
kernels because OFFSET(mount_mnt) will be invalid:
if (!found&& symbol_exists("pipe_mnt")) {
get_symbol_data("pipe_mnt", sizeof(long),&vfs);
if (VALID_STRUCT(mount))
readmem(vfs - OFFSET(mount_mnt), KVADDR, mount_buf,
SIZE(mount),
"mount buffer", FAULT_ON_ERROR);
else
readmem(vfs, KVADDR, vfsmount_buf, SIZE(vfsmount),
"vfsmount buffer", FAULT_ON_ERROR);
sb = ULONG(vfsmount_buf + OFFSET(vfsmount_mnt_sb));
if (superblock&& (sb == superblock)) {
get_pathname(dentry, pathname, BUFSIZE, 1,
vfs - OFFSET(mount_mnt));
found = TRUE;
}
}
if (!found&& symbol_exists("sock_mnt")) {
get_symbol_data("sock_mnt", sizeof(long),&vfs);
if (VALID_STRUCT(mount))
readmem(vfs - OFFSET(mount_mnt), KVADDR, mount_buf,
SIZE(mount),
"mount buffer", FAULT_ON_ERROR);
else
readmem(vfs, KVADDR, vfsmount_buf, SIZE(vfsmount),
"vfsmount buffer", FAULT_ON_ERROR);
sb = ULONG(vfsmount_buf + OFFSET(vfsmount_mnt_sb));
if (superblock&& (sb == superblock)) {
get_pathname(dentry, pathname, BUFSIZE, 1,
vfs - OFFSET(mount_mnt));
found = TRUE;
}
}
If we can continue to have get_pathname() receive a vfsmount pointer,
it reduces the chance of introducing new bugs like the above, and future
maintenance will be easier.
It does reduce complication of the code. Still, places needing the
judgement of struct mount's existing remain. I will modify the patch,
and post later.
About the function, OFFSET_OPTION(), thanks for your advice.
Comments?
Dave
--
Crash-utility mailing list
Crash-utility(a)redhat.com
https://www.redhat.com/mailman/listinfo/crash-utility
--
--
Regards
Qiao Nuohan
1:23 a.m.
At 2012-3-13 11:45, qiaonuohan wrote:
At 2012-3-13 3:27, Dave Anderson wrote:
>
>
> ----- Original Message -----
>> At 2012-3-10 5:50, Dave Anderson wrote:
>>>
>>> It's failing in its call to get_pathname(). I haven't had a chance
to
>>
>> You are right. I forgot to check every call to get_pathname. The new
>> patch has fixed the problem of swap.
>
> Although the patch appears to work in the simple cases that I have
> tested,
> I have a few issues with it.
>
> First, the get_pathname() function prototype is -- and has always been
> -- this:
>
> void get_pathname(ulong dentry, char *pathname, int length, int full,
> ulong vfsmnt)
>
> where the "vfsmnt" argument has always been a struct vfsmount pointer.
>
> With your patch, prior to making get_pathname() calls, you typically
> make a VALID_STRUCT(mount) call first, and if true, you modify the
> vfsmnt argument to be a struct mount pointer.
>
> It would make more sense maintenance-wise, and would simplify the patch,
> if the get_pathname() "vfsmnt" argument would continue to be a struct
> vfsmount pointer. And at the top of get_pathname(), you could make the
> adjustments for when it is embedded inside a struct mount.
>
> For example, taking these patches to open_files_dump() and file_dump():
>
> @@ -2226,12 +2299,16 @@ open_files_dump(ulong task, int flags, struct
> reference *ref)
> if (VALID_MEMBER(fs_struct_rootmnt)) {
> vfsmnt = ULONG(fs_struct_buf +
> OFFSET(fs_struct_rootmnt));
> + if (VALID_STRUCT(mount))
> + vfsmnt = vfsmnt - OFFSET(mount_mnt);
> get_pathname(root_dentry, root_pathname,
> BUFSIZE, 1, vfsmnt);
> } else if (use_path) {
> vfsmnt = ULONG(fs_struct_buf +
> OFFSET(fs_struct_root) +
> OFFSET(path_mnt));
> + if (VALID_STRUCT(mount))
> + vfsmnt = vfsmnt - OFFSET(mount_mnt);
> get_pathname(root_dentry, root_pathname,
> BUFSIZE, 1, vfsmnt);
> } else {
> @@ -2250,12 +2327,16 @@ open_files_dump(ulong task, int flags, struct
> reference *ref)
> if (VALID_MEMBER(fs_struct_pwdmnt)) {
> vfsmnt = ULONG(fs_struct_buf +
> OFFSET(fs_struct_pwdmnt));
> + if (VALID_STRUCT(mount))
> + vfsmnt = vfsmnt - OFFSET(mount_mnt);
> get_pathname(pwd_dentry, pwd_pathname,
> BUFSIZE, 1, vfsmnt);
> } else if (use_path) {
> vfsmnt = ULONG(fs_struct_buf +
> OFFSET(fs_struct_pwd) +
> OFFSET(path_mnt));
> + if (VALID_STRUCT(mount))
> + vfsmnt = vfsmnt - OFFSET(mount_mnt);
> get_pathname(pwd_dentry, pwd_pathname,
> BUFSIZE, 1, vfsmnt);
> @@ -2686,7 +2767,12 @@ file_dump(ulong file, ulong dentry, ulong
> inode, int fd, int flags)
> if (flags& DUMP_FULL_NAME) {
> if (VALID_MEMBER(file_f_vfsmnt)) {
> vfsmnt = get_root_vfsmount(file_buf);
> - get_pathname(dentry, pathname, BUFSIZE, 1, vfsmnt);
> + if (VALID_STRUCT(mount))
> + get_pathname(dentry, pathname, BUFSIZE, 1,
> + vfsmnt - OFFSET(mount_mnt));
> + else
> + get_pathname(dentry, pathname, BUFSIZE, 1,
> + vfsmnt);
> if (STRNEQ(pathname, "/pts/")&&
> STREQ(vfsmount_devname(vfsmnt, buf1, BUFSIZE),
> "devpts"))
>
> If the vfsmount-to-mount calculation were always done in
> get_pathname() itself,
> then the patches above would be unnecessary. The same is true for a
> few other
> parts of the patch.
>
> Did you consider doing it that way?
>
> And consider the following patched version of display_dentry_info()
> below.
> The two get_pathname() calls below would generate fatal errors on older
> kernels because OFFSET(mount_mnt) will be invalid:
>
> if (!found&& symbol_exists("pipe_mnt")) {
> get_symbol_data("pipe_mnt", sizeof(long),&vfs);
> if (VALID_STRUCT(mount))
> readmem(vfs - OFFSET(mount_mnt), KVADDR, mount_buf, SIZE(mount),
> "mount buffer", FAULT_ON_ERROR);
> else
> readmem(vfs, KVADDR, vfsmount_buf, SIZE(vfsmount),
> "vfsmount buffer", FAULT_ON_ERROR);
> sb = ULONG(vfsmount_buf + OFFSET(vfsmount_mnt_sb));
> if (superblock&& (sb == superblock)) {
> get_pathname(dentry, pathname, BUFSIZE, 1,
> vfs - OFFSET(mount_mnt));
> found = TRUE;
> }
> }
> if (!found&& symbol_exists("sock_mnt")) {
> get_symbol_data("sock_mnt", sizeof(long),&vfs);
> if (VALID_STRUCT(mount))
> readmem(vfs - OFFSET(mount_mnt), KVADDR, mount_buf, SIZE(mount),
> "mount buffer", FAULT_ON_ERROR);
> else
> readmem(vfs, KVADDR, vfsmount_buf, SIZE(vfsmount),
> "vfsmount buffer", FAULT_ON_ERROR);
> sb = ULONG(vfsmount_buf + OFFSET(vfsmount_mnt_sb));
> if (superblock&& (sb == superblock)) {
> get_pathname(dentry, pathname, BUFSIZE, 1,
> vfs - OFFSET(mount_mnt));
> found = TRUE;
> }
> }
>
> If we can continue to have get_pathname() receive a vfsmount pointer,
> it reduces the chance of introducing new bugs like the above, and future
> maintenance will be easier.
It does reduce complication of the code. Still, places needing the
judgement of struct mount's existing remain. I will modify the patch,
and post later.
About the function, OFFSET_OPTION(), thanks for your advice.
The attachment is the modified patch.
>
> Comments?
>
> Dave
>
> --
> Crash-utility mailing list
> Crash-utility(a)redhat.com
> https://www.redhat.com/mailman/listinfo/crash-utility
>
>
--
--
Regards
Qiao Nuohan
3:45 p.m.
New subject: [PATCH] add -s option to vm to display one vma at a time
----- Original Message -----
At 2012-3-13 17:56, qiaonuohan wrote:
Hello Dave,
When I am using "vm -p" command, I feel it is chaotic when too
much data is printed. I think it is clear to display one vma
each time.
In the patch, I compare all vmas with the argument of -s option.
If an equal vma is found, it will be printed like below.
crash> vm -ps ffff88028ff7d3a0
VMA START END FLAGS FILE
ffff88028ff7d3a0 7fff29b71000 7fff29b87000 100173
VIRTUAL PHYSICAL
7fff29b71000 (not mapped)
7fff29b72000 (not mapped)
7fff29b73000 (not mapped)
7fff29b74000 (not mapped)
7fff29b75000 (not mapped)
7fff29b76000 (not mapped)
7fff29b77000 27faad000
7fff29b78000 2807aa000
7fff29b79000 280781000
7fff29b7a000 280787000
7fff29b7b000 280776000
7fff29b7c000 280786000
7fff29b7d000 27f2df000
7fff29b7e000 27f2e0000
7fff29b7f000 27f2e1000
7fff29b80000 27f2d7000
7fff29b81000 28b1ac000
7fff29b82000 28ecc1000
7fff29b83000 28c5c2000
7fff29b84000 28aaf4000
7fff29b85000 28aaf9000
7fff29b86000 289566000
crash>
Seems like a reasonable request.
However, I don't like your mixing it with the "-R reference" usage,
because it confuses things like this simple example:
crash> vm -p -s ffff810ec9f57818
VMA START END FLAGS FILE
ffff810ec9f57818 4010d000 40110000 101877 /usr/local/java/jdk1.5.0_19/bin/java
VIRTUAL PHYSICAL
4010d000 ec89b1000
4010e000 FILE: /usr/local/java/jdk1.5.0_19/bin/java OFFSET: e000
4010f000 e99803000
crash>
If the command above were to be qualified with a "-R 4010e000" reference
check, it results in a nonsensical error message:
crash> vm -p -s ffff810ec9f57818 -R 4010e000
vm: only one -R option allowed
Usage:
vm [-p | -v | -m | [-R reference] | [-f vm_flags]] [pid | taskp] ...
Enter "help vm" for details.
crash>
A few suggestions:
(1) Don't even bother to tie it in with the "vm -v" option, because if
you already know the vm_area_struct address, then just print it with
"vm_area_struct <address>".
(2) Then, since it *only* applies with the -p functionality, make it
a new "-P <vmaddr>" option, where the help page explains that the
<vmaddr> argument must be a vm_area_struct of the current context:
Usage:
vm [-p | -P vmaddr | -v | -m | [-R reference] | [-f vm_flags]] [pid | taskp] ...
(3) Make -P mutually exclusive with all of the other options.
(4) Do not use the reference structure for this feature. Just use your new
flag, and pass the vma address in the 3rd "vaddr" argument to
vm_area_dump(),
since it's not even being used by cmd_vm().
Dave
9:05 p.m.
New subject: [PATCH] add -s option to vm to display one vma at a time
At 2012-3-14 4:45, Dave Anderson wrote:
----- Original Message -----
> At 2012-3-13 17:56, qiaonuohan wrote:
>
> Hello Dave,
>
> When I am using "vm -p" command, I feel it is chaotic when too
> much data is printed. I think it is clear to display one vma
> each time.
>
> In the patch, I compare all vmas with the argument of -s option.
> If an equal vma is found, it will be printed like below.
>
> crash> vm -ps ffff88028ff7d3a0
> VMA START END FLAGS FILE
> ffff88028ff7d3a0 7fff29b71000 7fff29b87000 100173
> VIRTUAL PHYSICAL
> 7fff29b71000 (not mapped)
> 7fff29b72000 (not mapped)
> 7fff29b73000 (not mapped)
> 7fff29b74000 (not mapped)
> 7fff29b75000 (not mapped)
> 7fff29b76000 (not mapped)
> 7fff29b77000 27faad000
> 7fff29b78000 2807aa000
> 7fff29b79000 280781000
> 7fff29b7a000 280787000
> 7fff29b7b000 280776000
> 7fff29b7c000 280786000
> 7fff29b7d000 27f2df000
> 7fff29b7e000 27f2e0000
> 7fff29b7f000 27f2e1000
> 7fff29b80000 27f2d7000
> 7fff29b81000 28b1ac000
> 7fff29b82000 28ecc1000
> 7fff29b83000 28c5c2000
> 7fff29b84000 28aaf4000
> 7fff29b85000 28aaf9000
> 7fff29b86000 289566000
> crash>
Seems like a reasonable request.
However, I don't like your mixing it with the "-R reference" usage,
because it confuses things like this simple example:
crash> vm -p -s ffff810ec9f57818
VMA START END FLAGS FILE
ffff810ec9f57818 4010d000 40110000 101877 /usr/local/java/jdk1.5.0_19/bin/java
VIRTUAL PHYSICAL
4010d000 ec89b1000
4010e000 FILE: /usr/local/java/jdk1.5.0_19/bin/java OFFSET: e000
4010f000 e99803000
crash>
If the command above were to be qualified with a "-R 4010e000" reference
check, it results in a nonsensical error message:
crash> vm -p -s ffff810ec9f57818 -R 4010e000
vm: only one -R option allowed
Usage:
vm [-p | -v | -m | [-R reference] | [-f vm_flags]] [pid | taskp] ...
Enter "help vm" for details.
crash>
A few suggestions:
(1) Don't even bother to tie it in with the "vm -v" option, because if
you already know the vm_area_struct address, then just print it with
"vm_area_struct<address>".
(2) Then, since it *only* applies with the -p functionality, make it
a new "-P<vmaddr>" option, where the help page explains that the
<vmaddr> argument must be a vm_area_struct of the current context:
Usage:
vm [-p | -P vmaddr | -v | -m | [-R reference] | [-f vm_flags]] [pid | taskp] ...
(3) Make -P mutually exclusive with all of the other options.
(4) Do not use the reference structure for this feature. Just use your new
flag, and pass the vma address in the 3rd "vaddr" argument to
vm_area_dump(),
since it's not even being used by cmd_vm().
I have modified the patch, please check.
I need to refer one more think. Yesterday, I sent the modified patch
used to fix the problem of "swap/mount/file/...", please check it.
Dave
--
Crash-utility mailing list
Crash-utility(a)redhat.com
https://www.redhat.com/mailman/listinfo/crash-utility
--
--
Regards
Qiao Nuohan
2:06 p.m.
New subject: [PATCH] add -s option to vm to display one vma at a time
----- Original Message -----
At 2012-3-14 4:45, Dave Anderson wrote:
>
>
> ----- Original Message -----
>> At 2012-3-13 17:56, qiaonuohan wrote:
>>
>> Hello Dave,
>>
>> When I am using "vm -p" command, I feel it is chaotic when too
>> much data is printed. I think it is clear to display one vma
>> each time.
>>
>> In the patch, I compare all vmas with the argument of -s option.
>> If an equal vma is found, it will be printed like below.
>>
>> crash> vm -ps ffff88028ff7d3a0
>> VMA START END FLAGS FILE
>> ffff88028ff7d3a0 7fff29b71000 7fff29b87000 100173
>> VIRTUAL PHYSICAL
>> 7fff29b71000 (not mapped)
>> 7fff29b72000 (not mapped)
>> 7fff29b73000 (not mapped)
>> 7fff29b74000 (not mapped)
>> 7fff29b75000 (not mapped)
>> 7fff29b76000 (not mapped)
>> 7fff29b77000 27faad000
>> 7fff29b78000 2807aa000
>> 7fff29b79000 280781000
>> 7fff29b7a000 280787000
>> 7fff29b7b000 280776000
>> 7fff29b7c000 280786000
>> 7fff29b7d000 27f2df000
>> 7fff29b7e000 27f2e0000
>> 7fff29b7f000 27f2e1000
>> 7fff29b80000 27f2d7000
>> 7fff29b81000 28b1ac000
>> 7fff29b82000 28ecc1000
>> 7fff29b83000 28c5c2000
>> 7fff29b84000 28aaf4000
>> 7fff29b85000 28aaf9000
>> 7fff29b86000 289566000
>> crash>
>
> Seems like a reasonable request.
>
> However, I don't like your mixing it with the "-R reference" usage,
> because it confuses things like this simple example:
>
> crash> vm -p -s ffff810ec9f57818
> VMA START END FLAGS FILE
> ffff810ec9f57818 4010d000 40110000 101877
/usr/local/java/jdk1.5.0_19/bin/java
> VIRTUAL PHYSICAL
> 4010d000 ec89b1000
> 4010e000 FILE: /usr/local/java/jdk1.5.0_19/bin/java OFFSET: e000
> 4010f000 e99803000
> crash>
>
> If the command above were to be qualified with a "-R 4010e000" reference
> check, it results in a nonsensical error message:
>
> crash> vm -p -s ffff810ec9f57818 -R 4010e000
> vm: only one -R option allowed
> Usage:
> vm [-p | -v | -m | [-R reference] | [-f vm_flags]] [pid |
> taskp] ...
> Enter "help vm" for details.
> crash>
>
> A few suggestions:
>
> (1) Don't even bother to tie it in with the "vm -v" option, because
if
> you already know the vm_area_struct address, then just print it with
> "vm_area_struct <address>".
>
> (2) Then, since it *only* applies with the -p functionality, make it
> a new "-P<vmaddr>" option, where the help page explains that
the
> <vmaddr> argument must be a vm_area_struct of the current context:
>
> Usage:
> vm [-p | -P vmaddr | -v | -m | [-R reference] | [-f
> vm_flags]] [pid | taskp] ...
>
> (3) Make -P mutually exclusive with all of the other options.
>
> (4) Do not use the reference structure for this feature. Just use your new
> flag, and pass the vma address in the 3rd "vaddr" argument to
vm_area_dump(),
> since it's not even being used by cmd_vm().
I have modified the patch, please check.
OK, these are my suggestions to finalize this patch. First, fix this:
# make warn
...
cc -c -g -DX86_64 -DGDB_7_3_1 memory.c -Wall -O2 -Wstrict-prototypes
-Wmissing-prototypes -fstack-protector
memory.c: In function "vm_area_dump":
memory.c:3503:7: warning: "single_vma" may be used uninitialized in this
function [-Wuninitialized]
...
Secondly, this command is no different from the other context-sensitive
"vm" command options, so please always print the task header like they
do. Just remove the three restrictions in cmd_vm() that do this:
- if (!ref)
+ if (!(ref || (flag & PRINT_SINGLE_VMA)))
print_task_header(fp, CURRENT_CONTEXT(), 0);
Third, the option is either going to work OK or fail to find the VMA.
Your patch does this if an invalid vm_area_struct address is entered:
crash> vm -P ffff880617e7af44
VMA START END FLAGS FILE
crash>
Regardless of success or failure, the print_task_header() call in cmd_vm()
should always show the task information. Then, if you actually find the VMA,
print the "VMA START ..." header along with its data:
crash> vm -P ffff880284cf87e0
PID: 13318 TASK: ffff8806294f2690 CPU: 11 COMMAND: "crash"
VMA START END FLAGS FILE
ffff880284cf87e0 400000 9fa000 8001875 /root/crash-6.0.4/crash
VIRTUAL PHYSICAL
400000 28c7fa000
401000 29470a000
402000 29470b000
403000 29470c000
404000 29470d000
405000 29470e000
406000 29470f000
...
But if the command fails to find the VMA, then indicate that under
the task header:
crash> vm -P ffff880284cf87e8
PID: 13318 TASK: ffff8806294f2690 CPU: 11 COMMAND: "crash"
(not found)
crash>
Lastly, please don't use subterfuge to accomplish the task at
hand. The setting of PHYSADDR in the vm_area_dump() function
makes no sense at all:
+ single_vma_header = 0;
+ if (flag & PRINT_SINGLE_VMA) {
+ flag |= PHYSADDR;
+ single_vma = vaddr;
+ vaddr = 0;
+ }
It may "work" by doing the above, but I'm sure you can accomplish
it just as easily using your PRINT_SINGLE_VMA flag and/or your new
local variables, making it both easier to understand and more
maintainable.
Thanks,
Dave
9:31 p.m.
New subject: [PATCH] add -s option to vm to display one vma at a time
At 2012-3-15 3:06, Dave Anderson wrote:
----- Original Message -----
> At 2012-3-14 4:45, Dave Anderson wrote:
>>
>>
>> ----- Original Message -----
>>> At 2012-3-13 17:56, qiaonuohan wrote:
>>>
>>> Hello Dave,
>>>
>>> When I am using "vm -p" command, I feel it is chaotic when too
>>> much data is printed. I think it is clear to display one vma
>>> each time.
>>>
>>> In the patch, I compare all vmas with the argument of -s option.
>>> If an equal vma is found, it will be printed like below.
>>>
>>> crash> vm -ps ffff88028ff7d3a0
>>> VMA START END FLAGS FILE
>>> ffff88028ff7d3a0 7fff29b71000 7fff29b87000 100173
>>> VIRTUAL PHYSICAL
>>> 7fff29b71000 (not mapped)
>>> 7fff29b72000 (not mapped)
>>> 7fff29b73000 (not mapped)
>>> 7fff29b74000 (not mapped)
>>> 7fff29b75000 (not mapped)
>>> 7fff29b76000 (not mapped)
>>> 7fff29b77000 27faad000
>>> 7fff29b78000 2807aa000
>>> 7fff29b79000 280781000
>>> 7fff29b7a000 280787000
>>> 7fff29b7b000 280776000
>>> 7fff29b7c000 280786000
>>> 7fff29b7d000 27f2df000
>>> 7fff29b7e000 27f2e0000
>>> 7fff29b7f000 27f2e1000
>>> 7fff29b80000 27f2d7000
>>> 7fff29b81000 28b1ac000
>>> 7fff29b82000 28ecc1000
>>> 7fff29b83000 28c5c2000
>>> 7fff29b84000 28aaf4000
>>> 7fff29b85000 28aaf9000
>>> 7fff29b86000 289566000
>>> crash>
>>
>> Seems like a reasonable request.
>>
>> However, I don't like your mixing it with the "-R reference"
usage,
>> because it confuses things like this simple example:
>>
>> crash> vm -p -s ffff810ec9f57818
>> VMA START END FLAGS FILE
>> ffff810ec9f57818 4010d000 40110000 101877
/usr/local/java/jdk1.5.0_19/bin/java
>> VIRTUAL PHYSICAL
>> 4010d000 ec89b1000
>> 4010e000 FILE: /usr/local/java/jdk1.5.0_19/bin/java OFFSET: e000
>> 4010f000 e99803000
>> crash>
>>
>> If the command above were to be qualified with a "-R 4010e000"
reference
>> check, it results in a nonsensical error message:
>>
>> crash> vm -p -s ffff810ec9f57818 -R 4010e000
>> vm: only one -R option allowed
>> Usage:
>> vm [-p | -v | -m | [-R reference] | [-f vm_flags]] [pid |
>> taskp] ...
>> Enter "help vm" for details.
>> crash>
>>
>> A few suggestions:
>>
>> (1) Don't even bother to tie it in with the "vm -v" option, because
if
>> you already know the vm_area_struct address, then just print it with
>> "vm_area_struct<address>".
>>
>> (2) Then, since it *only* applies with the -p functionality, make it
>> a new "-P<vmaddr>" option, where the help page explains
that the
>> <vmaddr> argument must be a vm_area_struct of the current context:
>>
>> Usage:
>> vm [-p | -P vmaddr | -v | -m | [-R reference] | [-f
>> vm_flags]] [pid | taskp] ...
>>
>> (3) Make -P mutually exclusive with all of the other options.
>>
>> (4) Do not use the reference structure for this feature. Just use your new
>> flag, and pass the vma address in the 3rd "vaddr" argument to
vm_area_dump(),
>> since it's not even being used by cmd_vm().
>
> I have modified the patch, please check.
OK, these are my suggestions to finalize this patch. First, fix this:
# make warn
...
cc -c -g -DX86_64 -DGDB_7_3_1 memory.c -Wall -O2 -Wstrict-prototypes
-Wmissing-prototypes -fstack-protector
memory.c: In function "vm_area_dump":
memory.c:3503:7: warning: "single_vma" may be used uninitialized in this
function [-Wuninitialized]
...
Secondly, this command is no different from the other context-sensitive
"vm" command options, so please always print the task header like they
do. Just remove the three restrictions in cmd_vm() that do this:
- if (!ref)
+ if (!(ref || (flag& PRINT_SINGLE_VMA)))
print_task_header(fp, CURRENT_CONTEXT(), 0);
Third, the option is either going to work OK or fail to find the VMA.
Your patch does this if an invalid vm_area_struct address is entered:
crash> vm -P ffff880617e7af44
VMA START END FLAGS FILE
crash>
Regardless of success or failure, the print_task_header() call in cmd_vm()
should always show the task information. Then, if you actually find the VMA,
print the "VMA START ..." header along with its data:
crash> vm -P ffff880284cf87e0
PID: 13318 TASK: ffff8806294f2690 CPU: 11 COMMAND: "crash"
VMA START END FLAGS FILE
ffff880284cf87e0 400000 9fa000 8001875 /root/crash-6.0.4/crash
VIRTUAL PHYSICAL
400000 28c7fa000
401000 29470a000
402000 29470b000
403000 29470c000
404000 29470d000
405000 29470e000
406000 29470f000
...
But if the command fails to find the VMA, then indicate that under
the task header:
crash> vm -P ffff880284cf87e8
PID: 13318 TASK: ffff8806294f2690 CPU: 11 COMMAND: "crash"
(not found)
crash>
Lastly, please don't use subterfuge to accomplish the task at
hand. The setting of PHYSADDR in the vm_area_dump() function
makes no sense at all:
+ single_vma_header = 0;
+ if (flag& PRINT_SINGLE_VMA) {
+ flag |= PHYSADDR;
+ single_vma = vaddr;
+ vaddr = 0;
+ }
It may "work" by doing the above, but I'm sure you can accomplish
it just as easily using your PRINT_SINGLE_VMA flag and/or your new
local variables, making it both easier to understand and more
maintainable.
The patch is modified. I think I need to pay more attention to
maintainable and thanks for your patience.
Thanks,
Dave
--
Crash-utility mailing list
Crash-utility(a)redhat.com
https://www.redhat.com/mailman/listinfo/crash-utility
--
--
Regards
Qiao Nuohan
3:28 p.m.
New subject: [PATCH] add -s option to vm to display one vma at a time
----- Original Message -----
At 2012-3-15 3:06, Dave Anderson wrote:
>
>
> ----- Original Message -----
>> At 2012-3-14 4:45, Dave Anderson wrote:
>>>
>>>
>>> ----- Original Message -----
>>>> At 2012-3-13 17:56, qiaonuohan wrote:
>>>>
>>>> Hello Dave,
>>>>
>>>> When I am using "vm -p" command, I feel it is chaotic when
too
>>>> much data is printed. I think it is clear to display one vma
>>>> each time.
>>>>
>>>> In the patch, I compare all vmas with the argument of -s option.
>>>> If an equal vma is found, it will be printed like below.
>>>>
>>>> crash> vm -ps ffff88028ff7d3a0
>>>> VMA START END FLAGS FILE
>>>> ffff88028ff7d3a0 7fff29b71000 7fff29b87000 100173
>>>> VIRTUAL PHYSICAL
>>>> 7fff29b71000 (not mapped)
>>>> 7fff29b72000 (not mapped)
>>>> 7fff29b73000 (not mapped)
>>>> 7fff29b74000 (not mapped)
>>>> 7fff29b75000 (not mapped)
>>>> 7fff29b76000 (not mapped)
>>>> 7fff29b77000 27faad000
>>>> 7fff29b78000 2807aa000
>>>> 7fff29b79000 280781000
>>>> 7fff29b7a000 280787000
>>>> 7fff29b7b000 280776000
>>>> 7fff29b7c000 280786000
>>>> 7fff29b7d000 27f2df000
>>>> 7fff29b7e000 27f2e0000
>>>> 7fff29b7f000 27f2e1000
>>>> 7fff29b80000 27f2d7000
>>>> 7fff29b81000 28b1ac000
>>>> 7fff29b82000 28ecc1000
>>>> 7fff29b83000 28c5c2000
>>>> 7fff29b84000 28aaf4000
>>>> 7fff29b85000 28aaf9000
>>>> 7fff29b86000 289566000
>>>> crash>
>>>
>>> Seems like a reasonable request.
>>>
>>> However, I don't like your mixing it with the "-R reference"
usage,
>>> because it confuses things like this simple example:
>>>
>>> crash> vm -p -s ffff810ec9f57818
>>> VMA START END FLAGS FILE
>>> ffff810ec9f57818 4010d000 40110000 101877
>>> /usr/local/java/jdk1.5.0_19/bin/java
>>> VIRTUAL PHYSICAL
>>> 4010d000 ec89b1000
>>> 4010e000 FILE: /usr/local/java/jdk1.5.0_19/bin/java OFFSET: e000
>>> 4010f000 e99803000
>>> crash>
>>>
>>> If the command above were to be qualified with a "-R 4010e000"
reference
>>> check, it results in a nonsensical error message:
>>>
>>> crash> vm -p -s ffff810ec9f57818 -R 4010e000
>>> vm: only one -R option allowed
>>> Usage:
>>> vm [-p | -v | -m | [-R reference] | [-f vm_flags]] [pid |
>>> taskp] ...
>>> Enter "help vm" for details.
>>> crash>
>>>
>>> A few suggestions:
>>>
>>> (1) Don't even bother to tie it in with the "vm -v" option,
because if
>>> you already know the vm_area_struct address, then just print it with
>>> "vm_area_struct<address>".
>>>
>>> (2) Then, since it *only* applies with the -p functionality, make it
>>> a new "-P<vmaddr>" option, where the help page explains
that the
>>> <vmaddr> argument must be a vm_area_struct of the current
context:
>>>
>>> Usage:
>>> vm [-p | -P vmaddr | -v | -m | [-R reference] | [-f
>>> vm_flags]] [pid | taskp] ...
>>>
>>> (3) Make -P mutually exclusive with all of the other options.
>>>
>>> (4) Do not use the reference structure for this feature. Just use your new
>>> flag, and pass the vma address in the 3rd "vaddr" argument to
vm_area_dump(),
>>> since it's not even being used by cmd_vm().
>>
>> I have modified the patch, please check.
>
> OK, these are my suggestions to finalize this patch. First, fix this:
>
> # make warn
> ...
> cc -c -g -DX86_64 -DGDB_7_3_1 memory.c -Wall -O2
> -Wstrict-prototypes -Wmissing-prototypes -fstack-protector
> memory.c: In function "vm_area_dump":
> memory.c:3503:7: warning: "single_vma" may be used uninitialized
> in this function [-Wuninitialized]
> ...
>
> Secondly, this command is no different from the other context-sensitive
> "vm" command options, so please always print the task header like they
> do. Just remove the three restrictions in cmd_vm() that do this:
>
> - if (!ref)
> + if (!(ref || (flag& PRINT_SINGLE_VMA)))
> print_task_header(fp, CURRENT_CONTEXT(),
> 0);
>
> Third, the option is either going to work OK or fail to find the VMA.
> Your patch does this if an invalid vm_area_struct address is
> entered:
>
> crash> vm -P ffff880617e7af44
> VMA START END FLAGS FILE
> crash>
>
> Regardless of success or failure, the print_task_header() call in cmd_vm()
> should always show the task information. Then, if you actually find the VMA,
> print the "VMA START ..." header along with its data:
>
> crash> vm -P ffff880284cf87e0
> PID: 13318 TASK: ffff8806294f2690 CPU: 11 COMMAND: "crash"
> VMA START END FLAGS FILE
> ffff880284cf87e0 400000 9fa000 8001875
> /root/crash-6.0.4/crash
> VIRTUAL PHYSICAL
> 400000 28c7fa000
> 401000 29470a000
> 402000 29470b000
> 403000 29470c000
> 404000 29470d000
> 405000 29470e000
> 406000 29470f000
> ...
>
> But if the command fails to find the VMA, then indicate that under
> the task header:
>
> crash> vm -P ffff880284cf87e8
> PID: 13318 TASK: ffff8806294f2690 CPU: 11 COMMAND: "crash"
> (not found)
> crash>
>
> Lastly, please don't use subterfuge to accomplish the task at
> hand. The setting of PHYSADDR in the vm_area_dump() function
> makes no sense at all:
>
> + single_vma_header = 0;
> + if (flag& PRINT_SINGLE_VMA) {
> + flag |= PHYSADDR;
> + single_vma = vaddr;
> + vaddr = 0;
> + }
>
> It may "work" by doing the above, but I'm sure you can accomplish
> it just as easily using your PRINT_SINGLE_VMA flag and/or your new
> local variables, making it both easier to understand and more
> maintainable.
The patch is modified. I think I need to pay more attention to
maintainable and thanks for your patience.
I reworded the help page a little, but other than that the patch
looks good, and is queued for crash-6.0.5.
Thanks,
Dave
12:26 a.m.
New subject: The "ctrl + c" doesn't work in crash
Hi Dave,
I found the "ctrl + c" doesn't work well in crash. Sometimes, we may type a
wrong command and want to directly cancel it by "ctrl+c" like in the bash. But
this combination only do seldom response, what I mean is I once see it works once or
twice.
I also notice there was a mail thread talking about it:
http://www.redhat.com/archives/crash-utility/2009-October/msg00006.html
But on the most new crash 6.0.4, simply ctrl+c three times or more would not lead to a new
prompt...
Thanks,
Lei
8:30 a.m.
New subject: The "ctrl + c" doesn't work in crash
----- Original Message -----
Hi Dave,
I found the "ctrl + c" doesn't work well in crash. Sometimes, we may
type a wrong command and want to directly cancel it by "ctrl+c" like
in the bash. But this combination only do seldom response, what I
mean is I once see it works once or twice.
I also notice there was a mail thread talking about it:
http://www.redhat.com/archives/crash-utility/2009-October/msg00006.html
But on the most new crash 6.0.4, simply ctrl+c three times or more
would not lead to a new prompt...
SIGINT's are always caught, and it depends upon whether the command is
currently being fed into the scroller, and in some cases, the command
itself. See the callers of received_SIGINT() for example.
Dave
8:55 a.m.
New subject: The "ctrl + c" doesn't work in crash
Hi Dave,
-----Original Message-----
From: crash-utility-bounces(a)redhat.com
[mailto:crash-utility-bounces@redhat.com] On Behalf Of Dave Anderson
Sent: Thursday, March 15, 2012 9:31 PM
To: Discussion list for crash utility usage, maintenance and development
Subject: Re: [Crash-utility] The "ctrl + c" doesn't work in crash
----- Original Message -----
> Hi Dave,
>
> I found the "ctrl + c" doesn't work well in crash. Sometimes, we may
> type a wrong command and want to directly cancel it by "ctrl+c" like
> in the bash. But this combination only do seldom response, what I
> mean is I once see it works once or twice.
>
> I also notice there was a mail thread talking about it:
> http://www.redhat.com/archives/crash-utility/2009-October/msg00006.html
>
> But on the most new crash 6.0.4, simply ctrl+c three times or more
> would not lead to a new prompt...
SIGINT's are always caught, and it depends upon whether the command is
currently being fed into the scroller, and in some cases, the command
itself. See the callers of received_SIGINT() for example.
I see...
And while I redo the test, I can see SIGINT work again, prompt the input > when type
three ctrl+c. No idea why previous ctrl+c not working...
I still get a another display issue regarding the ctrl+c.
I use the "log" command to show the kernel dmesg, and always the message buffer
is large which cannot be displayed in one screen. So I could see the below notice in the
foot of the screen:
-- MORE -- forward: <SPACE>, <ENTER> or j backward: b or k quit: q
If I type "ctrl+c" when the message still not get fully displayed, eventually I
would see the previous only one foot notice would show up several and keep growing.
Like this:
[ 413.741638] Unable to handle kernel NULL pointer dereference at virtual address
00000000
[ 413.749725] pgd = d3e90000
[ 413.752441] [00000000] *pgd=12109831, *pte=00000000, *ppte=00000000
-- MORE -- forward: <SPACE>, <ENTER> or j backward: b or k quit: q
-- MORE -- forward: <SPACE>, <ENTER> or j backward: b or k quit: q
-- MORE -- forward: <SPACE>, <ENTER> or j backward: b or k quit: q
And when this abnormal happens, the <space> would be displayed normally as before,
but when the whole message is drained out, the console prompt would not be displayed. And
each <enter> after this abnormal, would lead to another such notice to be showed up
like the message buffer itself.
So is it a bug for the cmdline handling?
Dave
Thanks,
Lei
9:13 a.m.
New subject: The "ctrl + c" doesn't work in crash
----- Original Message -----
Hi Dave,
> -----Original Message-----
> From: crash-utility-bounces(a)redhat.com
> [mailto:crash-utility-bounces@redhat.com] On Behalf Of Dave
> Anderson
> Sent: Thursday, March 15, 2012 9:31 PM
> To: Discussion list for crash utility usage, maintenance and
> development
> Subject: Re: [Crash-utility] The "ctrl + c" doesn't work in crash
>
>
>
> ----- Original Message -----
> > Hi Dave,
> >
> > I found the "ctrl + c" doesn't work well in crash. Sometimes, we
> > may
> > type a wrong command and want to directly cancel it by "ctrl+c"
> > like
> > in the bash. But this combination only do seldom response, what I
> > mean is I once see it works once or twice.
> >
> > I also notice there was a mail thread talking about it:
> > http://www.redhat.com/archives/crash-utility/2009-October/msg00006.html
> >
> > But on the most new crash 6.0.4, simply ctrl+c three times or
> > more
> > would not lead to a new prompt...
>
> SIGINT's are always caught, and it depends upon whether the command is
> currently being fed into the scroller, and in some cases, the command
> itself. See the callers of received_SIGINT() for example.
I see...
And while I redo the test, I can see SIGINT work again, prompt the
input > when type three ctrl+c. No idea why previous ctrl+c not
working...
I still get a another display issue regarding the ctrl+c.
I use the "log" command to show the kernel dmesg, and always the
message buffer is large which cannot be displayed in one screen. So
I could see the below notice in the foot of the screen:
-- MORE -- forward: <SPACE>, <ENTER> or j backward: b or k quit: q
If I type "ctrl+c" when the message still not get fully displayed,
eventually I would see the previous only one foot notice would show
up several and keep growing.
Like this:
[ 413.741638] Unable to handle kernel NULL pointer dereference at virtual address
00000000
[ 413.749725] pgd = d3e90000
[ 413.752441] [00000000] *pgd=12109831, *pte=00000000, *ppte=00000000
-- MORE -- forward: <SPACE>, <ENTER> or j backward: b or k quit: q
-- MORE -- forward: <SPACE>, <ENTER> or j backward: b or k quit: q
-- MORE -- forward: <SPACE>, <ENTER> or j backward: b or k quit: q
And when this abnormal happens, the <space> would be displayed
normally as before, but when the whole message is drained out, the
console prompt would not be displayed. And each <enter> after this
abnormal, would lead to another such notice to be showed up like the
message buffer itself.
So is it a bug for the cmdline handling?
Well, it's just the behavior when the "less" scroller is being used.
Why not just do what the prompt says, and just enter "q"?
Dave
I don't fully understand your description, but why not do what the
prompt says, and just type 'q'?
Dave
9:50 a.m.
New subject: The "ctrl + c" doesn't work in crash
-----Original Message-----
From: crash-utility-bounces(a)redhat.com
[mailto:crash-utility-bounces@redhat.com] On Behalf Of Dave Anderson
Sent: Thursday, March 15, 2012 10:13 PM
To: Discussion list for crash utility usage, maintenance and development
Subject: Re: [Crash-utility] The "ctrl + c" doesn't work in crash
----- Original Message -----
> Hi Dave,
>
> > -----Original Message-----
> > From: crash-utility-bounces(a)redhat.com
> > [mailto:crash-utility-bounces@redhat.com] On Behalf Of Dave
> > Anderson
> > Sent: Thursday, March 15, 2012 9:31 PM
> > To: Discussion list for crash utility usage, maintenance and
> > development
> > Subject: Re: [Crash-utility] The "ctrl + c" doesn't work in
crash
> >
> >
> >
> > ----- Original Message -----
> > > Hi Dave,
> > >
> > > I found the "ctrl + c" doesn't work well in crash.
Sometimes, we
> > > may
> > > type a wrong command and want to directly cancel it by "ctrl+c"
> > > like
> > > in the bash. But this combination only do seldom response, what I
> > > mean is I once see it works once or twice.
> > >
> > > I also notice there was a mail thread talking about it:
> > >
http://www.redhat.com/archives/crash-utility/2009-October/msg00006.html
> > >
> > > But on the most new crash 6.0.4, simply ctrl+c three times or
> > > more
> > > would not lead to a new prompt...
> >
> > SIGINT's are always caught, and it depends upon whether the command is
> > currently being fed into the scroller, and in some cases, the command
> > itself. See the callers of received_SIGINT() for example.
>
> I see...
> And while I redo the test, I can see SIGINT work again, prompt the
> input > when type three ctrl+c. No idea why previous ctrl+c not
> working...
>
> I still get a another display issue regarding the ctrl+c.
> I use the "log" command to show the kernel dmesg, and always the
> message buffer is large which cannot be displayed in one screen. So
> I could see the below notice in the foot of the screen:
> -- MORE -- forward: <SPACE>, <ENTER> or j backward: b or k quit: q
>
> If I type "ctrl+c" when the message still not get fully displayed,
> eventually I would see the previous only one foot notice would show
> up several and keep growing.
> Like this:
> [ 413.741638] Unable to handle kernel NULL pointer dereference at virtual
address 00000000
> [ 413.749725] pgd = d3e90000
> [ 413.752441] [00000000] *pgd=12109831, *pte=00000000, *ppte=00000000
> -- MORE -- forward: <SPACE>, <ENTER> or j backward: b or k quit: q
> -- MORE -- forward: <SPACE>, <ENTER> or j backward: b or k quit: q
> -- MORE -- forward: <SPACE>, <ENTER> or j backward: b or k quit: q
>
> And when this abnormal happens, the <space> would be displayed
> normally as before, but when the whole message is drained out, the
> console prompt would not be displayed. And each <enter> after this
> abnormal, would lead to another such notice to be showed up like the
> message buffer itself.
>
> So is it a bug for the cmdline handling?
Well, it's just the behavior when the "less" scroller is being used.
Why not just do what the prompt says, and just enter "q"?
Dave
I don't fully understand your description, but why not do what the
prompt says, and just type 'q'?
Yep, the 'q' certainly works. Just for testing. :)
Dave
Thanks,
Lei
10:31 a.m.
----- Original Message -----
At 2012-3-13 11:45, qiaonuohan wrote:
> At 2012-3-13 3:27, Dave Anderson wrote:
>>
>>
>> ----- Original Message -----
>>> At 2012-3-10 5:50, Dave Anderson wrote:
>>>>
>>>> It's failing in its call to get_pathname(). I haven't had a
chance to
>>>
>>> You are right. I forgot to check every call to get_pathname. The new
>>> patch has fixed the problem of swap.
>>
>> Although the patch appears to work in the simple cases that I have tested,
>> I have a few issues with it.
>>
>> First, the get_pathname() function prototype is -- and has always been -- this:
>>
>> void get_pathname(ulong dentry, char *pathname, int length, int full, ulong
vfsmnt)
>>
>> where the "vfsmnt" argument has always been a struct vfsmount
pointer.
>>
>> With your patch, prior to making get_pathname() calls, you typically
>> make a VALID_STRUCT(mount) call first, and if true, you modify the
>> vfsmnt argument to be a struct mount pointer.
>>
>> It would make more sense maintenance-wise, and would simplify the patch,
>> if the get_pathname() "vfsmnt" argument would continue to be a struct
>> vfsmount pointer. And at the top of get_pathname(), you could make the
>> adjustments for when it is embedded inside a struct mount.
... [ cut ] ...
>> If we can continue to have get_pathname() receive a vfsmount
pointer,
>> it reduces the chance of introducing new bugs like the above, and future
>> maintenance will be easier.
>
> It does reduce complication of the code. Still, places needing the
> judgement of struct mount's existing remain. I will modify the patch,
> and post later.
>
> About the function, OFFSET_OPTION(), thanks for your advice.
>
The attachment is the modified patch.
I have reviewed and tested this patch, and it looks to be both complete
and simpler to maintain. Queued for crash-6.0.5.
Thanks again for undertaking this task,
Dave
5049
days inactive
5056
days old
devel@lists.crash-utility.osci.io
18 comments
3 participants
participants (3)
-
Dave Anderson -
Lei Wen -
qiaonuohan