On Mon, 5 Aug 2024 at 16:06, chenqiwu <qiwuchen55@xxxxxxxxx> wrote: > > On Mon, Aug 05, 2024 at 08:50:57AM +0200, Marco Elver wrote: > > > > The "allocated/freed" info is superfluous, as freed objects will have > > a free stack. > > > > Consider a slightly better script vs. just using grep. > Well, I think using grep is eaiser than a script to find leaks by a > large number of alloc tracks. Sure. But a slightly more complex script is a better trade-off vs. impacting _all_ KFENCE users world-wide with slightly less readable error reports. > > /sys/kernel/debug/kfence/objects is of secondary concern and was added > > primarily as a debugging aid for KFENCE developers. We never thought > > it could be used to look for leaks, but good you found another use for > > it. ;-) > > The priority is to keep regular error reports generated by KFENCE > > readable. Adding this "allocated/freed" info just makes the line > > longer and is not useful. > > > How about print meta->state directly to get the object state for its > alloc/free track? > - seq_con_printf(seq, "%s by task %d on cpu %d at %lu.%06lus:\n", > + seq_con_printf(seq, "%s by task %d on cpu %d at %lu.%06lus (%lu.%06lus ago) state %d:\n", > show_alloc ? "allocated" : "freed", track->pid, > - track->cpu, (unsigned long)ts_sec, rem_nsec / 1000); > + track->cpu, (unsigned long)ts_sec, rem_nsec / 1000, > + (unsigned long)interval_nsec, rem_interval_nsec / 1000, > + meta->state); > > I'm happy with the "(%lu.%06lus ago)" part alone. > If it's still a not good idea, I will follow your suggestion and resend > it as v2. No, that's just making it more ugly for no reason. It's replicating the state info (just like before) for alloc and free stacks and generally does not add anything useful. See, we are writing code that is deployed on millions of machines, and KFENCE error reports do appear in the wild occasionally. We have to optimize for the common case. Your change might be useful for you, which is a relatively unique usecase. The common use case of KFENCE is to detect memory-safety errors, and good error reports are a major feature of KFENCE. All information is already present in the reports (and /sys/kernel/debug/kfence/objects). I argue that you are able to write a slightly more complex script that simply looks for the free stack right after the allocation stack to determine if an object is live or freed. Maybe doing it in bash won't work so nicely, but a small Python script can easily do that job. Once you have that Python script you might even do further processing, sort things by age, size, etc. etc., and then print whole stack traces. Just grep can't do that. So if you want something useful, you'd have to give up on grep sooner or later.
