linux-kernel - Re: lockdep warning: console vs. mem hotplug

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 30 Mar 2017 11:45:00 +0200
From:   Petr Mladek <pmladek@...e.com>
To:     Michal Hocko <mhocko@...nel.org>
Cc:     Sergey Senozhatsky <sergey.senozhatsky@...il.com>,
        Sebastian Ott <sebott@...ux.vnet.ibm.com>,
        Steven Rostedt <rostedt@...dmis.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Peter Zijlstra <peterz@...radead.org>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Heiko Carstens <heiko.carstens@...ibm.com>,
        Martin Schwidefsky <schwidefsky@...ibm.com>,
        linux-kernel@...r.kernel.org,
        Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>
Subject: Re: lockdep warning: console vs. mem hotplug

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