From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932439AbcASQRD (ORCPT ); Tue, 19 Jan 2016 11:17:03 -0500 Received: from mx2.suse.de ([195.135.220.15]:44084 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932168AbcASQRA (ORCPT ); Tue, 19 Jan 2016 11:17:00 -0500 Date: Tue, 19 Jan 2016 17:16:57 +0100 From: Petr Mladek To: Sergey Senozhatsky Cc: Sergey Senozhatsky , Andrew Morton , Tejun Heo , Jan Kara , Kyle McMartin , Dave Jones , Calvin Owens , linux-kernel@vger.kernel.org Subject: Re: [RFC][PATCH -next 1/2] printk: move can_use_console out of console_trylock_for_printk Message-ID: <20160119161656.GC2148@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> <20160119133136.GA2148@dhcp128.suse.cz> <20160119150040.GA558@swordfish> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20160119150040.GA558@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 Wed 2016-01-20 00:00:40, Sergey Senozhatsky wrote: > > But do we really need to repeat the check in every cycle? > > well, on every iteration in the best case we check cpu_online() > only. which is what we would have done anyway in vprintk_emit(), > so no additional checks added. at the same time call_console_drivers > does not do '!cpu_online && !CON_ANYTIME' for each console now, so > in some sense there are less checks now (this is far even from a > micro-optimization, just noted). Hmm, we need to keep the check in call_console_drivers(). It iterates over all registered consoles. Only some of them could habe CON_ANYTIME flag. We need to skip the others when the CPU is not online. > console_unlock() /* w/o can_use_console() in logbuf_lock section */ > .... > again: > for (;;) { > raw_spin_lock_irqsave logbuf_lock > msg_print_text > raw_spin_unlock logbuf_lock > call_console_drivers > local_irq_restore > } > > up_console_sem > > raw_spin_lock logbuf_lock > retry = console_seq != log_next_seq > raw_spin_unlock_irqrestore logbuf_lock > > if retry && console_trylock() > goto again; > > > when we up_console_sem(), consoles may appear and disappear, since we > don't keep the semaphore. Suppose that we have OFFLINE cpu and we had a > CON_ANYTIME console, but in between up_console_sem--console_trylock > that single CON_ANYTIME console was removed. So now we have !cpu_online > and !CON_ANYTIME. Ah, I have missed that the console_sem is released/taken before goto again. Hmm, your proposed solution adds even more twists into the code. I think how to make it easier. I think that we could move the again: target and call console_cont_flush() when there is some new content. It is a corner case, anyway. Then we could do: do_cond_resched = console_may_schedule; console_may_schedule = 0; +again: + if (!can_use_console()) { + console_locked = 0; + up_console_sem(); + return; } /* flush buffered message fragment immediately to console */ console_cont_flush(text, sizeof(text)); -again: for (;;) { struct printk_log *msg; Then we remove this check from console_trylock_for_printk() and eventually remove this function altogether. It means that the code will be easier and protected against the theoretical race. How does that sound, please? Best Regards, Petr