Messages in this thread | | | Date | Sat, 23 Jan 2016 13:46:39 +0900 | From | Sergey Senozhatsky <> | Subject | Re: [RFC][PATCH -next 2/2] printk: set may_schedule for some of console_trylock callers |
| |
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 >
| |