linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: ebiederm@xmission.com (Eric W. Biederman)
To: Christophe Leroy <christophe.leroy@csgroup.eu>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>,
	Paul Mackerras <paulus@samba.org>,
	Michael Ellerman <mpe@ellerman.id.au>,
	hch@infradead.org, linux-kernel@vger.kernel.org,
	linuxppc-dev@lists.ozlabs.org
Subject: Re: [PATCH RESEND v3 6/6] powerpc/signal: Use unsafe_copy_siginfo_to_user()
Date: Mon, 13 Sep 2021 14:11:11 -0500	[thread overview]
Message-ID: <87ilz4mgts.fsf@disp2133> (raw)
In-Reply-To: <96d06ad9-5a9b-b8c3-3c1d-ed8837091a60@csgroup.eu> (Christophe Leroy's message of "Mon, 13 Sep 2021 19:19:26 +0200")

Christophe Leroy <christophe.leroy@csgroup.eu> writes:

> Le 13/09/2021 à 18:21, Eric W. Biederman a écrit :
>> ebiederm@xmission.com (Eric W. Biederman) writes:
>>
>>> Christophe Leroy <christophe.leroy@csgroup.eu> writes:
>>>
>>>> Use unsafe_copy_siginfo_to_user() in order to do the copy
>>>> within the user access block.
>>>>
>>>> On an mpc 8321 (book3s/32) the improvment is about 5% on a process
>>>> sending a signal to itself.
>>
>> If you can't make function calls from an unsafe macro there is another
>> way to handle this that doesn't require everything to be inline.
>>
>>  From a safety perspective it is probably even a better approach.
>
> Yes but that's exactly what I wanted to avoid for the native ppc32 case: this
> double hop means useless pressure on the cache. The siginfo_t structure is 128
> bytes large, that means 8 lines of cache on powerpc 8xx.
>
> But maybe it is acceptable to do that only for the compat case. Let me think
> about it, it might be quite easy.

The places get_signal is called tend to be well known.  So I think we
are safe from a capacity standpoint.

I am not certain it makes a difference in capacity as there is a high
probability that the stack was deeper recently than it is now which
suggests the cache blocks might already be in the cache.

My sense it is worth benchmarking before optimizing out the extra copy
like that.

On the extreme side there is simply building the entire sigframe on the
stack and then just calling it copy_to_user.  As the stack cache lines
are likely to be hot, and copy_to_user is quite well optimized
there is a real possibility that it is faster to build everything
on the kernel stack, and then copy it to the user space stack.

It is also possible that I am wrong and we may want to figure out how
far up we can push the conversion to the 32bit siginfo format.

If could move the work into collect_signal we could guarantee there
would be no extra work.  That would require adjusting the sigframe
generation code on all of the architectures.

There is a lot we can do but we need benchmarking to tell if it is
worth it.

Eric






  reply	other threads:[~2021-09-13 19:11 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-13 15:19 [PATCH RESEND v3 1/6] powerpc/signal64: Access function descriptor with user access block Christophe Leroy
2021-09-13 15:19 ` [PATCH RESEND v3 2/6] powerpc/signal: Include the new stack frame inside the " Christophe Leroy
2021-09-13 15:19 ` [PATCH RESEND v3 3/6] signal: Add unsafe_copy_siginfo_to_user() Christophe Leroy
2021-09-13 15:19 ` [PATCH RESEND v3 4/6] signal: Add unsafe_copy_siginfo_to_user32() Christophe Leroy
2021-09-13 15:54   ` Eric W. Biederman
2021-09-13 17:01     ` Christophe Leroy
2021-09-13 15:19 ` [PATCH RESEND v3 5/6] powerpc/uaccess: Add unsafe_clear_user() Christophe Leroy
2021-09-13 15:19 ` [PATCH RESEND v3 6/6] powerpc/signal: Use unsafe_copy_siginfo_to_user() Christophe Leroy
2021-09-13 15:57   ` Eric W. Biederman
2021-09-13 16:21     ` Eric W. Biederman
2021-09-13 17:19       ` Christophe Leroy
2021-09-13 19:11         ` Eric W. Biederman [this message]
2021-09-14 14:00           ` Christophe Leroy
2021-09-13 17:14     ` Christophe Leroy

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=87ilz4mgts.fsf@disp2133 \
    --to=ebiederm@xmission.com \
    --cc=benh@kernel.crashing.org \
    --cc=christophe.leroy@csgroup.eu \
    --cc=hch@infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=mpe@ellerman.id.au \
    --cc=paulus@samba.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).