All of lore.kernel.org
 help / color / mirror / Atom feed
From: Oleg Nesterov <oleg@redhat.com>
To: 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: Thu, 22 Feb 2024 20:05:47 +0100	[thread overview]
Message-ID: <20240222190546.GA5993@redhat.com> (raw)
In-Reply-To: <tencent_195B55A5521705954D5EA4873327F8E53D0A@qq.com>

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(&current->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.

> @@ -2732,8 +2732,9 @@ bool get_signal(struct ksignal *ksig)
>  		     signal->group_exec_task) {
>  			clear_siginfo(&ksig->info);
>  			ksig->info.si_signo = signr = SIGKILL;
> +			ksig->info.si_code = SI_USER;
>  			sigdelset(&current->pending.signal, SIGKILL);
> -			trace_signal_deliver(SIGKILL, SEND_SIG_NOINFO,
> +			trace_signal_deliver(SIGKILL, &ksig->info,

Well. to me this look like the minor but unnecessary pessimization.

AFAICS, we do not need to initialize .si_code. The usage if ksig->info
instead of ksig->info means that TP_STORE_SIGINFO() will actually read
the memory.

Sorry, I don't understand the point at all :/

and it seems that we can simply kill clear_siginfo(), but this is
another story.

Oleg.


  reply	other threads:[~2024-02-22 19:09 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 [this message]
2024-02-23  5:16   ` Wen Yang
2024-02-23  9:46     ` Oleg Nesterov

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=20240222190546.GA5993@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.