1:34 a.m.
On Wed, Jun 12, 2024 at 2:08 PM <devel-request(a)lists.crash-utility.osci.io>
wrote:
Date: Wed, 12 Jun 2024 06:06:35 +0000
From: HAGIO KAZUHITO(萩尾 一仁) <k-hagio-ab(a)nec.com>
Subject: [Crash-utility] Re: PATCH] Fix "kmem -i" and "swap"
commands
on Linux 6.10-rc1 and later kernels
To: lijiang <lijiang(a)redhat.com>, "devel(a)lists.crash-utility.osci.io"
<devel(a)lists.crash-utility.osci.io>
Message-ID: <2db40a61-d404-4733-a62a-6068f2911346(a)nec.com>
Content-Type: text/plain; charset="utf-8"
On 2024/06/12 13:55, lijiang wrote:
> Thank you for the fix, Kazu.
>
> On Tue, Jun 11, 2024 at 10:42 AM <
devel-request(a)lists.crash-utility.osci.io <mailto:
devel-request(a)lists.crash-utility.osci.io>> wrote:
>
> Date: Tue, 11 Jun 2024 02:40:55 +0000
> From: HAGIO KAZUHITO(萩尾 一仁) <k-hagio-ab(a)nec.com <mailto:
k-hagio-ab(a)nec.com>>
> Subject: [Crash-utility] [PATCH] Fix "kmem -i" and "swap"
commands on
> Linux 6.10-rc1 and later kernels
> To: "devel(a)lists.crash-utility.osci.io <mailto:
devel(a)lists.crash-utility.osci.io>"
> <devel(a)lists.crash-utility.osci.io <mailto:
devel(a)lists.crash-utility.osci.io>>
> Message-ID: <1718073652-13444-1-git-send-email-k-hagio-ab(a)nec.com
<mailto:1718073652-13444-1-git-send-email-k-hagio-ab@nec.com>>
> Content-Type: text/plain; charset="iso-2022-jp"
>
> Kernel commit 798cb7f9aec3 ("swapon(2)/swapoff(2): don't bother with
> block size") removed swap_info_struct.old_block_size member at Linux
> 6.10-rc1. The crash-utility has used this to determine whether a
swap
> is a partition or file and to determine the way to get the swap path.
>
> Withtout the patch, the "kmem -i" and "swap" commands fail
with the
> following error messsage:
>
> crash> kmem -i
> ...
> TOTAL HUGE 13179392 50.3 GB ----
> HUGE FREE 13179392 50.3 GB 100% of TOTAL HUGE
>
> swap: invalid (optional) structure member offsets:
swap_info_struct_swap_device or swap_info_struct_old_block_size
> FILE: memory.c LINE: 16032 FUNCTION: dump_swap_info()
>
> The swap_file member of recent swap_info_struct is a pointer to a
> struct file (once upon a time it was dentry), use this fact directly.
>
> Signed-off-by: Kazuhito Hagio <k-hagio-ab(a)nec.com <mailto:
k-hagio-ab(a)nec.com>>
> ---
> NOTE: This patch can be applied on top of my '[PATCH] Fix "kmem
-v"
> option on Linux 6.9 and later kernels' patch. Otherwise, please
adjust
> the hunk for offset_table.
>
> defs.h | 1 +
> filesys.c | 1 +
> memory.c | 28 +++++++++++++++++++++++-----
> symbols.c | 1 +
> 4 files changed, 26 insertions(+), 5 deletions(-)
>
> diff --git a/defs.h b/defs.h
> index 95de33188070..64bf785b7a1b 100644
> --- a/defs.h
> +++ b/defs.h
> @@ -2242,6 +2242,7 @@ struct offset_table { /*
stash of commonly-used offsets */
> long log_caller_id;
> long vmap_node_busy;
> long rb_list_head;
> + long file_f_inode;
> };
>
> struct size_table { /* stash of commonly-used sizes */
> diff --git a/filesys.c b/filesys.c
> index 81fe856699e1..406ebb299780 100644
> --- a/filesys.c
> +++ b/filesys.c
> @@ -2038,6 +2038,7 @@ vfs_init(void)
> MEMBER_OFFSET_INIT(file_f_dentry, "file",
"f_dentry");
> MEMBER_OFFSET_INIT(file_f_vfsmnt, "file",
"f_vfsmnt");
> MEMBER_OFFSET_INIT(file_f_count, "file",
"f_count");
> + MEMBER_OFFSET_INIT(file_f_inode, "file",
"f_inode");
> MEMBER_OFFSET_INIT(path_mnt, "path", "mnt");
> MEMBER_OFFSET_INIT(path_dentry, "path", "dentry");
> if (INVALID_MEMBER(file_f_dentry)) {
> diff --git a/memory.c b/memory.c
> index acb8507cfb75..28f901f16adf 100644
> --- a/memory.c
> +++ b/memory.c
> @@ -16075,6 +16075,8 @@ dump_swap_info(ulong swapflags, ulong
*totalswap_pages, ulong *totalused_pages)
> char buf3[BUFSIZE];
> char buf4[BUFSIZE];
> char buf5[BUFSIZE];
> + int swap_file_is_file =
> + STREQ(MEMBER_TYPE_NAME("swap_info_struct",
"swap_file"), "file");
>
>
> This patch looks good, but I still have one question:
> The 'swap_file' has been added into the struct swap_info_struct since
linux v2.6.12-rc2,
> and until now it(its type) has been never changed, it indicates that the
above checking is always true.
>
> STREQ(MEMBER_TYPE_NAME("swap_info_struct", "swap_file"),
"file")
>
> Is that expected behavior? Just want to confirm with you.
Yes.
Thank you for the confirmation, and explanation below, Kazu.
As far as I checked, swap_file was changed from 'dentry *' to 'file *'
at somewhere between v2.4.0 and v2.6.0, so not always true.
Seems no full history in linux github. I checked it from the linux initial
git repo:
commit 1da177e4c3f41524e886b7f1b8a0c1fc7321cac2 (tag: v2.6.12-rc2)
Author: Linus Torvalds <torvalds(a)ppc970.osdl.org>
Date: Sat Apr 16 15:20:36 2005 -0700
Linux-2.6.12-rc2
Initial git repository build. I'm not bothering with the full history,
even though we have it. We can create a separate "historical" git
archive of that later if we want to, and in the meantime it's about
3.2GB when imported into git - space that would just make the early
git days unnecessarily complicated, when we don't have a lot of good
infrastructure for it.
Let it rip!
get_pathname()'s first arg is dentry, we have to choose the else-if block
without old_block_size, so I decided to use its type information
directly
} else if
(VALID_MEMBER(swap_info_struct_old_block_size) ||
swap_file_is_file) {
The other if / else blocks are for very old kernels, so maybe they won't
be used, but if we don't remove these, I think this is a reasonable way.
Sounds good to me.
Does this answer your question?
Yes. Thank you, Kazu. I have no other issues. For the patch: Ack.
Thanks
Lianbo
Thanks,
Kazu
>
> Thanks
> Lianbo
>
> if (!symbol_exists("nr_swapfiles"))
> error(FATAL, "nr_swapfiles doesn't exist in this
kernel!\n");
> @@ -16118,9 +16120,21 @@ dump_swap_info(ulong swapflags, ulong
*totalswap_pages, ulong *totalused_pages)
> swap_file = ULONG(vt->swap_info_struct +
> OFFSET(swap_info_struct_swap_file));
>
> - swap_device = INT(vt->swap_info_struct +
> - OFFSET_OPTION(swap_info_struct_swap_device,
> - swap_info_struct_old_block_size));
> + /* Linux 6.10 and later */
> + if (INVALID_MEMBER(swap_info_struct_swap_device) &&
> + INVALID_MEMBER(swap_info_struct_old_block_size)
&&
> + swap_file_is_file) {
> + ulong inode;
> + ushort mode;
> + readmem(swap_file + OFFSET(file_f_inode),
KVADDR, &inode,
> + sizeof(ulong), "swap_file.f_inode",
FAULT_ON_ERROR);
> + readmem(inode + OFFSET(inode_i_mode),
KVADDR, &mode,
> + sizeof(ushort), "inode.i_mode",
FAULT_ON_ERROR);
> + swap_device = S_ISBLK(mode);
> + } else
> + swap_device = INT(vt->swap_info_struct +
> +
OFFSET_OPTION(swap_info_struct_swap_device,
> + swap_info_struct_old_block_size));
>
> pages = INT(vt->swap_info_struct +
> OFFSET(swap_info_struct_pages));
> @@ -16161,8 +16175,12 @@ dump_swap_info(ulong swapflags, ulong
*totalswap_pages, ulong *totalused_pages)
>
OFFSET(swap_info_struct_swap_vfsmnt));
> get_pathname(swap_file, buf,
BUFSIZE,
> 1, vfsmnt);
> - } else if (VALID_MEMBER
> - (swap_info_struct_old_block_size)) {
> + } else if
(VALID_MEMBER(swap_info_struct_old_block_size) ||
> + swap_file_is_file) {
> + /*
> + * Linux 6.10 and later kernels do
not have old_block_size,
> + * but this still should work, if
swap_file is file.
> + */
> devname =
vfsmount_devname(file_to_vfsmnt(swap_file),
> buf1, BUFSIZE);
>
get_pathname(file_to_dentry(swap_file),
> diff --git a/symbols.c b/symbols.c
> index 283b98a8fbfe..0bf050ab62d0 100644
> --- a/symbols.c
> +++ b/symbols.c
> @@ -10522,6 +10522,7 @@ dump_offset_table(char *spec, ulong
makestruct)
> OFFSET(file_f_count));
> fprintf(fp, " file_f_path: %ld\n",
> OFFSET(file_f_path));
> + fprintf(fp, " file_f_inode: %ld\n",
OFFSET(file_f_inode));
> fprintf(fp, " path_mnt: %ld\n",
> OFFSET(path_mnt));
> fprintf(fp, " path_dentry: %ld\n",
> --
> 2.31.1
>
7:01 p.m.
New subject: PATCH] Fix "kmem -i" and "swap" commands on Linux 6.10-rc1 and later kernels
On 2024/06/12 15:34, lijiang wrote:
> --- a/memory.c
> +++ b/memory.c
> @@ -16075,6 +16075,8 @@ dump_swap_info(ulong swapflags, ulong
*totalswap_pages, ulong *totalused_pages)
> char buf3[BUFSIZE];
> char buf4[BUFSIZE];
> char buf5[BUFSIZE];
> + int swap_file_is_file =
> + STREQ(MEMBER_TYPE_NAME("swap_info_struct",
"swap_file"), "file");
>
>
> This patch looks good, but I still have one question:
> The 'swap_file' has been added into the struct swap_info_struct since
linux v2.6.12-rc2,
> and until now it(its type) has been never changed, it indicates that the above
checking is always true.
>
> STREQ(MEMBER_TYPE_NAME("swap_info_struct", "swap_file"),
"file")
>
> Is that expected behavior? Just want to confirm with you.
Yes.
Thank you for the confirmation, and explanation below, Kazu.
As far as I checked, swap_file was changed from 'dentry *' to 'file
*'
at somewhere between v2.4.0 and v2.6.0, so not always true.
Seems no full history in linux github. I checked it from the linux initial git repo:
commit 1da177e4c3f41524e886b7f1b8a0c1fc7321cac2 (tag: v2.6.12-rc2)
Author: Linus Torvalds <torvalds(a)ppc970.osdl.org
<mailto:torvalds@ppc970.osdl.org>>
Date: Sat Apr 16 15:20:36 2005 -0700
Linux-2.6.12-rc2
Initial git repository build. I'm not bothering with the full history,
even though we have it. We can create a separate "historical" git
archive of that later if we want to, and in the meantime it's about
3.2GB when imported into git - space that would just make the early
git days unnecessarily complicated, when we don't have a lot of good
infrastructure for it.
Let it rip!
get_pathname()'s first arg is dentry, we have to choose the else-if block
without old_block_size, so I decided to use its type information directly
} else if (VALID_MEMBER(swap_info_struct_old_block_size)
||
swap_file_is_file) {
The other if / else blocks are for very old kernels, so maybe they won't
be used, but if we don't remove these, I think this is a reasonable way.
Sounds good to me.
Does this answer your question?
Yes. Thank you, Kazu. I have no other issues. For the patch: Ack.
Good, thank you for your ack.
btw, I forgot to add the From: line below to the commit log, it would be
helpful if you would add this when merging:
From: Kazuhito Hagio <k-hagio-ab(a)nec.com>
Thanks,
Kazu
8:35 p.m.
New subject: PATCH] Fix "kmem -i" and "swap" commands on Linux 6.10-rc1 and later kernels
On Thu, Jun 13, 2024 at 8:02 AM HAGIO KAZUHITO(萩尾 一仁) <k-hagio-ab(a)nec.com>
wrote:
On 2024/06/12 15:34, lijiang wrote:
> > --- a/memory.c
> > +++ b/memory.c
> > @@ -16075,6 +16075,8 @@ dump_swap_info(ulong swapflags, ulong
*totalswap_pages, ulong *totalused_pages)
> > char buf3[BUFSIZE];
> > char buf4[BUFSIZE];
> > char buf5[BUFSIZE];
> > + int swap_file_is_file =
> > + STREQ(MEMBER_TYPE_NAME("swap_info_struct",
"swap_file"), "file");
> >
> >
> > This patch looks good, but I still have one question:
> > The 'swap_file' has been added into the struct swap_info_struct
since linux v2.6.12-rc2,
> > and until now it(its type) has been never changed, it indicates
that the above checking is always true.
> >
> > STREQ(MEMBER_TYPE_NAME("swap_info_struct",
"swap_file"), "file")
> >
> > Is that expected behavior? Just want to confirm with you.
>
> Yes.
>
>
> Thank you for the confirmation, and explanation below, Kazu.
>
> As far as I checked, swap_file was changed from 'dentry *' to 'file
*'
> at somewhere between v2.4.0 and v2.6.0, so not always true.
>
> Seems no full history in linux github. I checked it from the linux
initial git repo:
>
> commit 1da177e4c3f41524e886b7f1b8a0c1fc7321cac2 (tag: v2.6.12-rc2)
> Author: Linus Torvalds <torvalds(a)ppc970.osdl.org <mailto:
torvalds(a)ppc970.osdl.org>>
> Date: Sat Apr 16 15:20:36 2005 -0700
>
> Linux-2.6.12-rc2
>
> Initial git repository build. I'm not bothering with the full
history,
> even though we have it. We can create a separate "historical" git
> archive of that later if we want to, and in the meantime it's about
> 3.2GB when imported into git - space that would just make the early
> git days unnecessarily complicated, when we don't have a lot of good
> infrastructure for it.
>
> Let it rip!
>
>
> get_pathname()'s first arg is dentry, we have to choose the else-if
block
> without old_block_size, so I decided to use its type information
directly
>
> } else if
(VALID_MEMBER(swap_info_struct_old_block_size) ||
> swap_file_is_file) {
>
> The other if / else blocks are for very old kernels, so maybe they
won't
> be used, but if we don't remove these, I think this is a reasonable
way.
>
> Sounds good to me.
>
> Does this answer your question?
>
> Yes. Thank you, Kazu. I have no other issues. For the patch: Ack.
Good, thank you for your ack.
btw, I forgot to add the From: line below to the commit log, it would be
helpful if you would add this when merging:
From: Kazuhito Hagio <k-hagio-ab(a)nec.com>
Ok, thanks for the info.
Lianbo
Thanks,
Kazu
505
days inactive
506
days old
devel@lists.crash-utility.osci.io
2 comments
2 participants
participants (2)
-
HAGIO KAZUHITO(萩尾 一仁) -
lijiang