Messages in this thread | | | Date | Mon, 1 Dec 2008 08:49:38 +0200 | From | "Pekka Enberg" <> | Subject | Re: [PATCH 01/15] kmemleak: Add the base support |
| |
Hi!
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.
Also, I see some other BUG_ON() calls in the code which probably should be converted to WARN_ON() as well.
| |