From: Thomas Gleixner <tglx@linutronix.de>
To: "Eric W. Biederman" <ebiederm@xmission.com>
Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>,
linux-kernel@vger.kernel.org, Ingo Molnar <mingo@redhat.com>,
Peter Zijlstra <peterz@infradead.org>,
Juri Lelli <juri.lelli@redhat.com>,
Vincent Guittot <vincent.guittot@linaro.org>,
Dietmar Eggemann <dietmar.eggemann@arm.com>,
Steven Rostedt <rostedt@goodmis.org>,
Ben Segall <bsegall@google.com>, Mel Gorman <mgorman@suse.de>,
Daniel Bristot de Oliveira <bristot@redhat.com>,
Oleg Nesterov <oleg@redhat.com>,
Matt Fleming <matt@codeblueprint.co.uk>
Subject: Re: [PATCH] signal: Allow RT tasks to cache one sigqueue struct
Date: Wed, 10 Mar 2021 19:54:18 +0100 [thread overview]
Message-ID: <87sg524z6t.fsf@nanos.tec.linutronix.de> (raw)
In-Reply-To: <87lfb263h2.fsf@nanos.tec.linutronix.de>
On Thu, Mar 04 2021 at 21:58, Thomas Gleixner wrote:
> On Thu, Mar 04 2021 at 13:04, Eric W. Biederman wrote:
>> Thomas Gleixner <tglx@linutronix.de> writes:
>>>
>>> We could of course do the caching unconditionally for all tasks.
>>
>> Is there any advantage to only doing this for realtime tasks?
>
> It was mostly to avoid tons of cached entries hanging around all over
> the place. So I limited it to the case which the RT users deeply cared
> about. Also related to the accounting question below.
>
>> If not it probably makes sense to do the caching for all tasks.
>>
>> I am wondering if we want to count the cached sigqueue structure to the
>> users rt signal rlimit?
>
> That makes some sense, but that's a user visible change as a single
> signal will up the count for a tasks lifetime while today it is removed
> from accounting again once the signal is delivered. So that needs some
> thought.
Thought more about it. To make this accounting useful we'd need:
- a seperate user::sigqueue_cached counter
- a seperate RLIMIT_SIGQUEUE_CACHED
Then you need to think about the defaults. Any signal heavy application
will want this enabled and obviously automagically.
Also there is an argument not to have this due to possible pointless
memory consumption.
But what are we talking about? 80 bytes worth of memory per task in the
worst case. Which is compared to the rest of a task's memory consumption
just noise.
Looking at some statistics from a devel system there are less than 10
items cached when the machine is fully idle after boot. During a kernel
compile the cache utilization goes up to ~150 at max (make -j128 and 64
CPUs). What's interesting is the allocation statistics after boot and
full kernel compile:
from slab: 23996
from task cache: 52223
A typical pattern there is:
<ls>-58490 [010] d..2 7765.664198: __sigqueue_alloc: 58488 from slab ffff8881132df460 10
<ls>-58488 [002] d..1 7765.664294: __sigqueue_free.part.35: cache ffff8881132df460 10
<ls>-58488 [002] d..2 7765.665146: __sigqueue_alloc: 1149 from cache ffff8881103dc550 10
bash-1149 [000] d..2 7765.665220: exit_task_sighand: free ffff8881132df460 8 9
bash-1149 [000] d..1 7765.665662: __sigqueue_free.part.35: cache ffff8881103dc550 9
58488 grabs the sigqueue from bash's task cache and bash sticks it back
in. Lather, rinse and repeat.
IMO, not bothering with an extra counter and rlimit plus the required
atomic operations is just fine and having this for all tasks
unconditionally looks like a clear win.
I'll post an updated version of this soonish.
Thanks,
tglx
next prev parent reply other threads:[~2021-03-10 18:55 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-03-03 14:20 [PATCH] signal: Allow RT tasks to cache one sigqueue struct Sebastian Andrzej Siewior
2021-03-03 15:37 ` Oleg Nesterov
2021-03-04 21:10 ` Thomas Gleixner
2021-03-05 10:57 ` Oleg Nesterov
2021-03-04 21:14 ` Thomas Gleixner
2021-03-03 22:09 ` Eric W. Biederman
2021-03-04 8:11 ` Sebastian Andrzej Siewior
2021-03-04 15:02 ` Thomas Gleixner
2021-03-04 19:04 ` Eric W. Biederman
2021-03-04 20:58 ` Thomas Gleixner
2021-03-10 18:54 ` Thomas Gleixner [this message]
2021-03-10 21:57 ` Eric W. Biederman
2021-03-10 23:56 ` Thomas Gleixner
2021-03-11 12:45 ` Thomas Gleixner
2021-03-11 14:20 ` Thomas Gleixner
2021-03-11 16:32 ` Eric W. Biederman
2021-03-04 19:01 ` Eric W. Biederman
[not found] <draft-874khk5yed.fsf@nanos.tec.linutronix.de>
2021-03-10 8:57 ` Thomas Gleixner
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=87sg524z6t.fsf@nanos.tec.linutronix.de \
--to=tglx@linutronix.de \
--cc=bigeasy@linutronix.de \
--cc=bristot@redhat.com \
--cc=bsegall@google.com \
--cc=dietmar.eggemann@arm.com \
--cc=ebiederm@xmission.com \
--cc=juri.lelli@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=matt@codeblueprint.co.uk \
--cc=mgorman@suse.de \
--cc=mingo@redhat.com \
--cc=oleg@redhat.com \
--cc=peterz@infradead.org \
--cc=rostedt@goodmis.org \
--cc=vincent.guittot@linaro.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