Re: [PATCH v2 07/17] debugobjects: Move printk out of db lock critical sections

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

On (11/21/18 11:49), Waiman Long wrote:
[..]
> >  	case ODEBUG_STATE_ACTIVE:
> > -		debug_print_object(obj, "init");
> >  		state = obj->state;
> >  		raw_spin_unlock_irqrestore(&db->lock, flags);
> > +		debug_print_object(obj, "init");
> >  		debug_object_fixup(descr->fixup_init, addr, state);
> >  		return;
> >  
> >  	case ODEBUG_STATE_DESTROYED:
> > -		debug_print_object(obj, "init");
> > +		debug_printobj = true;
> >  		break;
> >  	default:
> >  		break;
> >  	}
> >  
> >  	raw_spin_unlock_irqrestore(&db->lock, flags);
> > +	if (debug_chkstack)
> > +		debug_object_is_on_stack(addr, onstack);
> > +	if (debug_printobj)
> > +		debug_print_object(obj, "init");
> >
[..]
>
> As a side note, one of the test systems that I used generated a
> debugobjects splat in the bootup process and the system hanged
> afterward. Applying this patch alone fix the hanging problem and the
> system booted up successfully. So it is not really a good idea to call
> printk() while holding a raw spinlock.

Right, I like this patch.
And I think that we, maybe, can go even further.

Some serial consoles call mod_timer(). So what we could have with the
debug objects enabled was

	mod_timer()
	 lock_timer_base()
	  debug_activate()
	   printk()
	    call_console_drivers()
	     foo_console()
	      mod_timer()
	       lock_timer_base()       << deadlock

That's one possible scenario. The other one can involve console's
IRQ handler, uart port spinlock, mod_timer, debug objects, printk,
and an eventual deadlock on the uart port spinlock. This one can
be mitigated with printk_safe. But mod_timer() deadlock will require
a different fix.

So maybe we need to switch debug objects print-outs to _always_
printk_deferred(). Debug objects can be used in code which cannot
do direct printk() - timekeeping is just one example.

	-ss