On 2023/03/22 22:20, lijiang wrote:
>> So how about adding a sentence saying it?
>>
> -M mm if the mm_struct address has been removed from the
>> task_struct
>> of an exiting task, the virtual memory data cannot be
>> displayed.
>> However, if the address can be determined from the kernel
>> stack,
>> it can be entered manually in order to try to resurrect
>> the
>> - virtual memory data of the task.
>> + virtual memory data of the task. This option is ignored
>> when
>> + the task_struct has a non-NULL mm_struct address.
>>
>>
> This looks much better. But it still accepts an invalid argument, for
> example:
> crash> vm -M 0xh
> crash> vm -M 0xdeadbeef
>
> How about the following changes?
>
> diff --git a/help.c b/help.c
> index 56a9d8274525..3006ac9180ae 100644
> --- a/help.c
> +++ b/help.c
> @@ -4695,6 +4695,7 @@ char *help_vm[] = {
> " However, if the address can be determined from the kernel
> stack,",
> " it can be entered manually in order to try to resurrect
> the",
> " virtual memory data of the task.",
> +" NOTE: this option is only used when the task_struct's mm
> is NULL.",
> " -R reference search for references to this number or filename.",
> " -m dump the mm_struct associated with the task.",
> " -v dump all of the vm_area_structs associated with the
> task.",
> diff --git a/memory.c b/memory.c
> index 592a5ef49d50..0968ff930eb9 100644
> --- a/memory.c
> +++ b/memory.c
> @@ -3559,6 +3559,8 @@ cmd_vm(void)
> case 'M':
> pc->curcmd_private = htoll(optarg, FAULT_ON_ERROR, NULL);
> pc->curcmd_flags |= MM_STRUCT_FORCE;
> + if (!IS_KVADDR(pc->curcmd_private))
> + argerrs++;
> break;
>
> case 'f':
>
>
> With the above patch, it can properly deal with an invalid argument as
> below:
>
> crash> vm -M 0xh
> Usage:
> vm [-p | -P vma | -M mm | -v | -m | -x | -d | [-R reference] [pid | task]]
> [-f vm_flags]
> Enter "help vm" for details.
> crash> vm -M 0xdeadbeef
> Usage:
> vm [-p | -P vma | -M mm | -v | -m | -x | -d | [-R reference] [pid | task]]
> [-f vm_flags]
> Enter "help vm" for details.
It's fine to check here, but shall we say why it's wrong?
I'd prefer something like this.
if (!IS_KVADDR(pc->curcmd_private))
error(FATAL, "invalid mm_struct address: %s\n", optarg);
Thanks,
Kazu
--
Crash-utility mailing list
Crash-utility@xxxxxxxxxx
https://listman.redhat.com/mailman/listinfo/crash-utility
Contribution Guidelines: https://github.com/crash-utility/crash/wiki
