From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S934682AbcATDtu (ORCPT ); Tue, 19 Jan 2016 22:49:50 -0500 Received: from mail-pf0-f176.google.com ([209.85.192.176]:32979 "EHLO mail-pf0-f176.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933097AbcATDto (ORCPT ); Tue, 19 Jan 2016 22:49:44 -0500 Date: Wed, 20 Jan 2016 12:50:56 +0900 From: Sergey Senozhatsky To: Petr Mladek Cc: Sergey Senozhatsky , Sergey Senozhatsky , Andrew Morton , Tejun Heo , Jan Kara , Kyle McMartin , Dave Jones , Calvin Owens , linux-kernel@vger.kernel.org Subject: Re: [RFC][PATCH -next 2/2] printk: set may_schedule for some of console_trylock callers Message-ID: <20160120035056.GA506@swordfish> References: <1452747443-9927-1-git-send-email-sergey.senozhatsky@gmail.com> <1452747443-9927-3-git-send-email-sergey.senozhatsky@gmail.com> <20160117141137.GA631@swordfish> <20160117142332.GA543@swordfish> <20160118161744.GZ3178@pathway.suse.cz> <20160119011510.GB4963@swordfish> <20160119151801.GB2148@dhcp128.suse.cz> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20160119151801.GB2148@dhcp128.suse.cz> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hello, On (01/19/16 16:18), Petr Mladek wrote: [..] > > > > console_locked = 1; > > > > - console_may_schedule = 0; > > > > + console_may_schedule = !(oops_in_progress || > > > > + irqs_disabled() || > > IMHO, we should also check if we are in irq context or not. > > > > > + in_atomic() || > > Hmm, it seems that in_atomic() is not safe enough. There is the > following comment: > > /* > * Are we running in atomic context? WARNING: this macro cannot > * always detect atomic context; in particular, it cannot know about > * held spinlocks in non-preemptible kernels. Thus it should not be > * used in the general case to determine whether sleeping is possible. > * Do not use in_atomic() in driver code. > */ > #define in_atomic() (preempt_count() != 0) > I had in_interrupt() check in trylock, but dropped it. As of 4.4 in_interrupt() does the following #define hardirq_count() (preempt_count() & HARDIRQ_MASK) #define softirq_count() (preempt_count() & SOFTIRQ_MASK) #define irq_count() (preempt_count() & (HARDIRQ_MASK | SOFTIRQ_MASK \ | NMI_MASK)) #define in_interrupt() (irq_count()) while in_atomic() does #define in_atomic() (preempt_count() != 0) so in_atomic() covers both preemption disabled and in_interrupt(). I take your `non-preemptible kernel' point, thanks. > It might mean that there is no safe way to detect a safe context > on non-preemptible kernels. This might be the most important reason > why we disable preemption when calling console in vprint_emit(). well, if we run a !PREEMPT_COUNT kernel, then #define preempt_disable() barrier() #define preempt_enable() barrier() #define preemptible() 0 so I don't think that preempt_disable()/preempt_enable() were for non-preempt kernels there. otoh, preempt_count still counts irqs, because in_interrupt() (and friends) macro is supposed to work regardless the preempt config selection, so we just lose preempt_disable bit from preempt_count... hm... I think I'll just introduce preemptible() check there. for preempt kernels, preemptible() does #define preemptible() (preempt_count() == 0 && !irqs_disabled()) and for !preempt kernels #define preemptible() 0 iow, no cond_resched() in console_unlock() called from vprintk_emit() on non-preempt kernels. so the console_may_schedule now turns into (composed in mail-app, not actually tested): console_may_schedule = !oops_in_progress && preemptible() && !rcu_preempt_depth(); /* * or a slightly less readable version * !(oops_in_progress || !preemptible() || rcu_preempt_depth()) */ preemptible() implies "preempt_enabled && !in_interrupt() && !irqs_disabled()"; or is always 0 (so it can be optimized out). will test it, but looks legit to me. > > > > + rcu_preempt_depth()); > > > > > > Is it safe to call cond_resched() when the CPU is not online > > > and preemption is enabled, please? Your previous patch > > > suggests that this situation might happen. I guess that > > > it might break the CPU initialization code. > > > > CPU notifies are preemptible. CPU_UP_PREPARE/etc. can schedule, > > there are GFP_KERNEL kmalloc-s from CPU_UP_PREPARE (where cpu > > is not yet online), mutex_lock() calls in cpu_notify handlers, > > and so on. > > Hmm, the notifiers are called via __raw_notifier_call_chain(). > There is a comment above this function: > > * Calls each function in a notifier chain in turn. The functions > * run in an undefined context. > * All locking must be provided by the caller. > > But hmm, you are right that the notifiers do malloc, take mutextes, > etc. The question is if schedule does something in this case. I would > expect that the is no running task assigned to this CPU at this stage. > So, cond_resched is probably a noop in this case. CPU's run queue is not marked online yet at this point (it becomes online in response to CPU_ONLINE notification). so there is not too many tasks to schedule on that CPU -- swapper/X perhaps.. migration/X?. > I am always surprised that printk-world is so tricky. -ss