Re: printk: reset may_schedule if console_trylock() from console_unlock()

[Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>]

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