Messages in this thread | | | Date | Thu, 5 Jan 2017 19:56:18 +0900 | From | Sergey Senozhatsky <> | Subject | Re: printk: reset may_schedule if console_trylock() from console_unlock() |
| |
On (01/05/17 19:27), Tetsuo Handa wrote: [..] > > the other thing is... do we really need to console_conditional_schedule() > > from fbcon_*()? console_unlock() does cond_resched() after every line it > > prints. wouldn't that be enough? > > Every record, isn't it?
yes. after every call to console drivers.
> How many bytes can a record write to consoles?
1024 - PREFIX_MAX bytes at most.
> > so may be we can drop some of console_conditional_schedule() > > call sites in fbcon. or update console_conditional_schedule() > > function to always return the current preemption value, not the > > one we saw in console_trylock(). > > Replacing console_may_schedule with get_console_may_schedule() will avoid > this bug. I noticed that we forgot to reset console_may_schedule to 0 > because the "again:" label is located after the assignment line.
well... we call cond_resched() from under the spin_lock(), which modifies the preempt count and cond_resched() which we call from console_conditional_schedule() checks that current->preempt count is 0 before it calls tif_need_resched().
there are configs/setups where spinlock does not modify preempt count, but I suspect that on those setups cond_resched() does nothing.
so it looks to me that we just WARN from ___might_sleep() but, at least in this particular case, I don't think we can actually schedule(). but I just had a very quick look, so I may be completely wrong. need to double check.
what I tried to say: -- I will send out a patch for printk() once we settle down the current work in progress.
why do I want to address this in printk? because who knows, may be there is (or there will be) something out there that takes rcu_read_lock() and then does the console_conditional_schedule(). rcu does not modify current preempt count, it has its own preempt counter, and get_console_may_schedule() takes that into account.
> What happened here was: > > [...]
I need more time to walk through your analysis/proposal.
-ss
| |