Re: [PATCH] exit: dump thread info on global init exit

[Oleg Nesterov]

On 11/10, chenqiwu wrote:
>
> On Thu, Nov 09, 2023 at 12:01:30PM +0100, Oleg Nesterov wrote:
> > I've just noticed we discuss this offlist. Add lkml...
> >
> > On 11/09, chenqiwu wrote:
> > >
> > > On Wed, Nov 08, 2023 at 10:57:32AM +0100, Oleg Nesterov wrote:
> > >
> > > > > +	if (mmap_read_lock_killable(mm)) {
> > > >
> > > > why do you need _killable ?
> > > >
> > > I'm not sure which type lock (killable or unkillable) should be used here
> >
> > killable should be used to allow to kill the task which waits for this lock.
> > Who can kill the global init? Yes it is possible (but very unlikely) that
> > fatal_signal_pending() is true, but I don't think this was your concern.
> >
> > > if there is a lock contention, perhaps using down_read_trylock is better.
> >
> > Perhaps. If we have another bug mmap_read_lock() can hang forever.
> >
> Yes, but we really don't want to hang here forever if we cannot get the mmap read
> lock.

Yes, this is what I tried to say.

> I think using down_read_trylock should be better, I wiil resend the patch as
> V2 cc LKML to discuss the thing.

OK.

> > > > > +static void dump_thread_info(struct task_struct *tsk)
> > > > > +{
> > > > > +	struct pt_regs *regs = task_pt_regs(tsk);
> > > > > +
> > > > > +	if (user_mode(regs))
> > > > > +		dump_thread_maps_info(tsk);
> > > > > +	show_regs(regs);
> > > >
> > > > This looks confusing to me...
> > > >
> > > > How can user_mode() return false in this case? And even if this is
> > > > possible, then show_regs() should depend on user_mode() as well?
> > > > I must have missed something.
> > > >
> > > Sure, the last global init thread cannot be exited in non-user mode.
> >
> > Forgot to mention... panic() should dump the regs, so I think show_regs()
> > is not needed?
> >
> In fact, panic don't dump the regs. For example, the current kill init panic log from Andriod system:

Hmm. OK, panic()->dump_stack() depends on CONFIG_DEBUG_BUGVERBOSE, so perhaps
Andriod runs without CONFIG_DEBUG_BUGVERBOSE... or perhaps show_stack() doesn't
do __show_regs() on arm64 ? I dunno.

OK, I see you have already sent V2.

Oleg.