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

Groups > linux.kernel > #1495401

Re: [RFC][PATCH 6/7] printk: use alternative printk buffers

Show all headers | View raw

On Sat 2016-10-01 11:48:29, Sergey Senozhatsky wrote:
> On (09/30/16 13:15), Petr Mladek wrote:
> > > do you mean that, once alt_printk is done properly, we can drop
> > > printk_deferred()? I was thinking of it, but decided not to
> > > mention/touch it in this patch set.
> > 
> > My understanding is the following:
> > 
> > The difference between normal printk() and printk_deferred() is
> > that the other does not call console_trylock()/console_unlock().
> > It means that printk_deferred() can avoid recursion only from these
> > two calls.
> 
> yes.
> 
> > printk_deferred() is used only in scheduler and timekeeping code.
> > Therefore it prevents only limited number of possible recursions
> > and deadlocks at the moment.
> > 
> > This patch guards most of the two calls a more generic way.
> > The redirected parts prevent recursion not only to into the
> > code guarded by console_sem but also into parts guarded
> > by lockbuf_lock.
> 
> yes. I'm considering to extend it to "non-recursive printk" cases
> sometime in the future. it's easy to protect lockbuf_lock, but not
> so easy to protect sleeping console_lock().
> 
> the cases I'm talking of are (for instance):
> 
>   devkmsg_open()
>    raw_spin_lock_irq(&logbuf_lock)
>     spin_dump()
>      printk()
>       raw_spin_lock_irq(&logbuf_lock)  << deadlock

I have finally got it. You are right, there are still some other
possible deadlocks.

But these are not protected at the moment. Neither printk_deferred()
not logbuf_cpu prevent this. Therefore this is not a reason to keep
either printk_deferred() or logbuf_cpu.


> entering to alt_printk mode each time we take the `logbuf_lock' sort
> of makes sense. can be done later, don't want to overload this patch set.

I agree that it does not make sense to solve these other problems in
this patchset.


> addressing sleeping console_sem function is not super hard in general.
> we just would have to put alt_printk_enter/exit into scheduler code, or
> at least into semaphore code. which can be hard to sell. there are
> gazillions of semaphores and we need to protect only one of them, but
> have to do alt_printk_enter/exit for every.

Yeah, so a generic solution for console_sem might be rather hard.
I would not complicate this patchset with it.


> > By other words, this patch is supposed to handle a superset
> > of the deadlocks that are currently prevented by printk_deferred().
> > If this is true, we do not longer need printk_deferred().
> 
> yes, I suppose so.
> the only difference here is that printk_deferred() immediately puts the
> message into logbuf (well, *if* it can lock the `logbuf_lock'), while
> vprintk_alt() puts it into per-cpu buffer first and needs an irq work in
> that CPU to flush it to logbuf. OTOH, vprintk_alt() does not depend on
> `logbuf_lock'.

On the other hand, the per-CPU buffer will include only error
messages from the printk() code. This is why I am fine with
the extra buffer. The only way is to use printk_deferred()
these days. And as you said, this was error prone and hard
to maintain.


> > The only question is if this patch guards enough parts of
> > console_try_lock()/console_unlock() to handle the superset
> > of the possible deadlocks.
> > 
> > I see that it does not guard two up_console_sem() calls
> > from console_unlock(). But this can be fixed in the next
> > version.
> >
> > Or is there any other catch that I do not see at the moment?
> 
> it's a bit tricky. we break from printing loop only with logbuf_lock
> spin_lock locked, irqs disabled and in alt_printk mode. so everything
> after the printing loop is still protected, up until
> 
> 	raw_spin_lock(&logbuf_lock);
> 	retry = console_seq != log_next_seq;
> 	raw_spin_unlock(&logbuf_lock);
> 	alt_printk_exit();
> 	local_irq_restore(flags);

Yes, but we are safe to call normal printk() at this point.
lockbuf_lock is released => no danger of a deadlock.
console_sem is taken but this is not a problem. printk()
will do console_trylock() protected by the alt_printk_enter()/exit().
Therefore the trylock will fail but it could not cause a deadlock.

We just need to replace

	if (retry && console_trylock())
		goto again;

with a safe variant, something like

	if (retry) {
		local_irq_save(flags);
		alt_printk_enter();
		lock_failed = console_trylock();
		alt_printk_exit();
		local_irq_restore(flags);

		if (!lock_failed)
			goto again;
	}

Or do I miss anything?

> > In each case, getting rid of printk_deferred() could be
> > a fantastic selling point for this patchset.
> 
> agree.

I believe that it is doable and worth try.

I am going to look at the second version of the patchset.

Best Regards,
Petr

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

Thread

[RFC][PATCH 0/7] printk: use alt_printk to handle printk() recursive calls Sergey Senozhatsky <sergey.senozhatsky@gmail.com> - 2016-09-27 16:30 +0200
  [RFC][PATCH 5/7] printk: drop vprintk_func function Sergey Senozhatsky <sergey.senozhatsky@gmail.com> - 2016-09-27 16:30 +0200
  [RFC][PATCH 3/7] printk: introduce per-cpu alt_print seq buffer Sergey Senozhatsky <sergey.senozhatsky@gmail.com> - 2016-09-27 16:30 +0200
    Re: [RFC][PATCH 3/7] printk: introduce per-cpu alt_print seq buffer Petr Mladek <pmladek@suse.com> - 2016-09-29 14:30 +0200
      Re: [RFC][PATCH 3/7] printk: introduce per-cpu alt_print seq buffer Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> - 2016-09-30 03:10 +0200
        Re: [RFC][PATCH 3/7] printk: introduce per-cpu alt_print seq buffer Petr Mladek <pmladek@suse.com> - 2016-09-30 13:40 +0200
  [RFC][PATCH 1/7] printk: use vprintk_func in vprintk() Sergey Senozhatsky <sergey.senozhatsky@gmail.com> - 2016-09-27 16:30 +0200
  [RFC][PATCH 2/7] printk: rename nmi.c and exported api Sergey Senozhatsky <sergey.senozhatsky@gmail.com> - 2016-09-27 16:30 +0200
  [RFC][PATCH 6/7] printk: use alternative printk buffers Sergey Senozhatsky <sergey.senozhatsky@gmail.com> - 2016-09-27 16:30 +0200
    Re: [RFC][PATCH 6/7] printk: use alternative printk buffers Petr Mladek <pmladek@suse.com> - 2016-09-29 15:10 +0200
      Re: [RFC][PATCH 6/7] printk: use alternative printk buffers Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> - 2016-09-30 03:20 +0200
        Re: [RFC][PATCH 6/7] printk: use alternative printk buffers Petr Mladek <pmladek@suse.com> - 2016-09-30 13:20 +0200
          Re: [RFC][PATCH 6/7] printk: use alternative printk buffers Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> - 2016-10-01 04:50 +0200
            Re: [RFC][PATCH 6/7] printk: use alternative printk buffers Petr Mladek <pmladek@suse.com> - 2016-10-04 14:30 +0200
              Re: [RFC][PATCH 6/7] printk: use alternative printk buffers Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> - 2016-10-05 03:40 +0200
                Re: [RFC][PATCH 6/7] printk: use alternative printk buffers Petr Mladek <pmladek@suse.com> - 2016-10-05 12:20 +0200
          Re: [RFC][PATCH 6/7] printk: use alternative printk buffers Sergey Senozhatsky <sergey.senozhatsky@gmail.com> - 2016-10-03 10:00 +0200
          Re: [RFC][PATCH 6/7] printk: use alternative printk buffers Petr Mladek <pmladek@suse.com> - 2016-10-04 17:00 +0200
            Re: [RFC][PATCH 6/7] printk: use alternative printk buffers Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> - 2016-10-05 03:30 +0200
              Re: [RFC][PATCH 6/7] printk: use alternative printk buffers Petr Mladek <pmladek@suse.com> - 2016-10-05 12:00 +0200
                Re: [RFC][PATCH 6/7] printk: use alternative printk buffers Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> - 2016-10-06 06:30 +0200
                Re: [RFC][PATCH 6/7] printk: use alternative printk buffers Petr Mladek <pmladek@suse.com> - 2016-10-06 13:40 +0200
                Re: [RFC][PATCH 6/7] printk: use alternative printk buffers Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> - 2016-10-10 06:20 +0200
                Re: [RFC][PATCH 6/7] printk: use alternative printk buffers Petr Mladek <pmladek@suse.com> - 2016-10-10 13:20 +0200
                Re: [RFC][PATCH 6/7] printk: use alternative printk buffers Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> - 2016-10-11 09:40 +0200
                Re: [RFC][PATCH 6/7] printk: use alternative printk buffers Petr Mladek <pmladek@suse.com> - 2016-10-11 11:40 +0200
  [RFC][PATCH 4/7] printk: make alt_printk available when config printk set Sergey Senozhatsky <sergey.senozhatsky@gmail.com> - 2016-09-27 16:30 +0200
  [RFC][PATCH 7/7] printk: new printk() recursion detection Sergey Senozhatsky <sergey.senozhatsky@gmail.com> - 2016-09-27 16:30 +0200
    Re: [RFC][PATCH 7/7] printk: new printk() recursion detection Petr Mladek <pmladek@suse.com> - 2016-09-29 15:30 +0200
      Re: [RFC][PATCH 7/7] printk: new printk() recursion detection Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> - 2016-09-30 04:10 +0200
  Re: [RFC][PATCH 0/7] printk: use alt_printk to handle printk()  recursive calls Petr Mladek <pmladek@suse.com> - 2016-09-29 15:30 +0200
    Re: [RFC][PATCH 0/7] printk: use alt_printk to handle printk()  recursive calls Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> - 2016-09-30 04:50 +0200
      Re: [RFC][PATCH 0/7] printk: use alt_printk to handle printk()  recursive calls Petr Mladek <pmladek@suse.com> - 2016-09-30 13:30 +0200
        Re: [RFC][PATCH 0/7] printk: use alt_printk to handle printk()  recursive calls Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> - 2016-10-01 05:10 +0200
          Re: [RFC][PATCH 0/7] printk: use alt_printk to handle printk()  recursive calls Petr Mladek <pmladek@suse.com> - 2016-10-04 13:40 +0200

csiph-web