Re: [PATCH 2/2] mm/kmemleak: No need to check kmemleak_initialized in set_track_prepare()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 8/15/23 12:20 AM, Catalin Marinas wrote:
CAUTION: This email comes from a non Wind River email account!
Do not click links or open attachments unless you recognize the sender and know the content is safe.

On Fri, Aug 11, 2023 at 10:09:08AM +0200, Vlastimil Babka wrote:
On 8/11/23 04:03, wang xiaolei wrote:
On 8/10/23 9:16 PM, Vlastimil Babka wrote:
Looking closer, I think what you want could be achieved by kmemleak_init()
setting a variable that is checked in kmemleak_initialized() instead of the
kmemleak_initialized that's set too late.

I think this should work because:
- I assume kmemleak can't record anything before kmemleak_init()
- stack depot early init is requested one way or the other
- mm_core_init() calls stack_depot_early_init() before kmemleak_init()

But I also wonder how kmemleak can even reach set_track_prepare() before
kmemleak_init(), maybe that's the issue?
Before kmemleak_init, many places also need to allocate kmemleak_object,

and also need to save stack in advance, but kmemleak_object is allocated

in the form of an array, after kmemleak_init 'object_cache =
KMEM_CACHE(kmemleak_object, SLAB_NOLEAKTRACE);'
Hm I see, kmemleak has this static mempool so it really can record some
objects very early.
Indeed, otherwise we'd get a lot of false positives.

I think there is still some memory not recorded on the backtrace before

stack_depot_early_init(), does anyone have a better suggestion?
No we can't record the backtrace earlier. But I don't think it's a problem
in practice. AFAIU kmemleak needs to record these very early allocations so
if they point to further objects, those are not suspected as orphans. But
the early allocations themselves also are very unlikely to be leaks, so does
it really matter that we don't have a backtrace for their allocation?
Because the backtrace is the only thing that's missing - the object is
otherwise recorded even if set_track_prepare() returns 0.
It's not a functional problem, just a reporting one. There are
rare early leaks (usually false positives) so identifying them would
help. That said, I think set_track_prepare() is too conservative in
waiting for kmemleak_initialized to be set in kmemleak_late_init().
That's a late_initcall() meant for the scanning thread etc. not the core
kmemleak functionality (which is on from early boot).

We can instead use a different variable to check in set_track_prepare(),
e.g. object_cache. stack_depot_early_init() is called prior to
kmemleak_init(), so it should be fine.

If "kmemleak_initialized" is confusing, we could rename it to
"kmemleak_late_initialized" or "kmemleak_fully_initialized". I'm not too
fussed about this as long as we add some comment on why we check
object_cache instead of kmemleak_initialized.

Ok, I will send v2 version, use object_cache instead of kmemleak_initialized

to check in set_track_prepare, and update kmemleak_initialized to kmemleak_late_initialized

thanks

xiaolei


--
Catalin




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux