linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
From: Kuniyuki Iwashima <kuniyu@google.com>
To: dave.hansen@intel.com
Cc: alex@ghiti.fr, aou@eecs.berkeley.edu, axboe@kernel.dk,
	bp@alien8.de,  brauner@kernel.org, catalin.marinas@arm.com,
	christophe.leroy@csgroup.eu,  dave.hansen@linux.intel.com,
	edumazet@google.com, hpa@zytor.com,  kuni1840@gmail.com,
	kuniyu@google.com, linux-arm-kernel@lists.infradead.org,
	 linux-kernel@vger.kernel.org, linux-riscv@lists.infradead.org,
	 linuxppc-dev@lists.ozlabs.org, maddy@linux.ibm.com,
	mingo@redhat.com,  mpe@ellerman.id.au, npiggin@gmail.com,
	palmer@dabbelt.com, pjw@kernel.org,  tglx@linutronix.de,
	torvalds@linux-foundation.org, will@kernel.org,  x86@kernel.org
Subject: Re: [PATCH v1 2/2] epoll: Use __user_write_access_begin() and unsafe_put_user() in epoll_put_uevent().
Date: Tue, 28 Oct 2025 05:32:13 +0000	[thread overview]
Message-ID: <20251028053330.2391078-1-kuniyu@google.com> (raw)
In-Reply-To: <ea7552f1-842c-4bb8-b19e-0410bf18c305@intel.com>

From: Dave Hansen <dave.hansen@intel.com>
Date: Fri, 24 Oct 2025 07:05:50 -0700
> On 10/23/25 22:16, Kuniyuki Iwashima wrote:
> >> This makes me nervous. The access_ok() check is quite a distance away.
> >> I'd kinda want to see some performance numbers before doing this. Is
> >> removing a single access_ok() even measurable?
> > I noticed I made a typo in commit message, s/tcp_rr/udp_rr/.
> > 
> > epoll_put_uevent() can be called multiple times in a single
> > epoll_wait(), and we can see 1.7% more pps on UDP even when
> > 1 thread has 1000 sockets only:
> > 
> > server: $ udp_rr --nolog -6 -F 1000 -T 1 -l 3600
> > client: $ udp_rr --nolog -6 -F 1000 -T 256 -l 3600 -c -H $SERVER
> > server: $ nstat > /dev/null; sleep 10; nstat | grep -i udp
> > 
> > Without patch (2 stac/clac):
> > Udp6InDatagrams                 2205209            0.0
> > 
> > With patch (1 stac/clac):
> > Udp6InDatagrams                 2242602            0.0
> 
> I'm totally with you about removing a stac/clac:
> 
> 	https://lore.kernel.org/lkml/20250228203722.CAEB63AC@davehans-spike.ostc.intel.com/
> 
> The thing I'm worried about is having the access_ok() so distant
> from the unsafe_put_user(). I'm wondering if this:
> 
> -	__user_write_access_begin(uevent, sizeof(*uevent));
> +	if (!user_write_access_begin(uevent, sizeof(*uevent))
> +		return NULL;
> 	unsafe_put_user(revents, &uevent->events, efault);
> 	unsafe_put_user(data, &uevent->data, efault);
> 	user_access_end();
> 
> is measurably slower than what was in your series. If it is
> not measurably slower, then the series gets simpler because it
> does not need to refactor user_write_access_begin(). It also ends
> up more obviously correct because the access check is closer to
> the unsafe_put_user() calls.
> 
> Also, the extra access_ok() is *much* cheaper than stac/clac.

Sorry for the late!

I rebased on 19ab0a22efbd and tested 4 versions on
AMD EPYC 7B12 machine:

 1) Base 19ab0a22efbd

 2) masked_user_access_begin()
    -> 97% pps and 96% calls of ep_try_send_events()

 3) user_write_access_begin() (Dave's diff above) (NEW)
    -> 102.2% pps and 103% calls of ep_try_send_events()

 4) __user_write_access_begin() (This patch)
    -> 102.4% pps and 103% calls of ep_try_send_events().

Interestingly user_write_access_begin() was as fast as
__user_write_access_begin() !

Also, as with the previous result, masked_user_access_begin()
was the worst somehow.

So, I'll drop patch 1 and post v2 with user_write_access_begin().

Thank you!


1) Base (19ab0a22efbd)

# nstat > /dev/null; sleep 10; nstat | grep -i udp
Udp6InDatagrams                 2184011            0.0

@ep_try_send_events_ns:
[256, 512)       2796601 |@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@|
[512, 1K)         627863 |@@@@@@@@@@@                                         |
[1K, 2K)          166403 |@@@                                                 |
[2K, 4K)           10437 |                                                    |
[4K, 8K)            1396 |                                                    |
[8K, 16K)            116 |                                                    |


2) masked_user_access_begin() + masked_user_access_begin()
97% pps compared to 1).
96% calls of ep_try_send_events().

# nstat > /dev/null; sleep 10; nstat | grep -i udp
Udp6InDatagrams                 2120498            0.0

@ep_try_send_events_ns:
[256, 512)       2690803 |@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@|
[512, 1K)         533750 |@@@@@@@@@@                                          |
[1K, 2K)          225969 |@@@@                                                |
[2K, 4K)           35176 |                                                    |
[4K, 8K)            2428 |                                                    |
[8K, 16K)            199 |                                                    |


3) user_write_access_begin()
102.2% pps compared to 1).
103% calls of ep_try_send_events().

# nstat > /dev/null; sleep 10; nstat | grep -i udp
Udp6InDatagrams                 2232730            0.0

@ep_try_send_events_ns:
[256, 512)       2900655 |@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@|
[512, 1K)         622045 |@@@@@@@@@@@                                         |
[1K, 2K)          172831 |@@@                                                 |
[2K, 4K)           17687 |                                                    |
[4K, 8K)            1103 |                                                    |
[8K, 16K)            174 |                                                    |


4) __user_write_access_begin()
102.4% pps compared to 1).
103% calls of ep_try_send_events().

# nstat > /dev/null; sleep 10; nstat | grep -i udp
Udp6InDatagrams                 2238524            0.0

@ep_try_send_events_ns:
[256, 512)       2906752 |@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@|
[512, 1K)         630199 |@@@@@@@@@@@                                         |
[1K, 2K)          161741 |@@                                                  |
[2K, 4K)           17141 |                                                    |
[4K, 8K)            1041 |                                                    |
[8K, 16K)             61 |                                                    |



  parent reply	other threads:[~2025-10-28  5:33 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-10-23  0:04 [PATCH v1 0/2] epoll: Save one stac/clac pair in epoll_put_uevent() Kuniyuki Iwashima
2025-10-23  0:04 ` [PATCH v1 1/2] uaccess: Add __user_write_access_begin() Kuniyuki Iwashima
2025-10-23  5:37   ` Linus Torvalds
2025-10-23  8:29     ` David Laight
2025-10-24  5:31       ` Kuniyuki Iwashima
2025-10-23  0:04 ` [PATCH v1 2/2] epoll: Use __user_write_access_begin() and unsafe_put_user() in epoll_put_uevent() Kuniyuki Iwashima
2025-10-23 19:40   ` Dave Hansen
2025-10-24  5:16     ` Kuniyuki Iwashima
2025-10-24 14:05       ` Dave Hansen
2025-10-24 14:47         ` David Laight
2025-10-28  5:32         ` Kuniyuki Iwashima [this message]
2025-10-28  9:54           ` David Laight
2025-10-28 16:42             ` Kuniyuki Iwashima
2025-10-28 16:58               ` Linus Torvalds
2025-10-29  1:42                 ` Andrew Cooper
2025-10-28 22:30               ` David Laight

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=20251028053330.2391078-1-kuniyu@google.com \
    --to=kuniyu@google.com \
    --cc=alex@ghiti.fr \
    --cc=aou@eecs.berkeley.edu \
    --cc=axboe@kernel.dk \
    --cc=bp@alien8.de \
    --cc=brauner@kernel.org \
    --cc=catalin.marinas@arm.com \
    --cc=christophe.leroy@csgroup.eu \
    --cc=dave.hansen@intel.com \
    --cc=dave.hansen@linux.intel.com \
    --cc=edumazet@google.com \
    --cc=hpa@zytor.com \
    --cc=kuni1840@gmail.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-riscv@lists.infradead.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=maddy@linux.ibm.com \
    --cc=mingo@redhat.com \
    --cc=mpe@ellerman.id.au \
    --cc=npiggin@gmail.com \
    --cc=palmer@dabbelt.com \
    --cc=pjw@kernel.org \
    --cc=tglx@linutronix.de \
    --cc=torvalds@linux-foundation.org \
    --cc=will@kernel.org \
    --cc=x86@kernel.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).