Hi Tao,
thank you for your review.
On 2024/06/28 9:19, Tao Liu wrote:
> Hi Kazu,
>
> sorry for the late response.
>
> On Tue, Jun 11, 2024 at 2:41 PM HAGIO KAZUHITO(萩尾 一仁)
> <k-hagio-ab@xxxxxxx> wrote:
>>
>> 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@xxxxxxx>
>> ---
>> 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");
>>
>> 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));
>>
>
> The above code change is OK to me, just my curiosity: it looks to me
> the swap_file->f_inode->i_mode->S_ISBLK approach will always work, not
> only for the Linux 6.10 and later versions right? E.g. I made the
> following change which works as expected on a 4.0.4-201 kernel:
This S_ISBLK approach will work with kernels that swap_file is 'file',
and at least 2.6.0 and later kernels have it, as far as I checked (as
written in [1]).
but I didn't want to change the current behavior (the else block) for
6.9 and older kernels, because it has worked well with them for a long
time and there is no need to change.
Does this answer your question?
[1]
https://lists.crash-utility.osci.io/archives/list/devel@xxxxxxxxxxxxxxxxxxxxxxxxxxx/message/46C66KSU6JN3RURAPMQAKON6TPHCLVNW/
Thanks,
Kazu
>
> @@ -16121,9 +16121,7 @@ dump_swap_info(ulong swapflags, ulong
> *totalswap_pages, ulong *totalused_pages)
> OFFSET(swap_info_struct_swap_file));
>
> /* 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) {
> + if (1) {
> ulong inode;
> ushort mode;
> readmem(swap_file + OFFSET(file_f_inode),
> KVADDR, &inode,
>
> KERNEL: burn.khw2.lab.eng.bos.redhat.com/4.0.4-201.fc21_sysrq-c/vmlinux.gz
> DUMPFILE: burn.khw2.lab.eng.bos.redhat.com/4.0.4-201.fc21_sysrq-c/vmcore
> [PARTIAL DUMP]
> CPUS: 4
> DATE: Wed May 27 14:23:22 EDT 2015
> UPTIME: 02:47:56
> LOAD AVERAGE: 0.00, 0.04, 0.05
> TASKS: 135
> NODENAME: dell-pec5125-04.lab.bos.redhat.com
> RELEASE: 4.0.4-201.fc21.x86_64
> VERSION: #1 SMP Thu May 21 15:58:47 UTC 2015
> MACHINE: x86_64 (2599 Mhz)
> MEMORY: 2 GB
> PANIC: "sysrq: SysRq : Trigger a crash"
> PID: 14037
> COMMAND: "bash"
> TASK: ffff88006a03d970 [THREAD_INFO: ffff880074734000]
> CPU: 1
> STATE: TASK_RUNNING (SYSRQ)
>
> crash> swap
> SWAP_INFO_STRUCT TYPE SIZE USED PCT PRI FILENAME
> ffff88007bf62000 PARTITION 2097148k 0k 0% -1 /dev/dm-1
> crash> kmem -i
> PAGES TOTAL PERCENTAGE
> TOTAL MEM 445983 1.7 GB ----
> FREE 275307 1.1 GB 61% of TOTAL MEM
> ...
>
> Thanks,
> Tao Liu
>
>> 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@xxxxxxxxxxxxxxxxxxxxxxxxxxx
>> To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxxxxxxxxxxxx
>> https://${domain_name}/admin/lists/devel.lists.crash-utility.osci.io/
>> Contribution Guidelines: https://github.com/crash-utility/crash/wiki
--
Crash-utility mailing list -- devel@xxxxxxxxxxxxxxxxxxxxxxxxxxx
To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxxxxxxxxxxxx
https://${domain_name}/admin/lists/devel.lists.crash-utility.osci.io/
Contribution Guidelines: https://github.com/crash-utility/crash/wiki
