On 2023/05/30 21:01, lijiang wrote:
> On Fri, May 26, 2023 at 9:56 AM HAGIO KAZUHITO(萩尾 一仁) <k-hagio-ab@xxxxxxx>
> wrote:
>
>> On 2023/05/25 11:44, HAGIO KAZUHITO(萩尾 一仁) wrote:
>>> On 2023/05/24 16:10, lijiang wrote:
>>>> On Thu, May 11, 2023 at 12:35 PM HAGIO KAZUHITO(萩尾 一仁) <
>> k-hagio-ab@xxxxxxx>
>>>> wrote:
>>>>
>>>>> To "sym -m" print the symbols of a module in address order.
>>>>> (but "sym -l" and "sym -M" still print modules in text address order.)
>>>>>
>>>>>
>>>> The current text address order is better to me, basically it can keep
>> the
>>>> same order with the definition mod_mem_type, which looks more natural.
>> For
>>>> example:
>>>> crash> sym -m kvm |grep MODULE
>>>> ffffffffc136e000 MODULE TEXT START: kvm
>>>> ffffffffc13e0000 MODULE TEXT END: kvm
>>>> ffffffffc1ceb000 MODULE DATA START: kvm
>>>> ffffffffc1d6b000 MODULE DATA END: kvm
>>>> ffffffffc291a000 MODULE RODATA START: kvm
>>>> ffffffffc296c000 MODULE RODATA END: kvm
>>>> ffffffffc11cf000 MODULE RO_AFTER_INIT START: kvm
>>>> ffffffffc11d1000 MODULE RO_AFTER_INIT END: kvm
>>
>> sorry, apparently I misunderstood your comments.
>>
>> Does this mean that sorting memory types in a module by address is
>> unnecessary?
>>
>>
> Yes. In fact, the above output is sorted naturally by the enum mod_mem_type
> order.
> At least for me, the *text address order* is more friendly and readable to
> me.
ok, I see. I perfer sorted output in a module, but we can use sort
command if we want.
The following comments are just for information.
>
>
>>>>
>>>> And the internal output in each module memory type is sorted by address
>> as
>>>> below:
>>
>> Yes, I see this.
>>
>>>> crash> sym -m kvm
>>>> ffffffffc136e000 MODULE TEXT START: kvm
>>>> ffffffffc136e000 (T) __pfx___traceiter_kvm_userspace_exit
>>>> ffffffffc136e010 (T) __traceiter_kvm_userspace_exit
>>>> ffffffffc136e050 (T) __pfx___traceiter_kvm_vcpu_wakeup
>>>> ffffffffc136e060 (T) __traceiter_kvm_vcpu_wakeup
>>>> ffffffffc136e0b0 (T) __pfx___traceiter_kvm_set_irq
>>>> ffffffffc136e0c0 (T) __traceiter_kvm_set_irq
>>>> ffffffffc136e110 (T) __pfx___traceiter_kvm_ioapic_set_irq
>>>> ffffffffc136e120 (T) __traceiter_kvm_ioapic_set_irq
>>>> ffffffffc136e170 (T) __pfx___traceiter_kvm_ioapic_delayed_eoi_inj
>>>> ...
>>>> ffffffffc13df650 (t) kvm_x86_exit
>>>> ffffffffc13df650 (T) cleanup_module
>>>> ffffffffc13e0000 MODULE TEXT END: kvm
>>>>
>>>> In addition, the sym -m option will be consistent with the styles of sym
>>>> -l/-M options,
>>
>> What does this mean?
>>
>
> Because you mentioned that(in patch log):
> "(but "sym -l" and "sym -M" still print modules in text address order.)"
This means that *modules* are sorted in text address order, and does not
say about the order of output in "sym -m".
>
> Without this change, the "sym -m" has also the same output style as the
> "sym -l/-M" options.
Even with this patch, "sym -m|-L|-M" have the same output for *a module*
as below:
>
> This patch's "sym -m" is already consistent with the "sym -l|-M" options:
>>
>> crash-6.4> sym -m cdrom | grep MODULE
>> ffffffffc084d000 MODULE RO_AFTER_INIT START: cdrom
>> ffffffffc084e000 MODULE RO_AFTER_INIT END: cdrom
>> ffffffffc08da000 MODULE TEXT START: cdrom
>> ffffffffc08e1000 MODULE TEXT END: cdrom
>> ffffffffc0aa3000 MODULE RODATA START: cdrom
>> ffffffffc0aa8000 MODULE RODATA END: cdrom
>> ffffffffc0b04000 MODULE DATA START: cdrom
>> ffffffffc0b0b000 MODULE DATA END: cdrom
>> crash-6.4> sym -l | grep MODULE
>> ...
>> ffffffffc084d000 MODULE RO_AFTER_INIT START: cdrom
>> ffffffffc084e000 MODULE RO_AFTER_INIT END: cdrom
>> ffffffffc08da000 MODULE TEXT START: cdrom
>> ffffffffc08e1000 MODULE TEXT END: cdrom
>> ffffffffc0aa3000 MODULE RODATA START: cdrom
>> ffffffffc0aa8000 MODULE RODATA END: cdrom
>> ffffffffc0b04000 MODULE DATA START: cdrom
>> ffffffffc0b0b000 MODULE DATA END: cdrom
>> ...
>>
>> > and really simplify the code.
>>
>> Does this mean that we can remove lm->address_order and related code?
>>
>> Yes, that is my original thoughts, but you mentioned the following patches
> will use it to speed up searching addresses. I'm not sure how much it would
> benefit as I do not have the test(performance) data.
Sorry, also I don't have any performance data.
>
> I have another issue: the variable address_order is initialized once in
> the store_module_symbols_v3()(called by the module_init()), how does it
> sort the modules loaded by "mod -s" command in address order? The
> relevant data can be updated to the "address_order", or not needed?
The order does not change after "mod -s", so no need.
>
> btw, this facility is helpful to speed up searching addresses in some
>> codes, e.g. patch 05/15. I think this is a good preprocessing.
>>
>>
> I don't have any performance data about it, although it might speed up
> searching addresses from the code perspective.
>
> If the performance can significantly benefit from these changes, it's worth
> doing that. But we can still keep the text address order, no need to make
> the following changes, seems they have no conflicts?
hmm, sorry I don't get this. it feels like "the text address order"
is different from what I meant...
Anyway, that pre-processing will be removed. If it is very slow,
then we can tune later.
Thanks,
Kazu
>
> @@ -1317,7 +1317,8 @@ module_symbol_dump(char *module)
> if (received_SIGINT() || output_closed())
> return;
>
> - for (j = MOD_TEXT; j < MOD_MEM_NUM_TYPES; j++) {
> + for (m = 0; m < lm->nr_mems; m++) {
> + j = lm->address_order[m];
>
> Thanks.
> Lianbo
>
>
--
Crash-utility mailing list
Crash-utility@xxxxxxxxxx
https://listman.redhat.com/mailman/listinfo/crash-utility
Contribution Guidelines: https://github.com/crash-utility/crash/wiki
