On Thu, 17 Jun 2021 at 16:55, Daniel Thompson <daniel.thomp...@linaro.org> wrote: > > On Tue, Mar 23, 2021 at 12:25:19PM +0530, Sumit Garg wrote: > > Currently the only user for debug heap is kdbnearsym() which can be > > modified to rather use statically allocated buffer for symbol name as > > per it's current usage. So do that and hence remove custom debug heap > > allocator. > > > > Note that this change puts a restriction on kdbnearsym() callers to > > carefully use shared namebuf such that a caller should consume the symbol > > returned immediately prior to another call to fetch a different symbol. > > > > This change has been tested using kgdbtest on arm64 which doesn't show > > any regressions. > > > > Suggested-by: Daniel Thompson <daniel.thomp...@linaro.org> > > Signed-off-by: Sumit Garg <sumit.g...@linaro.org> > > I'm afraid the passage of time (mostly due to my dropping the ball) > means this no longer applies cleanly to latest kernel and `git am > -3way` tells me that "sha1 information is lacking or useless". > Please can you rebase this on v5.13-rc4 and repost? >
Sure. > Also... > > > > diff --git a/kernel/debug/kdb/kdb_private.h b/kernel/debug/kdb/kdb_private.h > > index b857a84de3b5..ec91d7e02334 100644 > > diff --git a/kernel/debug/kdb/kdb_support.c b/kernel/debug/kdb/kdb_support.c > > index b59aad1f0b55..e131d74abb8d 100644 > > --- a/kernel/debug/kdb/kdb_support.c > > +++ b/kernel/debug/kdb/kdb_support.c > > @@ -57,35 +57,26 @@ int kdbgetsymval(const char *symname, kdb_symtab_t > > *symtab) > > } > > EXPORT_SYMBOL(kdbgetsymval); > > > > -static char *kdb_name_table[100]; /* arbitrary size */ > > - > > /* > > - * kdbnearsym - Return the name of the symbol with the nearest address > > - * less than 'addr'. > > + * kdbnearsym() - Return the name of the symbol with the nearest address > > + * less than @addr. > > + * @addr: Address to check for near symbol > > + * @symtab: Structure to receive results > > * > > - * Parameters: > > - * addr Address to check for symbol near > > - * symtab Structure to receive results > > - * Returns: > > - * 0 No sections contain this address, symtab zero filled > > - * 1 Address mapped to module/symbol/section, data in symtab > > - * Remarks: > > - * 2.6 kallsyms has a "feature" where it unpacks the name into a > > - * string. If that string is reused before the caller expects it > > - * then the caller sees its string change without warning. To > > - * avoid cluttering up the main kdb code with lots of kdb_strdup, > > - * tests and kfree calls, kdbnearsym maintains an LRU list of the > > - * last few unique strings. The list is sized large enough to > > - * hold active strings, no kdb caller of kdbnearsym makes more > > - * than ~20 later calls before using a saved value. > > + * Note here that only single statically allocated namebuf is used for > > every > > + * symbol, so the caller should consume it immediately prior to another > > call > > + * to fetch a different symbol. > > This still looks like it will confused experienced kernel devs who > aren't aware of kdb's calling context. Nor does it help future kdb > developers understand some of the subtlty of interactions here. > > Can you enlarge this to the following (editing anything below that you > don't want git to blame you for ;-) ): > > ~~~ > WARNING: This function may return a pointer to a single statically > allocated buffer (namebuf). kdb's unusual calling context (single > threaded, all other CPUs halted) provides us sufficient locking for > this to be safe. The only constraint imposed by the static buffer is > that the caller must consume any previous reply prior to another call > to lookup a new symbol. > > Note that, strictly speaking, some architectures may re-enter the kdb > trap if the system turns out to be very badly damaged and this breaks > the single-threaded assumption above. In these circumstances successful > continuation and exit from the inner trap is unlikely to work and any > user attempting this receives a prominent warning before being allowed > to progress. In these circumstances we remain memory safe because > namebuf[KSYM_NAME_LEN-1] will never change from '\0' although we do > tolerate the possibility of garbled symbol display from the outer kdb > trap. > ~~~ > Okay I will use this comment instead. -Sumit > Thanks > > > Daniel. _______________________________________________ Kgdb-bugreport mailing list Kgdb-bugreport@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kgdb-bugreport