From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752217AbeAWPl2 (ORCPT ); Tue, 23 Jan 2018 10:41:28 -0500 Received: from mail.kernel.org ([198.145.29.99]:45552 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751978AbeAWPlY (ORCPT ); Tue, 23 Jan 2018 10:41:24 -0500 DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 7A4642178E Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=goodmis.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=rostedt@goodmis.org Date: Tue, 23 Jan 2018 10:41:21 -0500 From: Steven Rostedt To: Sergey Senozhatsky Cc: Sergey Senozhatsky , Petr Mladek , Tejun Heo , akpm@linux-foundation.org, linux-mm@kvack.org, Cong Wang , Dave Hansen , Johannes Weiner , Mel Gorman , Michal Hocko , Vlastimil Babka , Peter Zijlstra , Linus Torvalds , Jan Kara , Mathieu Desnoyers , Tetsuo Handa , rostedt@rostedt.homelinux.com, Byungchul Park , Pavel Machek , linux-kernel@vger.kernel.org Subject: Re: [PATCH v5 0/2] printk: Console owner and waiter logic cleanup Message-ID: <20180123104121.2ef96d81@gandalf.local.home> In-Reply-To: <20180123152130.GB429@tigerII.localdomain> References: <20180117091208.ezvuhumnsarz5thh@pathway.suse.cz> <20180117151509.GT3460072@devbig577.frc2.facebook.com> <20180117121251.7283a56e@gandalf.local.home> <20180117134201.0a9cbbbf@gandalf.local.home> <20180119132052.02b89626@gandalf.local.home> <20180120071402.GB8371@jagdpanzerIV> <20180120104931.1942483e@gandalf.local.home> <20180121141521.GA429@tigerII.localdomain> <20180123064023.GA492@jagdpanzerIV> <20180123095652.5e14da85@gandalf.local.home> <20180123152130.GB429@tigerII.localdomain> X-Mailer: Claws Mail 3.16.0 (GTK+ 2.24.31; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, 24 Jan 2018 00:21:30 +0900 Sergey Senozhatsky wrote: > On (01/23/18 09:56), Steven Rostedt wrote: > [..] > > > Why do we even use irq_work for printk_safe? > > > > Why not? > > > > Really, I think you are trying to solve a symptom and not the problem. > > If we are having issues with irq_work, we are going to have issues with > > a work queue. It's just spreading out the problem instead of fixing it. > > I don't want to have heuristics in print_safe, I don't want to have a magic > number controlled by a user-space visible knob, I don't want to have the > first 3 lines of a lockdep splat. We can have more. But if printk is causing printks, that's a major bug. And work queues are not going to fix it, it will just spread out the pain. Have it be 100 printks, it needs to be fixed if it is happening. And having all printks just generate more printks is not helpful. Even if we slow them down. They will still never end. A printk causing a printk is a special case, and we need to just show enough to let the user know that its happening, and why printks are being throttled. Yes, we may lose data, but if every printk that goes out causes another printk, then there's going to be so much noise that we wont know what other things went wrong. Honestly, if someone showed me a report where the logs were filled with printks that caused printks, I'd stop right there and tell them that needs to be fixed before we do anything else. And if that recursion is happening because of another problem, I don't want to see the recursion printks. I want to see the printks that show what is causing the recursions. > The problem is - we flush printk_safe too soon and printing CPU ends up > in a lockup - it log_store()-s new messages while it's printing the pending No, the problem is that printks are causing more printks. Yes that will make flushing them soon more likely to lock up the system. But that's not the problem. The problem is printks causing printks. > ones. It's fine to do so when CPU is in preemptible context. Really, we > should not care in printk_safe as long as we don't lockup the kernel. The > misbehaving console must be fixed. If CPU is not in preemptible context then > we do lockup the kernel. Because we flush printk_safe regardless of the > current CPU context. If we will flush printk_safe via WQ then we automatically And if we can throttle recursive printks, then we should be able to stop that from happening. > add this "OK! The CPU is preemptible, we can log_store(), it's totally OK, we > will not lockup it up." thing. Yes, we fill up the logbuf with probably needed > and appreciated or unneeded messages. But we should not care in printk_safe. > We don't lockup the kernel... And the misbehaving console must be fixed. I agree. > > I disagree with "If we are having issues with irq_work, we are going to have > issues with a work queue". There is a tremendous difference between irq_work > on that CPU and queue_work_on(smp_proessor_id()). One does not care about CPU > context, the other one does. But switching to work queue does not address the underlining problem that printks are causing more printks. -- Steve