On 2023/05/26 21:40, lijiang wrote:
> On Thu, May 11, 2023 at 12:35 PM HAGIO KAZUHITO(萩尾 一仁) <k-hagio-ab@xxxxxxx>
> wrote:
>
>> Signed-off-by: Kazuhito Hagio <k-hagio-ab@xxxxxxx>
>> ---
>> symbols.c | 70 ++++++++++++++++++++++++++++++++++++++++++++++++-------
>> 1 file changed, 61 insertions(+), 9 deletions(-)
>>
>> diff --git a/symbols.c b/symbols.c
>> index 40e992e9ee12..5399c7494cb1 100644
>> --- a/symbols.c
>> +++ b/symbols.c
>> @@ -13565,7 +13565,7 @@ append_section_symbols:
>> void
>> delete_load_module(ulong base_addr)
>> {
>> - int i;
>> + int i, j;
>> struct load_module *lm;
>> struct gnu_request request, *req;
>>
>> @@ -13580,7 +13580,23 @@ delete_load_module(ulong base_addr)
>> req->name = lm->mod_namelist;
>> gdb_interface(req);
>> }
>> - mod_symtable_hash_remove_range(lm->mod_symtable,
>> lm->mod_symend);
>> + if (MODULE_MEMORY()) {
>> + if (lm->mod_load_symtable) {
>> +
>> mod_symtable_hash_remove_range(lm->mod_load_symtable, lm->mod_load_symend);
>>
>
> The following code can be replaced with the macro for_each_mod_mem_type()
> or memset(), for example:
> memset(lm->load_symtable, 0, MOD_MEM_NUM_TYPES-1);
> memset(lm->load_symend, 0, MOD_MEM_NUM_TYPES-1);
>
> The disadvantage is to invoke the memset() twice, but the code looks very
> clean.
ok, thanks.
>
> + for (j = MOD_TEXT; j <
>> MOD_MEM_NUM_TYPES; j++) {
>> + lm->load_symtable[j] =
>> NULL;
>> + lm->load_symend[j] = NULL;
>> + }
>> + } else { /* somehow this function runs for
>> unloaded modules */
>>
>
> Could you please explain why it may get into the "else" code path? And why
> do we need to handle this situation for now? But not needed before the
> module memory changes.
This path is used by "mod -d|-D" and reinit_modules() for unloaded modules.
The old procedure always does the cleanup and reinstall of the hash
entries, so kept it as it is.
Thanks,
Kazu
>
>
>> + for (j = MOD_TEXT; j <
>> MOD_MEM_NUM_TYPES; j++) {
>> + if (!lm->symtable[j])
>> + continue;
>> +
>> mod_symtable_hash_remove_range(lm->symtable[j], lm->symend[j]);
>> + }
>> + }
>> + } else
>> +
>> mod_symtable_hash_remove_range(lm->mod_symtable, lm->mod_symend);
>> +
>> if (lm->mod_load_symtable) {
>> free(lm->mod_load_symtable);
>> namespace_ctl(NAMESPACE_FREE,
>> @@ -13588,9 +13604,19 @@ delete_load_module(ulong base_addr)
>> }
>> if (lm->mod_flags & MOD_REMOTE)
>> unlink_module(lm);
>> - lm->mod_symtable = lm->mod_ext_symtable;
>> - lm->mod_symend = lm->mod_ext_symend;
>> - mod_symtable_hash_install_range(lm->mod_symtable,
>> lm->mod_symend);
>> + if (MODULE_MEMORY()) {
>> + lm->symtable = lm->ext_symtable;
>> + lm->symend = lm->ext_symend;
>> + for (j = MOD_TEXT; j < MOD_MEM_NUM_TYPES;
>> j++) {
>> + if (!lm->symtable[j])
>> + continue;
>> +
>> mod_symtable_hash_install_range(lm->symtable[j], lm->symend[j]);
>> + }
>> + } else {
>> + lm->mod_symtable = lm->mod_ext_symtable;
>> + lm->mod_symend = lm->mod_ext_symend;
>> +
>> mod_symtable_hash_install_range(lm->mod_symtable, lm->mod_symend);
>> + }
>> lm->mod_flags &=
>> ~(MOD_LOAD_SYMS|MOD_REMOTE|MOD_NOPATCH);
>> lm->mod_flags |= MOD_EXT_SYMS;
>> lm->mod_load_symtable = NULL;
>> @@ -13619,7 +13645,23 @@ delete_load_module(ulong base_addr)
>> req->name = lm->mod_namelist;
>> gdb_interface(req);
>> }
>> - mod_symtable_hash_remove_range(lm->mod_symtable,
>> lm->mod_symend);
>> + if (MODULE_MEMORY())
>> + if (lm->mod_load_symtable) {
>> +
>> mod_symtable_hash_remove_range(lm->mod_load_symtable, lm->mod_load_symend);
>> + for (j = MOD_TEXT; j <
>> MOD_MEM_NUM_TYPES; j++) {
>>
>
> Ditto.
>
>
>> + lm->load_symtable[j] =
>> NULL;
>> + lm->load_symend[j] = NULL;
>> + }
>> + } else { /* somehow this function runs for
>> unloaded modules */
>>
>
> Ditto.
>
> Thanks.
> Lianbo
>
>
>> + for (j = MOD_TEXT; j <
>> MOD_MEM_NUM_TYPES; j++) {
>> + if (!lm->symtable[j])
>> + continue;
>> +
>> mod_symtable_hash_remove_range(lm->symtable[j], lm->symend[j]);
>> + }
>> + }
>> + else
>> +
>> mod_symtable_hash_remove_range(lm->mod_symtable, lm->mod_symend);
>> +
>> if (lm->mod_load_symtable) {
>> free(lm->mod_load_symtable);
>> namespace_ctl(NAMESPACE_FREE,
>> @@ -13627,9 +13669,19 @@ delete_load_module(ulong base_addr)
>> }
>> if (lm->mod_flags & MOD_REMOTE)
>> unlink_module(lm);
>> - lm->mod_symtable = lm->mod_ext_symtable;
>> - lm->mod_symend = lm->mod_ext_symend;
>> - mod_symtable_hash_install_range(lm->mod_symtable,
>> lm->mod_symend);
>> + if (MODULE_MEMORY()) {
>> + lm->symtable = lm->ext_symtable;
>> + lm->symend = lm->ext_symend;
>> + for (j = MOD_TEXT; j < MOD_MEM_NUM_TYPES;
>> j++) {
>> + if (!lm->symtable[j])
>> + continue;
>> +
>> mod_symtable_hash_install_range(lm->symtable[j], lm->symend[j]);
>> + }
>> + } else {
>> + lm->mod_symtable = lm->mod_ext_symtable;
>> + lm->mod_symend = lm->mod_ext_symend;
>> +
>> mod_symtable_hash_install_range(lm->mod_symtable, lm->mod_symend);
>> + }
>> lm->mod_flags &=
>> ~(MOD_LOAD_SYMS|MOD_REMOTE|MOD_NOPATCH);
>> lm->mod_flags |= MOD_EXT_SYMS;
>> lm->mod_load_symtable = NULL;
>> --
>> 2.31.1
>>
>>
>>
--
Crash-utility mailing list
Crash-utility@xxxxxxxxxx
https://listman.redhat.com/mailman/listinfo/crash-utility
Contribution Guidelines: https://github.com/crash-utility/crash/wiki
