From 05a3a328fcd8920e49926b6d1c9c81ce0b6acbca Mon Sep 17 00:00:00 2001 From: Kazuhito Hagio Date: Thu, 9 Sep 2021 15:23:27 +0900 Subject: [PATCH] Remove text value cache code The text value cache was implemented for analysis of remote dumpfiles using the deprecated "crash daemon" running on the remote host. On updating GDB to 10.2, a regression occurred when we tried to fix a "help -x" command problem, and there was no performance degradation even without the text cache, so let's drop this functionality. Signed-off-by: Kazuhito Hagio --- alpha.c | 8 +- cmdline.c | 1 - defs.h | 4 - gdb_interface.c | 14 ---- help.c | 8 +- kernel.c | 1 - symbols.c | 196 ------------------------------------------------ x86.c | 7 -- 8 files changed, 3 insertions(+), 236 deletions(-) diff --git a/alpha.c b/alpha.c index fa2d5f5..7349712 100644 --- a/alpha.c +++ b/alpha.c @@ -776,12 +776,8 @@ alpha_frame_offset(struct gnu_request *req, ulong alt_pc) * Don't go any farther than "stq ra,0(sp)" (0xb75e0000) */ while (ival != 0xb75e0000) { - if (!text_value_cache((ulong)ip, 0, &ival)) { - readmem((ulong)ip, KVADDR, &ival, - sizeof(uint), "uncached text value", - FAULT_ON_ERROR); - text_value_cache((ulong)ip, ival, NULL); - } + readmem((ulong)ip, KVADDR, &ival, sizeof(uint), + "text value", FAULT_ON_ERROR); if ((ival & 0xffe01fff) == 0x43c0153e) { value = (ival & 0x1fe000) >> 13; diff --git a/cmdline.c b/cmdline.c index 796f7c5..ded6551 100644 --- a/cmdline.c +++ b/cmdline.c @@ -1235,7 +1235,6 @@ restore_sanity(void) if (CRASHDEBUG(5)) { dump_filesys_table(0); dump_vma_cache(0); - dump_text_value_cache(0); } if (REMOTE()) diff --git a/defs.h b/defs.h index b34c60e..cbd45e5 100644 --- a/defs.h +++ b/defs.h @@ -5327,10 +5327,6 @@ char *load_module_filter(char *, int); long datatype_info(char *, char *, struct datatype_member *); int get_symbol_type(char *, char *, struct gnu_request *); int get_symbol_length(char *); -int text_value_cache(ulong, uint32_t, uint32_t *); -int text_value_cache_byte(ulong, unsigned char *); -void dump_text_value_cache(int); -void clear_text_value_cache(void); void dump_numargs_cache(void); int patch_kernel_symbol(struct gnu_request *); struct syment *generic_machdep_value_to_symbol(ulong, ulong *); diff --git a/gdb_interface.c b/gdb_interface.c index ff45cdc..3a7fcc9 100644 --- a/gdb_interface.c +++ b/gdb_interface.c @@ -828,7 +828,6 @@ int gdb_readmem_callback(ulong addr, void *buf, int len, int write) { char locbuf[SIZEOF_32BIT], *p1; - uint32_t *p2; int memtype; ulong readflags; @@ -885,19 +884,12 @@ gdb_readmem_callback(ulong addr, void *buf, int len, int write) } p1 = (char *)buf; - if ((memtype == KVADDR) && - text_value_cache_byte(addr, (unsigned char *)p1)) - return TRUE; if (!readmem(addr, memtype, locbuf, SIZEOF_32BIT, "gdb_readmem_callback", readflags)) return FALSE; *p1 = locbuf[0]; - if (memtype == KVADDR) { - p2 = (uint32_t *)locbuf; - text_value_cache(addr, *p2, 0); - } return TRUE; case SIZEOF_32BIT: @@ -907,16 +899,10 @@ gdb_readmem_callback(ulong addr, void *buf, int len, int write) return TRUE; } - if ((memtype == KVADDR) && text_value_cache(addr, 0, buf)) - return TRUE; - if (!readmem(addr, memtype, buf, SIZEOF_32BIT, "gdb_readmem callback", readflags)) return FALSE; - if (memtype == KVADDR) - text_value_cache(addr, - (uint32_t)*((uint32_t *)buf), NULL); return TRUE; } diff --git a/help.c b/help.c index e67fde0..c19b69b 100644 --- a/help.c +++ b/help.c @@ -535,7 +535,7 @@ cmd_help(void) oflag = 0; while ((c = getopt(argcnt, args, - "efNDdmM:ngcaBbHhkKsvVoptTzLxOr")) != EOF) { + "efNDdmM:ngcaBbHhkKsvVoptTzLOr")) != EOF) { switch(c) { case 'e': @@ -551,10 +551,6 @@ cmd_help(void) dumpfile_memory(DUMPFILE_MEM_DUMP); return; - case 'x': - dump_text_value_cache(VERBOSE); - return; - case 'd': dump_dev_table(); return; @@ -666,7 +662,6 @@ cmd_help(void) fprintf(fp, " -T - task_table plus context_array\n"); fprintf(fp, " -v - vm_table\n"); fprintf(fp, " -V - vm_table (verbose)\n"); - fprintf(fp, " -x - text cache\n"); fprintf(fp, " -z - help options\n"); return; @@ -1026,7 +1021,6 @@ char *help_help[] = { " -T - task_table plus context_array", " -v - vm_table", " -V - vm_table (verbose)", -" -x - text cache", " -z - help options", NULL }; diff --git a/kernel.c b/kernel.c index a8be7bb..3ead4bb 100644 --- a/kernel.c +++ b/kernel.c @@ -4661,7 +4661,6 @@ reinit_modules(void) st->ext_module_symtable = NULL; st->load_modules = NULL; kt->mods_installed = 0; - clear_text_value_cache(); module_init(); } diff --git a/symbols.c b/symbols.c index 5d3c53a..69dccdb 100644 --- a/symbols.c +++ b/symbols.c @@ -12824,202 +12824,6 @@ gnu_qsort(bfd *bfd, qsort(minisyms, symcount, size, numeric_forward); } -/* - * Keep a stash of commonly-accessed text locations checked by the - * back_trace code. The saved values unsigned 32-bit values. - * The same routine is used to store and query, based upon whether - * the passed-in value and valptr args are non-zero. - */ -#define TEXT_CACHE (50) -#define MAX_TEXT_CACHE (TEXT_CACHE*4) - -struct text_cache_entry { - ulong vaddr; - uint32_t value; -}; - -static struct text_cache { - int index; - int entries; - ulong hits; - ulong refs; - struct text_cache_entry *cache; -} text_cache = { 0 }; - -/* - * Cache the contents of 32-bit text addresses. If "value" is set, the purpose - * is to cache it. If "valptr" is set, a query is being made for the text - * address. - */ -int -text_value_cache(ulong vaddr, uint32_t value, uint32_t *valptr) -{ - int i; - struct text_cache *tc; - - if (!is_kernel_text(vaddr)) - return FALSE; - - tc = &text_cache; - - if (!tc->cache) { - if (!(tc->cache = (struct text_cache_entry *) - malloc(sizeof(struct text_cache_entry) * TEXT_CACHE))) - return FALSE; - BZERO(tc->cache, sizeof(struct text_cache_entry) * TEXT_CACHE); - tc->index = 0; - tc->entries = TEXT_CACHE; - } - - if (value) { - for (i = 0; i < tc->entries; i++) { - if (tc->cache[i].vaddr == vaddr) - return TRUE; - } - - i = tc->index; - tc->cache[i].vaddr = vaddr; - tc->cache[i].value = value; - tc->index++; - if (tc->index == MAX_TEXT_CACHE) { - tc->index = 0; - } else if (tc->index == tc->entries) { - struct text_cache_entry *old_cache; - - old_cache = tc->cache; - if ((tc->cache = (struct text_cache_entry *) - realloc(old_cache, sizeof(struct text_cache_entry) * - (TEXT_CACHE+tc->entries)))) { - BZERO(&tc->cache[tc->index], - sizeof(struct text_cache_entry) * - TEXT_CACHE); - tc->entries += TEXT_CACHE; - } else { - tc->cache = old_cache; - tc->index = 0; - } - } - return TRUE; - } - - if (valptr) { - tc->refs++; - - for (i = 0; i < tc->entries; i++) { - if (!tc->cache[i].vaddr) - return FALSE; - - if (tc->cache[i].vaddr == vaddr) { - *valptr = tc->cache[i].value; - tc->hits++; - return TRUE; - } - } - } - - return FALSE; -} - -/* - * The gdb disassembler reads text memory byte-by-byte, so this routine - * acts as a front-end to the 32-bit (4-byte) text storage. - */ - -int -text_value_cache_byte(ulong vaddr, unsigned char *valptr) -{ - int i; - int shift; - struct text_cache *tc; - ulong valtmp; - - if (!is_kernel_text(vaddr)) - return FALSE; - - tc = &text_cache; - - tc->refs++; - - for (i = 0; i < tc->entries; i++) { - if (!tc->cache[i].vaddr) - return FALSE; - - if ((vaddr >= tc->cache[i].vaddr) && - (vaddr < (tc->cache[i].vaddr+SIZEOF_32BIT))) { - valtmp = tc->cache[i].value; - shift = (vaddr - tc->cache[i].vaddr) * 8; - valtmp >>= shift; - *valptr = valtmp & 0xff; - tc->hits++; - return TRUE; - } - } - return FALSE; -} - -void -dump_text_value_cache(int verbose) -{ - int i; - struct syment *sp; - ulong offset; - struct text_cache *tc; - - tc = &text_cache; - - if (!verbose) { - if (!tc->refs || !tc->cache) - return; - - fprintf(stderr, " text hit rate: %2ld%% (%ld of %ld)\n", - (tc->hits * 100)/tc->refs, - (ulong)tc->hits, (ulong)tc->refs); - return; - } - - for (i = 0; tc->cache && (i < tc->entries); i++) { - if (!tc->cache[i].vaddr) - break; - fprintf(fp, "[%2d]: %lx %08x ", i, tc->cache[i].vaddr, - tc->cache[i].value); - if ((sp = value_search(tc->cache[i].vaddr, &offset))) { - fprintf(fp, "(%s+", sp->name); - switch (pc->output_radix) - { - case 10: - fprintf(fp, "%ld)", offset); - break; - case 16: - fprintf(fp, "%lx)", offset); - break; - } - } - fprintf(fp, "\n"); - } - - fprintf(fp, - "text_cache entries: %d index: %d hit rate: %ld%% (%ld of %ld)\n", - tc->entries, tc->index, - (tc->hits * 100)/(tc->refs ? tc->refs : 1), - tc->hits, tc->refs); - -} - -void -clear_text_value_cache(void) -{ - int i; - struct text_cache *tc; - - tc = &text_cache; - tc->index = 0; - - for (i = 0; tc->cache && (i < tc->entries); i++) { - tc->cache[i].vaddr = 0; - tc->cache[i].value = 0; - } -} - /* * If a System.map file or a debug kernel was specified, the name hash * has been filled -- so sync up gdb's notion of symbol values with diff --git a/x86.c b/x86.c index a174bbc..b36f44c 100644 --- a/x86.c +++ b/x86.c @@ -474,17 +474,10 @@ db_get_value(addr, size, is_signed, bt) else GET_STACK_DATA(addr, data, size); } else { - if ((size == sizeof(int)) && - text_value_cache(addr, 0, (uint32_t *)&value)) - return value; - if (!readmem(addr, KVADDR, &value, size, "db_get_value", RETURN_ON_ERROR)) error(FATAL, "db_get_value: read error: address: %lx\n", addr); - - if (size == sizeof(int)) - text_value_cache(addr, value, NULL); } #endif