Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1312363

Re: [RFC][PATCH -next 1/2] printk: move can_use_console out of console_trylock_for_printk

Show all headers | View raw

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

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

Re: [RFC][PATCH -next 1/2] printk: move can_use_console out of  console_trylock_for_printk Petr Mladek <pmladek@suse.com> - 2016-01-18 16:50 +0100
  Re: [RFC][PATCH -next 1/2] printk: move can_use_console out of  console_trylock_for_printk Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> - 2016-01-19 01:50 +0100
    Re: [RFC][PATCH -next 1/2] printk: move can_use_console out of  console_trylock_for_printk Petr Mladek <pmladek@suse.com> - 2016-01-19 14:40 +0100
      Re: [RFC][PATCH -next 1/2] printk: move can_use_console out of  console_trylock_for_printk Sergey Senozhatsky <sergey.senozhatsky@gmail.com> - 2016-01-19 16:10 +0100
        Re: [RFC][PATCH -next 1/2] printk: move can_use_console out of  console_trylock_for_printk Petr Mladek <pmladek@suse.com> - 2016-01-19 17:20 +0100
          Re: [RFC][PATCH -next 1/2] printk: move can_use_console out of  console_trylock_for_printk Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> - 2016-01-20 05:20 +0100
            Re: [RFC][PATCH -next 1/2] printk: move can_use_console out of  console_trylock_for_printk Petr Mladek <pmladek@suse.com> - 2016-01-20 11:10 +0100

csiph-web