On Wed 2017-03-29 09:31:47, Michal Hocko wrote: > On Tue 28-03-17 18:00:16, Petr Mladek wrote: > > On Tue 2017-03-28 16:22:27, Michal Hocko wrote: > > > On Sat 25-03-17 09:04:42, Sergey Senozhatsky wrote: > > > > On (03/21/17 13:44), Sergey Senozhatsky wrote: > > > > [..] > > > > > so we probably can > > > > > > > > > > > > > > > 1) move pr_info() out of zone->lock in __offline_isolated_pages(). > > > > > meh... > > > > > > > > > > > > > > > 2) switch to printk_deferred() in __offline_isolated_pages(). > > > > > meh.. there might a bunch of other printks done from under > > > > > zone->lock. > > > > > > > > > > > > > > > 3) move add_timer() out of sclp_con_lock console in > > > > > sclp_console_write(). > > > > > well, there can be other consoles that do something similar. > > > > > > > > > > > > > > > 4) ... something smart. > > > > > > > > > > Regarding the timer code. The problem seems to be with static > > timers. They call debug_object_init() when the timer is used > > for the first time. See the special handling of not-found > > objects in debug_object_activate(). > > Thanks for the clarification! > > > Now, __debug_object_init() calls fill_pool() that allocates > > the memory. A solution would be to either use static > > struct kmem_cache > > I am not sure what do you mean by that.
The problem is when the timer is defined using DEFINE_TIMER() and initialized using __TIMER_INITIALIZER(). I had an unclear idea about extending the macro to define also the needed structure that might later be used by debug_object_init() instead of taking it from the pool. I am sorry for the confusion with struct kmem_cache. It is the type of the pointer that is passed when allocating the memory in fill_pool(). It seems we actually need struct debug_obj. But it might be even more complicated. > > for statically defined timers and avoid the allocation. Or we should > > call fill_pool() asynchronously from a safe context. > > I think we should avoid the allocation completely. It is GFP_ATOMIC and > so likely to fail under heavy memory pressure. Async fill will make it > slightly more complicated but still unreliable. > > > What do you think? > > Why cannot we simply embed this debugging data into the timer itself? > It will make the structure larger (I didn't check how much) but this is > an opt in feature so it should be acceptable. This sounds reasonable to me. > A subtle dependecny on the allocator is really bad and > we should get rid of it. Yup. Best Regards, Petr
