From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1750830AbeFAJJu (ORCPT ); Fri, 1 Jun 2018 05:09:50 -0400 Received: from mx2.suse.de ([195.135.220.15]:54846 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750718AbeFAJJo (ORCPT ); Fri, 1 Jun 2018 05:09:44 -0400 Date: Fri, 1 Jun 2018 11:09:42 +0200 From: Petr Mladek To: Sergey Senozhatsky Cc: Maninder Singh , sergey.senozhatsky@gmail.com, rostedt@goodmis.org, linux-kernel@vger.kernel.org, a.sahrawat@samsung.com, pankaj.m@samsung.com, v.narang@samsung.com Subject: Re: [PATCH 2/2] printk: make sure to print log on console. Message-ID: <20180601090942.ek3j4bpbhschljrw@pathway.suse.cz> References: <20180531102246epcas5p2f1cbc6ff217172e12e2f78bb88eb4a7e~zs5h59tMh2250222502epcas5p2S@epcas5p2.samsung.com> <20180531105215.GF477@jagdpanzerIV> <20180531122112.bfeqtmwpl2qc67a5@pathway.suse.cz> <20180601044050.GA5687@jagdpanzerIV> <20180601085356.kncuat7epkbtythv@pathway.suse.cz> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20180601085356.kncuat7epkbtythv@pathway.suse.cz> User-Agent: NeoMutt/20170421 (1.8.2) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri 2018-06-01 10:53:56, Petr Mladek wrote: > On Fri 2018-06-01 13:40:50, Sergey Senozhatsky wrote: > > On (05/31/18 14:21), Petr Mladek wrote: > > > > > > > > Upstream printk has no printing kthread. And we also run > > > > printk()->console_unlock() with disabled preemption. > > > > > > Yes, the comment was wrong > > > > Yes, that was the only thing I meant. > > I really didn't have any time to look at the patch yesterday, just > > commented on the most obvious thing. > > Fair enough. > > > > but the problem is real. > > > > Yep, could be. But not exactly the way it is described in the commit > > messages and the patch does not fully address the problem. > > > > The patch assumes that all those events happen sequentially. While > > in reality they can happen in parallel on different CPUs. > > > > Example: > > > > CPU0 CPU1 > > > > set console verbose > > > > dump_backtrace() > > { > > // for (;;) print frames > > printk("%pS\n", frame0); > > printk("%pS\n", frame1); > > printk("%pS\n", frame2); > > printk("%pS\n", frame3); > > ... console_loglevel = CONSOLE_LOGLEVEL_SILENT; > > printk("%pS\n", frame12); > > printk("%pS\n", frame13); > > } > > > > Part of backtrace or the entire backtrace will be missed, because > > we read the global console_loglevel. The problem is still there. > > [...] > > > So I'd say that most likely the following scenarios can suffer: > > > > - NMI comes in, sets loglevel to X, printk-s some data, restores the > > loglevel back to Y > > - IRQ comes in [like sysrq, etc] comes in and does the same thing > > - software exception comes in and does the same thing [e.g. bust_spinlocks() > > at arch/s390/mm/fault.c] I forgot to say that it was a great point and analyze. > My view is: > > The race with another printk() (console_lock owner) is much more > likely than a race between two CPUs manipulating console_loglevel. > > The proposed patch seems to be in the right direction. It is supposed > to fix the most likely scenario. We could block it and request full > solution but I wonder if it is worth it. > > I am personally fine with this partial solution for now. We could > always make it better if people meet the other scenarios. I am still fine with the partial solution. Well, I will think more about it before approving any patch. Best Regards, Petr