Messages in this thread | | | Date | Mon, 01 Dec 2008 17:11:59 +0900 | From | Yasunori Goto <> | 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.
I agree. It is reasonable to pass gfp flag from the caller.
Thanks.
> > Also, I see some other BUG_ON() calls in the code which probably > should be converted to WARN_ON() as well.
-- Yasunori Goto
| |