On Wed, Sep 06, 2023 at 12:23:33PM -0500, Scott Cheloha wrote: > On Wed, Sep 06, 2023 at 01:04:19PM +0100, Martin Pieuchot wrote: > > Debugging OOM is hard. UVM uses per-CPU counters and sadly > > counters_read(9) needs to allocate memory. This is not acceptable in > > ddb(4). As a result I cannot see the content of UVM counters in OOM > > situations. > > > > Diff below introduces a *_static() variant of counters_read(9) that > > takes a secondary buffer to avoid calling malloc(9). Is it fine? Do > > you have a better idea? Should we make it the default or using the > > stack might be a problem? > > Instead of adding a second interface I think we could get away with > just extending counters_read(9) to take a scratch buffer as an optional > fourth parameter: > > void > counters_read(struct cpumem *cm, uint64_t *output, unsigned int n, > uint64_t *scratch); > > "scratch"? "temp"? "tmp"?
Just "buf". > > Anyway, a NULL scratch means "allocate this for me", otherwise you're > saying you've brought your own. Obviously the contents of scratch are > undefined upon return. That's precisely what I did some a year ago to get rid of counter_read()'s sleep point in order to replace an rwlock with a mutex in network code. It was OK, but I think the approach that required it stalled... not sure. The diff mechanically added a NULL argument to all callers so that future diffs would actually change semantics along with the code that needs it. Here are the manual bits I wrote, in case it helps. diff --git a/share/man/man9/counters_alloc.9 b/share/man/man9/counters_alloc.9 index 1de8bffc639..b8e1438d5fd 100644 --- a/share/man/man9/counters_alloc.9 +++ b/share/man/man9/counters_alloc.9 @@ -64,6 +64,7 @@ .Fa "struct cpumem *cm" .Fa "uint64_t *counters" .Fa "unsigned int ncounters" +.Fa "uint64_t *buf" .Fc .Ft void .Fn counters_zero "struct cpumem *cm" "unsigned int ncounters" @@ -191,6 +192,12 @@ The sum of the counters is written to the array. The number of counters is specified with .Fa ncounters . +.Fa buf +may point to a buffer used to temporarily hold +.Fa counters . +If +.Dv NULL , +one will be dynamically allocated and freed. .Pp .Fn counters_zero iterates over each CPU's set of counters referenced by
