From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754594AbcASNbw (ORCPT ); Tue, 19 Jan 2016 08:31:52 -0500 Received: from mx2.suse.de ([195.135.220.15]:51015 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754150AbcASNbo (ORCPT ); Tue, 19 Jan 2016 08:31:44 -0500 Date: Tue, 19 Jan 2016 14:31:36 +0100 From: Petr Mladek To: Sergey Senozhatsky Cc: Andrew Morton , Tejun Heo , Jan Kara , Kyle McMartin , Dave Jones , Calvin Owens , linux-kernel@vger.kernel.org, Sergey Senozhatsky Subject: Re: [RFC][PATCH -next 1/2] printk: move can_use_console out of console_trylock_for_printk Message-ID: <20160119133136.GA2148@dhcp128.suse.cz> References: <1452747443-9927-1-git-send-email-sergey.senozhatsky@gmail.com> <1452747443-9927-2-git-send-email-sergey.senozhatsky@gmail.com> <20160118154228.GY3178@pathway.suse.cz> <20160119004236.GA4963@swordfish> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20160119004236.GA4963@swordfish> 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 On Tue 2016-01-19 09:42:36, Sergey Senozhatsky wrote: > Hello, > thanks for review. > > On (01/18/16 16:42), Petr Mladek wrote: > > On Thu 2016-01-14 13:57:22, Sergey Senozhatsky wrote: > > > vprintk_emit() disables preemption around console_trylock_for_printk() > > > and console_unlock() calls for a strong reason -- can_use_console() > > > check. The thing is that vprintl_emit() can be called on a CPU that > > > is not fully brought up yet (!cpu_online()), which potentially can > > > cause problems if console driver accesses per-cpu data. A console > > > driver can explicitly state that it's safe to call it from !online > > > cpu by setting CON_ANYTIME bit in console ->flags. That's why for > > > !cpu_online() can_use_console() iterates all the console to find out > > > if there is a CON_ANYTIME console, otherwise console_unlock() must be > > > avoided. > > > > > > call_console_drivers(), called from console_cont_flush() and > > > console_unlock(), does the same test during for_each_console() loop. > > > However, we can have the following corner case. Assume that we have 2 > > > cpus -- CPU0 is online, CPU1 is !online; and no CON_ANYTIME consoles > > > available. > > > > > > CPU0 online CPU1 !online > > > console_trylock() > > > ... > > > console_unlock() > > > > Please, where this console_unlock() comes from? > > from UP* or DOWN* (_PREPARE f.e.) notifiers on this CPU, for example, we don't > know what's going on there. what prevents it from calling console_trylock(), > grabbing the console_sem and eventually doing console_unlock()? there is > a can_use_console() check, but it handles only one case -- printk(). So, is it a theoretical problem or do you know about any particular path where this happens? Well, it might make sense to get rid of console_trylock_for_printk() and do the can_use_console() check at the beginning of unlock_console(). I mean to do - if (console_trylock_for_printk()) + if (console_trylock()) unlock_console(); But do we really need to repeat the check in every cycle? can_use_console() checks available consoles and if the CPU is online. Consoles could not get added or removed when we own console_sem. It seems that CPUs get disabled in a process context. Therefore it seems that it might happen only when unlock_console() gets rescheduled. I guess that it could not get scheduled back on an offline CPU. So, it seems that it is enough to check can_use_console() only once at the beginning. Or did I miss anything? Best Regards, Petr