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 -- To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
