From: Linus Torvalds <torvalds@linux-foundation.org>
To: kernel test robot <oliver.sang@intel.com>
Cc: Jens Axboe <axboe@kernel.dk>, Feng Tang <feng.tang@intel.com>,
0day robot <lkp@intel.com>,
Kernel Hardening <kernel-hardening@lists.openwall.com>,
Linux Containers <containers@lists.linux-foundation.org>,
Jann Horn <jannh@google.com>, LKML <linux-kernel@vger.kernel.org>,
Oleg Nesterov <oleg@redhat.com>, Linux-MM <linux-mm@kvack.org>,
lkp@lists.01.org, "Eric W . Biederman" <ebiederm@xmission.com>,
"Huang, Ying" <ying.huang@intel.com>,
Andrew Morton <akpm@linux-foundation.org>,
zhengjun.xing@intel.com, Alexey Gladkov <legion@kernel.org>,
Kees Cook <keescook@chromium.org>
Subject: Re: 08ed4efad6: stress-ng.sigsegv.ops_per_sec -41.9% regression
Date: Thu, 8 Apr 2021 09:22:40 -0700 [thread overview]
Message-ID: <CAHk-=wigPx+MMQMQ-7EA0pq5_5+kMCNV4qFsOss-WwdCSQmb-w@mail.gmail.com> (raw)
In-Reply-To: <20210408083026.GE1696@xsang-OptiPlex-9020>
On Thu, Apr 8, 2021 at 1:32 AM kernel test robot <oliver.sang@intel.com> wrote:
>
> FYI, we noticed a -41.9% regression of stress-ng.sigsegv.ops_per_sec due to commit
> 08ed4efad684 ("[PATCH v10 6/9] Reimplement RLIMIT_SIGPENDING on top of ucounts")
Ouch.
I *think* this test may be testing "send so many signals that it
triggers the signal queue overflow case".
And I *think* that the performance degradation may be due to lots of
unnecessary allocations, because ity looks like that commit changes
__sigqueue_alloc() to do
struct sigqueue *q = kmem_cache_alloc(sigqueue_cachep, flags);
*before* checking the signal limit, and then if the signal limit was
exceeded, it will just be free'd instead.
The old code would check the signal count against RLIMIT_SIGPENDING
*first*, and if there were m ore pending signals then it wouldn't do
anything at all (including not incrementing that expensive atomic
count).
Also, the old code was very careful to only do the "get_user()" for
the *first* signal it added to the queue, and do the "put_user()" for
when removing the last signal. Exactly because those atomics are very
expensive.
The new code just does a lot of these atomics unconditionally.
I dunno. The profile data in there is a bit hard to read, but there's
a lot more cachee misses, and a *lot* of node crossers:
> 5961544 +190.4% 17314361 perf-stat.i.cache-misses
> 22107466 +119.2% 48457656 perf-stat.i.cache-references
> 163292 ą 3% +4582.0% 7645410 perf-stat.i.node-load-misses
> 227388 ą 2% +3708.8% 8660824 perf-stat.i.node-loads
and (probably as a result) average instruction costs have gone up enormously:
> 3.47 +66.8% 5.79 perf-stat.overall.cpi
> 22849 -65.6% 7866 perf-stat.overall.cycles-between-cache-misses
and it does seem to be at least partly about "put_ucounts()":
> 0.00 +4.5 4.46 perf-profile.calltrace.cycles-pp.put_ucounts.__sigqueue_free.get_signal.arch_do_signal_or_restart.exit_to_user_mode_prepare
and a lot of "get_ucounts()".
But it may also be that the new "get sigpending" is just *so* much
more expensive than it used to be.
Linus
_______________________________________________
Containers mailing list
Containers@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/containers
WARNING: multiple messages have this Message-ID (diff)
From: Linus Torvalds <torvalds@linux-foundation.org>
To: kernel test robot <oliver.sang@intel.com>
Cc: Alexey Gladkov <gladkov.alexey@gmail.com>,
0day robot <lkp@intel.com>, LKML <linux-kernel@vger.kernel.org>,
lkp@lists.01.org, "Huang, Ying" <ying.huang@intel.com>,
Feng Tang <feng.tang@intel.com>,
zhengjun.xing@intel.com,
Kernel Hardening <kernel-hardening@lists.openwall.com>,
Linux Containers <containers@lists.linux-foundation.org>,
Linux-MM <linux-mm@kvack.org>, Alexey Gladkov <legion@kernel.org>,
Andrew Morton <akpm@linux-foundation.org>,
Christian Brauner <christian.brauner@ubuntu.com>,
"Eric W . Biederman" <ebiederm@xmission.com>,
Jann Horn <jannh@google.com>, Jens Axboe <axboe@kernel.dk>,
Kees Cook <keescook@chromium.org>,
Oleg Nesterov <oleg@redhat.com>
Subject: Re: 08ed4efad6: stress-ng.sigsegv.ops_per_sec -41.9% regression
Date: Thu, 8 Apr 2021 09:22:40 -0700 [thread overview]
Message-ID: <CAHk-=wigPx+MMQMQ-7EA0pq5_5+kMCNV4qFsOss-WwdCSQmb-w@mail.gmail.com> (raw)
In-Reply-To: <20210408083026.GE1696@xsang-OptiPlex-9020>
On Thu, Apr 8, 2021 at 1:32 AM kernel test robot <oliver.sang@intel.com> wrote:
>
> FYI, we noticed a -41.9% regression of stress-ng.sigsegv.ops_per_sec due to commit
> 08ed4efad684 ("[PATCH v10 6/9] Reimplement RLIMIT_SIGPENDING on top of ucounts")
Ouch.
I *think* this test may be testing "send so many signals that it
triggers the signal queue overflow case".
And I *think* that the performance degradation may be due to lots of
unnecessary allocations, because ity looks like that commit changes
__sigqueue_alloc() to do
struct sigqueue *q = kmem_cache_alloc(sigqueue_cachep, flags);
*before* checking the signal limit, and then if the signal limit was
exceeded, it will just be free'd instead.
The old code would check the signal count against RLIMIT_SIGPENDING
*first*, and if there were m ore pending signals then it wouldn't do
anything at all (including not incrementing that expensive atomic
count).
Also, the old code was very careful to only do the "get_user()" for
the *first* signal it added to the queue, and do the "put_user()" for
when removing the last signal. Exactly because those atomics are very
expensive.
The new code just does a lot of these atomics unconditionally.
I dunno. The profile data in there is a bit hard to read, but there's
a lot more cachee misses, and a *lot* of node crossers:
> 5961544 +190.4% 17314361 perf-stat.i.cache-misses
> 22107466 +119.2% 48457656 perf-stat.i.cache-references
> 163292 ą 3% +4582.0% 7645410 perf-stat.i.node-load-misses
> 227388 ą 2% +3708.8% 8660824 perf-stat.i.node-loads
and (probably as a result) average instruction costs have gone up enormously:
> 3.47 +66.8% 5.79 perf-stat.overall.cpi
> 22849 -65.6% 7866 perf-stat.overall.cycles-between-cache-misses
and it does seem to be at least partly about "put_ucounts()":
> 0.00 +4.5 4.46 perf-profile.calltrace.cycles-pp.put_ucounts.__sigqueue_free.get_signal.arch_do_signal_or_restart.exit_to_user_mode_prepare
and a lot of "get_ucounts()".
But it may also be that the new "get sigpending" is just *so* much
more expensive than it used to be.
Linus
WARNING: multiple messages have this Message-ID (diff)
From: Linus Torvalds <torvalds@linux-foundation.org>
To: kernel test robot <oliver.sang@intel.com>
Cc: Alexey Gladkov <gladkov.alexey@gmail.com>,
0day robot <lkp@intel.com>, LKML <linux-kernel@vger.kernel.org>,
lkp@lists.01.org, "Huang, Ying" <ying.huang@intel.com>,
Feng Tang <feng.tang@intel.com>,
zhengjun.xing@intel.com,
Kernel Hardening <kernel-hardening@lists.openwall.com>,
Linux Containers <containers@lists.linux-foundation.org>,
Linux-MM <linux-mm@kvack.org>,
Alexey Gladkov <legion@kernel.org>,
Andrew Morton <akpm@linux-foundation.org>,
Christian Brauner <christian.brauner@ubuntu.com>,
"Eric W . Biederman" <ebiederm@xmission.com>,
Jann Horn <jannh@google.com>, Jens Axboe <axboe@kernel.dk>,
Kees Cook <keescook@chromium.org>,
Oleg Nesterov <oleg@redhat.com>
Subject: Re: 08ed4efad6: stress-ng.sigsegv.ops_per_sec -41.9% regression
Date: Thu, 8 Apr 2021 09:22:40 -0700 [thread overview]
Message-ID: <CAHk-=wigPx+MMQMQ-7EA0pq5_5+kMCNV4qFsOss-WwdCSQmb-w@mail.gmail.com> (raw)
In-Reply-To: <20210408083026.GE1696@xsang-OptiPlex-9020>
On Thu, Apr 8, 2021 at 1:32 AM kernel test robot <oliver.sang@intel.com> wrote:
>
> FYI, we noticed a -41.9% regression of stress-ng.sigsegv.ops_per_sec due to commit
> 08ed4efad684 ("[PATCH v10 6/9] Reimplement RLIMIT_SIGPENDING on top of ucounts")
Ouch.
I *think* this test may be testing "send so many signals that it
triggers the signal queue overflow case".
And I *think* that the performance degradation may be due to lots of
unnecessary allocations, because ity looks like that commit changes
__sigqueue_alloc() to do
struct sigqueue *q = kmem_cache_alloc(sigqueue_cachep, flags);
*before* checking the signal limit, and then if the signal limit was
exceeded, it will just be free'd instead.
The old code would check the signal count against RLIMIT_SIGPENDING
*first*, and if there were m ore pending signals then it wouldn't do
anything at all (including not incrementing that expensive atomic
count).
Also, the old code was very careful to only do the "get_user()" for
the *first* signal it added to the queue, and do the "put_user()" for
when removing the last signal. Exactly because those atomics are very
expensive.
The new code just does a lot of these atomics unconditionally.
I dunno. The profile data in there is a bit hard to read, but there's
a lot more cachee misses, and a *lot* of node crossers:
> 5961544 +190.4% 17314361 perf-stat.i.cache-misses
> 22107466 +119.2% 48457656 perf-stat.i.cache-references
> 163292 ą 3% +4582.0% 7645410 perf-stat.i.node-load-misses
> 227388 ą 2% +3708.8% 8660824 perf-stat.i.node-loads
and (probably as a result) average instruction costs have gone up enormously:
> 3.47 +66.8% 5.79 perf-stat.overall.cpi
> 22849 -65.6% 7866 perf-stat.overall.cycles-between-cache-misses
and it does seem to be at least partly about "put_ucounts()":
> 0.00 +4.5 4.46 perf-profile.calltrace.cycles-pp.put_ucounts.__sigqueue_free.get_signal.arch_do_signal_or_restart.exit_to_user_mode_prepare
and a lot of "get_ucounts()".
But it may also be that the new "get sigpending" is just *so* much
more expensive than it used to be.
Linus
WARNING: multiple messages have this Message-ID (diff)
From: Linus Torvalds <torvalds@linux-foundation.org>
To: lkp@lists.01.org
Subject: Re: 08ed4efad6: stress-ng.sigsegv.ops_per_sec -41.9% regression
Date: Thu, 08 Apr 2021 09:22:40 -0700 [thread overview]
Message-ID: <CAHk-=wigPx+MMQMQ-7EA0pq5_5+kMCNV4qFsOss-WwdCSQmb-w@mail.gmail.com> (raw)
In-Reply-To: <20210408083026.GE1696@xsang-OptiPlex-9020>
[-- Attachment #1: Type: text/plain, Size: 2324 bytes --]
On Thu, Apr 8, 2021 at 1:32 AM kernel test robot <oliver.sang@intel.com> wrote:
>
> FYI, we noticed a -41.9% regression of stress-ng.sigsegv.ops_per_sec due to commit
> 08ed4efad684 ("[PATCH v10 6/9] Reimplement RLIMIT_SIGPENDING on top of ucounts")
Ouch.
I *think* this test may be testing "send so many signals that it
triggers the signal queue overflow case".
And I *think* that the performance degradation may be due to lots of
unnecessary allocations, because ity looks like that commit changes
__sigqueue_alloc() to do
struct sigqueue *q = kmem_cache_alloc(sigqueue_cachep, flags);
*before* checking the signal limit, and then if the signal limit was
exceeded, it will just be free'd instead.
The old code would check the signal count against RLIMIT_SIGPENDING
*first*, and if there were m ore pending signals then it wouldn't do
anything at all (including not incrementing that expensive atomic
count).
Also, the old code was very careful to only do the "get_user()" for
the *first* signal it added to the queue, and do the "put_user()" for
when removing the last signal. Exactly because those atomics are very
expensive.
The new code just does a lot of these atomics unconditionally.
I dunno. The profile data in there is a bit hard to read, but there's
a lot more cachee misses, and a *lot* of node crossers:
> 5961544 +190.4% 17314361 perf-stat.i.cache-misses
> 22107466 +119.2% 48457656 perf-stat.i.cache-references
> 163292 ą 3% +4582.0% 7645410 perf-stat.i.node-load-misses
> 227388 ą 2% +3708.8% 8660824 perf-stat.i.node-loads
and (probably as a result) average instruction costs have gone up enormously:
> 3.47 +66.8% 5.79 perf-stat.overall.cpi
> 22849 -65.6% 7866 perf-stat.overall.cycles-between-cache-misses
and it does seem to be at least partly about "put_ucounts()":
> 0.00 +4.5 4.46 perf-profile.calltrace.cycles-pp.put_ucounts.__sigqueue_free.get_signal.arch_do_signal_or_restart.exit_to_user_mode_prepare
and a lot of "get_ucounts()".
But it may also be that the new "get sigpending" is just *so* much
more expensive than it used to be.
Linus
next prev parent reply other threads:[~2021-04-08 16:23 UTC|newest]
Thread overview: 54+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-04-07 17:08 [PATCH v10 0/9] Count rlimits in each user namespace Alexey Gladkov
2021-04-07 17:08 ` Alexey Gladkov
2021-04-07 17:08 ` [PATCH v10 1/9] Increase size of ucounts to atomic_long_t Alexey Gladkov
2021-04-07 17:08 ` Alexey Gladkov
2021-04-07 17:08 ` [PATCH v10 2/9] Add a reference to ucounts for each cred Alexey Gladkov
2021-04-07 17:08 ` Alexey Gladkov
2021-04-07 17:08 ` [PATCH v10 3/9] Use atomic_t for ucounts reference counting Alexey Gladkov
2021-04-07 17:08 ` Alexey Gladkov
2021-04-07 17:08 ` [PATCH v10 4/9] Reimplement RLIMIT_NPROC on top of ucounts Alexey Gladkov
2021-04-07 17:08 ` Alexey Gladkov
2021-04-07 17:08 ` [PATCH v10 5/9] Reimplement RLIMIT_MSGQUEUE " Alexey Gladkov
2021-04-07 17:08 ` Alexey Gladkov
2021-04-07 17:08 ` [PATCH v10 6/9] Reimplement RLIMIT_SIGPENDING " Alexey Gladkov
2021-04-07 17:08 ` Alexey Gladkov
2021-04-08 8:30 ` 08ed4efad6: stress-ng.sigsegv.ops_per_sec -41.9% regression kernel test robot
2021-04-08 8:30 ` kernel test robot
2021-04-08 8:30 ` kernel test robot
2021-04-08 16:22 ` Linus Torvalds [this message]
2021-04-08 16:22 ` Linus Torvalds
2021-04-08 16:22 ` Linus Torvalds
2021-04-08 16:22 ` Linus Torvalds
2021-04-08 16:44 ` Alexey Gladkov
2021-04-08 16:44 ` Alexey Gladkov
2021-04-08 16:44 ` Alexey Gladkov
2021-04-08 18:44 ` Eric W. Biederman
2021-04-08 18:44 ` Eric W. Biederman
2021-04-08 18:44 ` Eric W. Biederman
2021-04-08 18:44 ` Eric W. Biederman
2021-04-16 11:33 ` Alexey Gladkov
2021-04-16 11:33 ` Alexey Gladkov
2021-04-16 11:33 ` Alexey Gladkov
2021-04-23 2:47 ` Oliver Sang
2021-04-23 2:47 ` Oliver Sang
2021-04-23 2:47 ` Oliver Sang
2021-04-23 7:44 ` Alexey Gladkov
2021-04-23 7:44 ` Alexey Gladkov
2021-04-23 7:44 ` Alexey Gladkov
2021-04-28 14:36 ` Oliver Sang
2021-04-28 14:36 ` Oliver Sang
2021-04-28 14:36 ` Oliver Sang
2021-04-28 15:09 ` Alexey Gladkov
2021-04-28 15:09 ` Alexey Gladkov
2021-04-28 15:09 ` Alexey Gladkov
2021-05-07 7:14 ` Oliver Sang
2021-05-07 7:14 ` Oliver Sang
2021-04-07 17:08 ` [PATCH v10 7/9] Reimplement RLIMIT_MEMLOCK on top of ucounts Alexey Gladkov
2021-04-07 17:08 ` Alexey Gladkov
2021-04-07 21:37 ` kernel test robot
2021-04-07 21:37 ` kernel test robot
2021-04-07 21:37 ` kernel test robot
2021-04-07 17:08 ` [PATCH v10 8/9] kselftests: Add test to check for rlimit changes in different user namespaces Alexey Gladkov
2021-04-07 17:08 ` Alexey Gladkov
2021-04-07 17:08 ` [PATCH v10 9/9] ucounts: Set ucount_max to the largest positive value the type can hold Alexey Gladkov
2021-04-07 17:08 ` Alexey Gladkov
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='CAHk-=wigPx+MMQMQ-7EA0pq5_5+kMCNV4qFsOss-WwdCSQmb-w@mail.gmail.com' \
--to=torvalds@linux-foundation.org \
--cc=akpm@linux-foundation.org \
--cc=axboe@kernel.dk \
--cc=containers@lists.linux-foundation.org \
--cc=ebiederm@xmission.com \
--cc=feng.tang@intel.com \
--cc=jannh@google.com \
--cc=keescook@chromium.org \
--cc=kernel-hardening@lists.openwall.com \
--cc=legion@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=lkp@intel.com \
--cc=lkp@lists.01.org \
--cc=oleg@redhat.com \
--cc=oliver.sang@intel.com \
--cc=ying.huang@intel.com \
--cc=zhengjun.xing@intel.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.