From: Oleg Nesterov <oleg@redhat.com>
To: Wen Yang <wenyang.linux@foxmail.com>
Cc: Christian Brauner <brauner@kernel.org>,
Andrew Morton <akpm@linux-foundation.org>,
Thomas Gleixner <tglx@linutronix.de>,
Sebastian Andrzej Siewior <bigeasy@linutronix.de>,
Luis Chamberlain <mcgrof@kernel.org>,
Mike Christie <michael.christie@oracle.com>,
Dmitry Vyukov <dvyukov@google.com>,
Vincent Whitchurch <vincent.whitchurch@axis.com>,
Ard Biesheuvel <ardb@kernel.org>,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/2] kernel/signal.c: explicitly initialize si_code and use ksig->info uniformly
Date: Fri, 23 Feb 2024 10:46:20 +0100 [thread overview]
Message-ID: <20240223094620.GA8267@redhat.com> (raw)
In-Reply-To: <tencent_3867DFAA296AACA094C9E8F413E6493FF407@qq.com>
On 02/23, Wen Yang wrote:
>
> On 2024/2/23 03:05, Oleg Nesterov wrote:
> >On 02/23, wenyang.linux@foxmail.com wrote:
> >>
> >>From: Wen Yang <wenyang.linux@foxmail.com>
> >>
> >>By explicitly initializing ksig->info.si_code and uniformly using ksig->info,
> >>get_signal() function could be slightly optimized, as folowes:
> >
> >I don't understand. Why do you think it will be optimized? in what sense?
> >
> >> clear_siginfo(&ksig->info);
> >> ksig->info.si_signo = signr = SIGKILL; --> missed si_code
> >
> >because we do not need to set .si_code in this case?
> >
> >> sigdelset(¤t->pending.signal, SIGKILL);
> >> trace_signal_deliver(SIGKILL, SEND_SIG_NOINFO, --> unnecessary SEND_SIG_NOINFO
> >
> >Why do you think the usage of SEND_SIG_NOINFO is "unnecessary" or bad?
> >To me this looks good.
> >
>
> Since it is called "SEND_SIG_NOINFO", but here it is neither SEND_SIG
> nor NOINFO.
I don't really understand what does this mean. But I can say that
SEND_SIG_NOINFO is exactly what we should use, this signal has no
info.
In fact, SIGKILL can never have the info, see the sig == SIGKILL
check in __send_signal_locked() but this is offtopic.
> It is get_signal() here, and ksig->info has also been partially
> initialized before calling trace_signal_deliver(). Below "goto fatal",
> do_coredump() also use the initialized ksig->info.
IIRC, do_coredump() paths use only siginfo->si_signo, but this doesn't
matter.
do_coredump() can't be called, sig_kernel_coredump(SIGKILL) is false.
> >and it seems that we can simply kill clear_siginfo(), but this is
> >another story.
>
> This is not right.
>
> ksig->info will be passed to user space through do_coredump(), and the
> clear_siginfo() cannot be killed.
See above.
Oleg.
prev parent reply other threads:[~2024-02-23 9:47 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-02-22 16:04 [PATCH 2/2] kernel/signal.c: explicitly initialize si_code and use ksig->info uniformly wenyang.linux
2024-02-22 19:05 ` Oleg Nesterov
2024-02-23 5:16 ` Wen Yang
2024-02-23 9:46 ` 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=20240223094620.GA8267@redhat.com \
--to=oleg@redhat.com \
--cc=akpm@linux-foundation.org \
--cc=ardb@kernel.org \
--cc=bigeasy@linutronix.de \
--cc=brauner@kernel.org \
--cc=dvyukov@google.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mcgrof@kernel.org \
--cc=michael.christie@oracle.com \
--cc=tglx@linutronix.de \
--cc=vincent.whitchurch@axis.com \
--cc=wenyang.linux@foxmail.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.