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

Sergey Senozhatsky
2018-11-22T02:04:29Z

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