lkml.org 
[lkml]   [2016]   [Jan]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [RFC][PATCH -next 1/2] printk: move can_use_console out of console_trylock_for_printk
On Wed 2016-01-20 00:00:40, Sergey Senozhatsky wrote:
> > But do we really need to repeat the check in every cycle?
>
> well, on every iteration in the best case we check cpu_online()
> only. which is what we would have done anyway in vprintk_emit(),
> so no additional checks added. at the same time call_console_drivers
> does not do '!cpu_online && !CON_ANYTIME' for each console now, so
> in some sense there are less checks now (this is far even from a
> micro-optimization, just noted).

Hmm, we need to keep the check in call_console_drivers(). It iterates
over all registered consoles. Only some of them could habe CON_ANYTIME
flag. We need to skip the others when the CPU is not online.


> console_unlock() /* w/o can_use_console() in logbuf_lock section */
> ....
> again:
> for (;;) {
> raw_spin_lock_irqsave logbuf_lock
> msg_print_text
> raw_spin_unlock logbuf_lock
> call_console_drivers
> local_irq_restore
> }
>
> up_console_sem
>
> raw_spin_lock logbuf_lock
> retry = console_seq != log_next_seq
> raw_spin_unlock_irqrestore logbuf_lock
>
> if retry && console_trylock()
> goto again;
>
>
> when we up_console_sem(), consoles may appear and disappear, since we
> don't keep the semaphore. Suppose that we have OFFLINE cpu and we had a
> CON_ANYTIME console, but in between up_console_sem--console_trylock
> that single CON_ANYTIME console was removed. So now we have !cpu_online
> and !CON_ANYTIME.

Ah, I have missed that the console_sem is released/taken before goto
again. Hmm, your proposed solution adds even more twists into the code.
I think how to make it easier. I think that we could move the again:
target and call console_cont_flush() when there is some new content.
It is a corner case, anyway. Then we could do:


do_cond_resched = console_may_schedule;
console_may_schedule = 0;

+again:
+ if (!can_use_console()) {
+ console_locked = 0;
+ up_console_sem();
+ return;
}

/* flush buffered message fragment immediately to console */
console_cont_flush(text, sizeof(text));
-again:
for (;;) {
struct printk_log *msg;


Then we remove this check from console_trylock_for_printk() and
eventually remove this function altogether.

It means that the code will be easier and protected against the
theoretical race.

How does that sound, please?

Best Regards,
Petr

\
 
 \ /
  Last update: 2016-01-19 17:21    [W:0.057 / U:0.324 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site