Re: [PATCH v2] printk: Relocate wake_klogd check close to the end of console_unlock()

[Sergey Senozhatsky]

Hello,

Sorry for the delay. Could not reply sooner.


On (02/19/18 16:58), Petr Mladek wrote:
[..]
> > @@ -2417,12 +2413,17 @@ void console_unlock(void)
> >  	up_console_sem();
> >  
> >  	/*
> > -	 * Someone could have filled up the buffer again, so re-check if there's
> > -	 * something to flush. In case we cannot trylock the console_sem again,
> > -	 * there's a new owner and the console_unlock() from them will do the
> > -	 * flush, no worries.
> > +	 * Check whether userland needs notification.  Also, someone could
> > +	 * have filled up the buffer again, so re-check if there's
> > +	 * something to flush. In case we cannot trylock the console_sem
> > +	 * again, there's a new owner and the console_unlock() from them
> > +	 * will do the flush, no worries.
> >  	 */
> >  	raw_spin_lock(&logbuf_lock);
> > +	if (seen_seq != log_next_seq) {
> > +		wake_klogd = true;
> > +		seen_seq = log_next_seq;
> 
> Sigh, there is actually still a race with console_trylock_spinning().

I see what you are talking about. Good catch.

> The simplest solution seems to be to do this only when !retry.

Yeah, offloading makes any "internal state" flags almost impossible to
use. We either need to upgrade those "internal state" flags to "global
state" flags, or to add more complex checks/race conditions workarounds
and so on. So I think that !retry should work. But, honestly, the whole
wakeup_klogd thing is getting too complex to like it.

	-ss