Messages in this thread | | | Subject | Re: [PATCH 01/15] kmemleak: Add the base support | From | Catalin Marinas <> | Date | Mon, 01 Dec 2008 12:29:22 +0000 |
| |
On Mon, 2008-12-01 at 08:49 +0200, Pekka Enberg wrote: > On Mon, Dec 1, 2008 at 7:39 AM, Yasunori Goto <y-goto@jp.fujitsu.com> wrote: > >> +/* > >> + * Insert a pointer into the pointer hash table. > >> + */ > >> +static inline void create_object(unsigned long ptr, size_t size, int ref_count) > >> +{ > >> + unsigned long flags; > >> + struct memleak_object *object; > >> + struct prio_tree_node *node; > >> + struct stack_trace trace; > >> + > >> + object = kmem_cache_alloc(object_cache, GFP_ATOMIC); > >> + if (!object) > >> + panic("kmemleak: cannot allocate a memleak_object structure\n"); > > > > IIRC, GFP_ATOMIC allocation sometimes fails. (ex. when page cache occupies all > > area). It seems to be easy to panic. Is it intended? > > Yup, GFP_ATOMIC can fail as can any memory allocation on out-of-memory > conditions unless you specify GFP_NOFAIL which will either succeed or > lock up the box. I think you can just WARN_ON() here? However, it's > probably safer to pass gfp flags from the callers here; otherwise we > end up doing tons of GFP_ATOMIC allocations which is not healthy in > general.
Yes, good idea, GFP_ATOMIC isn't always needed.
> Also, I see some other BUG_ON() calls in the code which probably > should be converted to WARN_ON() as well.
This was also raised in the past by Ingo and it's on my to-do list to fix. Basically, if an allocation for internal kmemleak structures fails, kmemleak should print a warning and disable the tracing since further leak reports are irrelevant. Once I add the run-time kmemleak configuration/disabling, I'll change the BUG_ON and panic calls to something less severe.
-- Catalin
| |