From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755987AbcAWEs3 (ORCPT ); Fri, 22 Jan 2016 23:48:29 -0500 Received: from mail-pf0-f173.google.com ([209.85.192.173]:33886 "EHLO mail-pf0-f173.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755785AbcAWEs1 (ORCPT ); Fri, 22 Jan 2016 23:48:27 -0500 Date: Sat, 23 Jan 2016 13:46:39 +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: <20160123044639.GA597@swordfish> References: <20160117141137.GA631@swordfish> <20160117142332.GA543@swordfish> <20160118161744.GZ3178@pathway.suse.cz> <20160119011510.GB4963@swordfish> <20160119151801.GB2148@dhcp128.suse.cz> <20160120035056.GA506@swordfish> <20160120123107.GB3305@pathway.suse.cz> <20160121012551.GA594@swordfish> <20160121055146.GA398@swordfish> <20160122094844.GY731@pathway.suse.cz> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20160122094844.GY731@pathway.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/22/16 10:48), Petr Mladek wrote: [..] > > and in console_unlock() > > > > - if (do_cond_resched) > > - cond_resched(); > > + console_conditional_schedule(); > > > > > > but for !CONFIG_PREEMPT_COUNT we can't. because of currently held spin_locks/etc > > that we don't know about. > > Ah, I was not aware that we did not have information about preemption > without PREEMPT_COUNT. yes, for example, static inline void __raw_spin_lock(raw_spinlock_t *lock) { preempt_disable(); spin_acquire(&lock->dep_map, 0, 0, _RET_IP_); LOCK_CONTENDED(lock, do_raw_spin_trylock, do_raw_spin_lock); } where preempt_disable() include/linux/preempt.h ... #else /* !CONFIG_PREEMPT_COUNT */ /* * Even if we don't have any preemption, we need preempt disable/enable * to be barriers, so that we don't have things like get_user/put_user * that can cause faults and scheduling migrate into our preempt-protected * region. */ #define preempt_disable() barrier() #define preempt_enable() barrier() so on !CONFIG_PREEMPT_COUNT kernels we can't rely on console_trylock() 'magic', we need the existing rules. > > `console_may_schedule' carries a bit of important information for > > console_conditional_schedule() caller. if it has acquired console_sem > > via console_lock() - then it can schedule, if via console_trylock() - it cannot. > > > > the last `if via console_trylock() - it cannot' rule is not always true, > > we clearly can have printk()->console_unlock() from non-atomic contexts > > (if we know that its non-atomic, which is not the case with !PREEMPT_COUNT). > > By other words, we could automatically detect save context for > cond_resched() only if PREEMPT_COUNT is enabled. Otherwise, we need to > keep the current logic (heuristic). Do I get it correctly, please? yes, I think so. > I would personally wait a bit for Jack's async console printing. > It will call console only if oops_in_progress is set. It means that > this partial optimization won't be needed at all. ok, thanks. I'd love to see Jan's printk() rework being merged. I have 99 problems with printk() and console_unlock(). People usually are not aware of the secrets that printk-console_unlock have; and tend to think that printk is just 'a kernel way' of spelling printf, with all the consequences that follows -- excessive printk usage, RCU stalls, soft lockups, etc. And that printk abuse does not necessarily hit the abuser. A completely 'innocent' user space application that does a syscall which involves console_lock-console_unlock, can spend seconds in console_unlock pushing someone's data to console_drivers. console_lock and console_unlock, I think, have a bit misleading naming. _lock has acquire semantics, _unlock, however, does not simply release the lock. I even think it'd be good to have console_unlock_fast(), that would just up_console_sem() w/o any penalty. So some of console_unlock() that are 'accessible' by user-space /* for example, tty_open() tty_lookup_driver() console_device() console_lock() console_unlock() or reading from /proc/consoles, and so on and forth */ could be replaced with console_unlock_fast(). The patch in question is simply a further extension on Tejun's work. And these two patches already made my life a bit simpler, albeit not all of the printk/console_unlock problems were addressed. Jan's patch set is a much more complicated effort, and it may take 2 or 3 (??) kernel releases to finish (there are corner cases: for example, workers can stall during OOM, etc.), I'd be happy to see it in -next for 4.6, personally, not sure how realistic this expectation is. > The other (first) patch still makes sense in the simplified form. thanks. let's do it this way - I'll keep the preempt disable/enable removal patch the last in the series, so we can easily drop it (if Jan's rework is much-much closer). How does that sound? -ss > Best Regards, > Petr >