On Wed, Jun 12, 2024 at 2:08 PM <devel-requ...@lists.crash-utility.osci.io>
wrote:

> Date: Wed, 12 Jun 2024 06:06:35 +0000
> From: HAGIO KAZUHITO(萩尾 一仁) <k-hagio...@nec.com>
> Subject: [Crash-utility] Re: PATCH] Fix "kmem -i" and "swap" commands
>         on Linux 6.10-rc1 and later kernels
> To: lijiang <liji...@redhat.com>, "devel@lists.crash-utility.osci.io"
>         <devel@lists.crash-utility.osci.io>
> Message-ID: <2db40a61-d404-4733-a62a-6068f2911...@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-requ...@lists.crash-utility.osci.io <mailto:
> devel-requ...@lists.crash-utility.osci.io>> wrote:
> >
> >     Date: Tue, 11 Jun 2024 02:40:55 +0000
> >     From: HAGIO KAZUHITO(萩尾 一仁) <k-hagio...@nec.com <mailto:
> k-hagio...@nec.com>>
> >     Subject: [Crash-utility] [PATCH] Fix "kmem -i" and "swap" commands on
> >              Linux 6.10-rc1 and later kernels
> >     To: "devel@lists.crash-utility.osci.io <mailto:
> devel@lists.crash-utility.osci.io>"
> >              <devel@lists.crash-utility.osci.io <mailto:
> devel@lists.crash-utility.osci.io>>
> >     Message-ID: <1718073652-13444-1-git-send-email-k-hagio...@nec.com
> <mailto:1718073652-13444-1-git-send-email-k-hagio...@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...@nec.com <mailto:
> k-hagio...@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 <torva...@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
> >
>
--
Crash-utility mailing list -- devel@lists.crash-utility.osci.io
To unsubscribe send an email to devel-le...@lists.crash-utility.osci.io
https://${domain_name}/admin/lists/devel.lists.crash-utility.osci.io/
Contribution Guidelines: https://github.com/crash-utility/crash/wiki

Reply via email to