Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754105AbdC1QAc (ORCPT ); Tue, 28 Mar 2017 12:00:32 -0400 Received: from mx2.suse.de ([195.135.220.15]:49698 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753590AbdC1QAa (ORCPT ); Tue, 28 Mar 2017 12:00:30 -0400 Date: Tue, 28 Mar 2017 18:00:16 +0200 From: Petr Mladek To: Michal Hocko Cc: Sergey Senozhatsky , Sebastian Ott , Steven Rostedt , Andrew Morton , Thomas Gleixner , Peter Zijlstra , Linus Torvalds , Heiko Carstens , Martin Schwidefsky , linux-kernel@vger.kernel.org, Sergey Senozhatsky Subject: Re: lockdep warning: console vs. mem hotplug Message-ID: <20170328160016.GK2846@pathway.suse.cz> References: <20170321044421.GB448@jagdpanzerIV.localdomain> <20170325000442.GD6356@tigerII.localdomain> <20170328142226.GK18241@dhcp22.suse.cz> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20170328142226.GK18241@dhcp22.suse.cz> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2791 Lines: 78 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. > > > > > > Sebastian, does this change make lockdep happy? > > > > it removes console drivers from the __offline_isolated_pages(). not the > > best solution I can think of, but the simplest one. > > > > --- > > > > mm/page_alloc.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/mm/page_alloc.c b/mm/page_alloc.c > > index f749b7ff7c50..eb61e6ab5f4f 100644 > > --- a/mm/page_alloc.c > > +++ b/mm/page_alloc.c > > @@ -7705,7 +7705,7 @@ __offline_isolated_pages(unsigned long start_pfn, unsigned long end_pfn) > > BUG_ON(!PageBuddy(page)); > > order = page_order(page); > > #ifdef CONFIG_DEBUG_VM > > - pr_info("remove from free list %lx %d %lx\n", > > + printk_deferred(KERN_INFO "remove from free list %lx %d %lx\n", > > pfn, 1 << order, end_pfn); > > #endif > > list_del(&page->lru); > > I believe this is not a proper fix. Although this code is ugly and maybe > it doesn't really need zone->lock because that should be the page > allocator internal thing the problem is that printk shouldn't impose > such a subtle dependency on locks. Why does the timer needs to allocate > at all? printk/console use timers to postpone flushing of buffers. There are often more consequent printks. The code wants to wait a bit and flush them together eventually. At the same time, it wants to set a deadline for the flushing. It makes sure that they will get flushed in a reasonable time even when the buffer is not full. It is questionable but it makes some sense. In each case, the timer code is used also by scheduler and we probably need to use scheduler from printk. 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(). Now, __debug_object_init() calls fill_pool() that allocates the memory. A solution would be to either use static struct kmem_cache for statically defined timers and avoid the allocation. Or we should call fill_pool() asynchronously from a safe context. What do you think? Best Regards, Petr