From: Oleg Nesterov <oleg@redhat.com>
To: chenqiwu <qiwuchen55@gmail.com>
Cc: axboe@kernel.dk, keescook@chromium.org,
akpm@linux-foundation.org, mcgrof@kernel.org,
ebiederm@xmission.com, jannh@google.com,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH] exit: dump thread info on global init exit
Date: Fri, 10 Nov 2023 08:41:23 +0100 [thread overview]
Message-ID: <20231110074123.GA7768@redhat.com> (raw)
In-Reply-To: <20231110022720.GA3087@rlk>
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.
prev parent reply other threads:[~2023-11-10 18:17 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20231108081506.149016-1-qiwu.chen@transsion.com>
[not found] ` <20231108095732.GA3678@redhat.com>
[not found] ` <20231109071341.GA14505@rlk>
2023-11-09 11:01 ` [PATCH] exit: dump thread info on global init exit Oleg Nesterov
2023-11-10 2:27 ` chenqiwu
2023-11-10 7:41 ` Oleg Nesterov [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20231110074123.GA7768@redhat.com \
--to=oleg@redhat.com \
--cc=akpm@linux-foundation.org \
--cc=axboe@kernel.dk \
--cc=ebiederm@xmission.com \
--cc=jannh@google.com \
--cc=keescook@chromium.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mcgrof@kernel.org \
--cc=qiwuchen55@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.