From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755356AbdKBMze (ORCPT ); Thu, 2 Nov 2017 08:55:34 -0400 Received: from mx2.suse.de ([195.135.220.15]:50496 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754987AbdKBMzb (ORCPT ); Thu, 2 Nov 2017 08:55:31 -0400 Date: Thu, 2 Nov 2017 13:55:28 +0100 From: Michal Hocko To: Steven Rostedt Cc: Tetsuo Handa , akpm@linux-foundation.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org, Cong Wang , Dave Hansen , Johannes Weiner , Mel Gorman , Petr Mladek , Sergey Senozhatsky , Vlastimil Babka , "yuwang.yuwang" Subject: Re: [PATCH] mm: don't warn about allocations which stall for too long Message-ID: <20171102125528.4upg5eaw7cgxmak6@dhcp22.suse.cz> References: <1509017339-4802-1-git-send-email-penguin-kernel@I-love.SAKURA.ne.jp> <20171031153225.218234b4@gandalf.local.home> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20171031153225.218234b4@gandalf.local.home> User-Agent: NeoMutt/20170609 (1.8.3) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue 31-10-17 15:32:25, Steven Rostedt wrote: > > Thank you for the perfect timing. You posted this the day after I > proposed a new solution at Kernel Summit in Prague for the printk lock > loop that you experienced here. > > I attached the pdf that I used for that discussion (ignore the last > slide, it was left over and I never went there). > > My proposal is to do something like this with printk: > > Three types of printk usages: > > 1) Active printer (actively writing to the console). > 2) Waiter (active printer, first user) > 3) Sees active printer and a waiter, and just adds to the log buffer > and leaves. > > (new globals) > static DEFINE_SPIN_LOCK(console_owner_lock); > static struct task_struct console_owner; > static bool waiter; > > console_unlock() { > > [ Assumes this part can not preempt ] > > spin_lock(console_owner_lock); > console_owner = current; > spin_unlock(console_owner_lock); > > for each message > write message out to console > > if (READ_ONCE(waiter)) > break; > > spin_lock(console_owner_lock); > console_owner = NULL; > spin_unlock(console_owner_lock); > > [ preemption possible ] > > [ Needs to make sure waiter gets semaphore ] > > up(console_sem); > } > > > Then printk can have something like: > > > if (console_trylock()) > console_unlock(); > else { > struct task_struct *owner = NULL; > > spin_lock(console_owner_lock); > if (waiter) > goto out; > WRITE_ONCE(waiter, true); > owner = READ_ONCE(console_owner); > out: > spin_unlock(console_owner_lock); > if (owner) { > while (!console_trylock()) > cpu_relax(); > spin_lock(console_owner_lock); > waiter = false; > spin_unlock(console_owner_lock); > } > } > > This way, only one CPU spins waiting to print, and only if the > console_lock owner is actively printing. If the console_lock owner > notices someone is waiting to print, it stops printing as a waiter will > always continue the prints. This will balance out the printks among all > the CPUs that are doing them and no one CPU will get stuck doing all > the printks. > > This would solve your issue because the next warn_alloc() caller would > become the waiter, and take over the next message in the queue. This > would spread out the load of who does the actual printing, and not have > one printer be stuck doing the work. That also means that we would shift the overhead only to the first waiter AFAIU. What if we have floods of warn_alloc from all CPUs? Something that Tetsuo's test case simulates. As Petr pointed out earlier in the thread, I do not think this is going to help cosiderably and offloading to a kernel thread sounds like a more viable option. It sounds really wrong to have printk basically indeterministic wrt. call duration depending on who happens to do the actual work. Either we make the call sync or completely offloaded to a dedicated kernel thread and make sure that the buffer gets flushed unconditionally on panic. I haven't been following all the printk discussion recently so maybe this has been discussed and deemed not viable for implementation details but in principle this should work. -- Michal Hocko SUSE Labs