From: Linus Torvalds <torvalds@linux-foundation.org>
To: Feng Tang <feng.tang@intel.com>
Cc: Jiri Olsa <jolsa@redhat.com>,
Peter Zijlstra <peterz@infradead.org>,
kernel test robot <rong.a.chen@intel.com>,
Ingo Molnar <mingo@kernel.org>,
Vince Weaver <vincent.weaver@maine.edu>,
Jiri Olsa <jolsa@kernel.org>,
Alexander Shishkin <alexander.shishkin@linux.intel.com>,
Arnaldo Carvalho de Melo <acme@kernel.org>,
Arnaldo Carvalho de Melo <acme@redhat.com>,
"Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com>,
Ravi Bangoria <ravi.bangoria@linux.ibm.com>,
Stephane Eranian <eranian@google.com>,
Thomas Gleixner <tglx@linutronix.de>,
LKML <linux-kernel@vger.kernel.org>,
lkp@lists.01.org, andi.kleen@intel.com, "Huang,
Ying" <ying.huang@intel.com>
Subject: Re: [LKP] Re: [perf/x86] 81ec3f3c4c: will-it-scale.per_process_ops -5.5% regression
Date: Mon, 24 Feb 2020 12:09:04 -0800 [thread overview]
Message-ID: <CAHk-=wifdJHrfnmwwzPpH-0X6SaZxtdmRWpSNwf8xsXD2iE4dA@mail.gmail.com> (raw)
In-Reply-To: <CAHk-=wjkSb1OkiCSn_fzf2v7A=K0bNsUEeQa+06XMhTO+oQUaA@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1104 bytes --]
On Mon, Feb 24, 2020 at 11:24 AM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> I don't know. This does not seem to be a particularly serious load.
> But it does feel like it should be possible to combine the two atomic
> accesses into one, where you don't need to do the refcount thing
> except for the case where sigcount goes from zero to non-zero (and
> back to zero again).
Ok, that looks just as simple as I thought it would be.
TOTALLY UNTESTED patch attached. It may be completely buggy garbage,
but it _looks_ trivial enough. Just make the rule be that "if we have
any user->sigpending cases, we'll get a ref to the user for the first
one, and drop it only when getting rid of the last one".
So it might be worth testing this. But again: I have NOT done so.
There might be some silly reason why this doesn't work because I just
did the tests wrong or missed some case.
Or there might be some subtle reason why it doesn't work because I
didn't think this through properly.
But it _looks_ obvious and simple enough. And it compiles for me. So
maybe it works.
Linus
[-- Attachment #2: patch.diff --]
[-- Type: text/x-patch, Size: 1434 bytes --]
kernel/signal.c | 17 +++++++++++------
1 file changed, 11 insertions(+), 6 deletions(-)
diff --git a/kernel/signal.c b/kernel/signal.c
index 9ad8dea93dbb..00addaa8319f 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -417,10 +417,15 @@ __sigqueue_alloc(int sig, struct task_struct *t, gfp_t flags, int override_rlimi
/*
* Protect access to @t credentials. This can go away when all
* callers hold rcu read lock.
+ *
+ * NOTE! A pending signal will hold on to the user refcount,
+ * and we get/put the refcount only when the sigpending count
+ * changes from/to zero.
*/
rcu_read_lock();
- user = get_uid(__task_cred(t)->user);
- atomic_inc(&user->sigpending);
+ user = __task_cred(t)->user;
+ if (atomic_inc_return(&user->sigpending) == 1)
+ get_uid(user);
rcu_read_unlock();
if (override_rlimit ||
@@ -432,8 +437,8 @@ __sigqueue_alloc(int sig, struct task_struct *t, gfp_t flags, int override_rlimi
}
if (unlikely(q == NULL)) {
- atomic_dec(&user->sigpending);
- free_uid(user);
+ if (atomic_dec_and_test(&user->sigpending))
+ free_uid(user);
} else {
INIT_LIST_HEAD(&q->list);
q->flags = 0;
@@ -447,8 +452,8 @@ static void __sigqueue_free(struct sigqueue *q)
{
if (q->flags & SIGQUEUE_PREALLOC)
return;
- atomic_dec(&q->user->sigpending);
- free_uid(q->user);
+ if (atomic_dec_and_test(&q->user->sigpending))
+ free_uid(q->user);
kmem_cache_free(sigqueue_cachep, q);
}
next prev parent reply other threads:[~2020-02-24 20:17 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-02-05 12:32 [perf/x86] 81ec3f3c4c: will-it-scale.per_process_ops -5.5% regression kernel test robot
2020-02-05 12:58 ` Peter Zijlstra
2020-02-06 3:04 ` [LKP] " Li, Philip
2020-02-21 8:03 ` Feng Tang
2020-02-21 10:58 ` Peter Zijlstra
2020-02-21 13:20 ` Jiri Olsa
2020-02-23 14:11 ` Feng Tang
2020-02-23 17:37 ` Linus Torvalds
2020-02-24 0:33 ` Feng Tang
2020-02-24 1:06 ` Linus Torvalds
2020-02-24 1:58 ` Huang, Ying
2020-02-24 2:19 ` Feng Tang
2020-02-24 13:20 ` Feng Tang
2020-02-24 19:24 ` Linus Torvalds
2020-02-24 19:42 ` Kleen, Andi
2020-02-24 20:09 ` Linus Torvalds [this message]
2020-02-24 20:47 ` Linus Torvalds
2020-02-24 21:20 ` Eric W. Biederman
2020-02-24 21:43 ` Linus Torvalds
2020-02-24 21:59 ` Eric W. Biederman
2020-02-24 22:12 ` Linus Torvalds
2020-02-25 2:57 ` Feng Tang
2020-02-25 3:15 ` Linus Torvalds
2020-02-25 4:53 ` Feng Tang
2020-02-23 19:36 ` Jiri Olsa
2020-02-21 18:05 ` Kleen, Andi
2020-02-22 12:43 ` Feng Tang
2020-02-22 17:08 ` Kleen, Andi
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-=wifdJHrfnmwwzPpH-0X6SaZxtdmRWpSNwf8xsXD2iE4dA@mail.gmail.com' \
--to=torvalds@linux-foundation.org \
--cc=acme@kernel.org \
--cc=acme@redhat.com \
--cc=alexander.shishkin@linux.intel.com \
--cc=andi.kleen@intel.com \
--cc=eranian@google.com \
--cc=feng.tang@intel.com \
--cc=jolsa@kernel.org \
--cc=jolsa@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=lkp@lists.01.org \
--cc=mingo@kernel.org \
--cc=naveen.n.rao@linux.vnet.ibm.com \
--cc=peterz@infradead.org \
--cc=ravi.bangoria@linux.ibm.com \
--cc=rong.a.chen@intel.com \
--cc=tglx@linutronix.de \
--cc=vincent.weaver@maine.edu \
--cc=ying.huang@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 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).