On 11/7/19 7:41 AM, Ondrej Mosnacek wrote:
On Wed, Nov 6, 2019 at 8:48 PM Stephen Smalley <sds@xxxxxxxxxxxxx> wrote:
On 11/6/19 2:22 PM, Stephen Smalley wrote:
On 11/6/19 11:11 AM, Stephen Smalley wrote:
On 11/6/19 3:26 AM, Ondrej Mosnacek wrote:
Translating a context struct to string can be quite slow, especially if
the context has a lot of category bits set. This can cause quite
noticeable performance impact in situations where the translation needs
to be done repeatedly. A common example is a UNIX datagram socket with
the SO_PASSSEC option enabled, which is used e.g. by systemd-journald
when receiving log messages via datagram socket. This scenario can be
reproduced with:
cat /dev/urandom | base64 | logger &
timeout 30s perf record -p $(pidof systemd-journald) -a -g
kill %1
perf report -g none --pretty raw | grep security_secid_to_secctx
Before the caching introduced by this patch, computing the context
string (security_secid_to_secctx() function) takes up ~65% of
systemd-journald's CPU time (assuming a context with 1024 categories
set and Fedora x86_64 release kernel configs). After this patch
(assuming near-perfect cache hit ratio) this overhead is reduced to just
~2%.
This patch addresses the issue by caching a certain number (compile-time
configurable) of recently used context strings to speed up repeated
translations of the same context, while using only a small amount of
memory.
The cache is integrated into the existing sidtab table by adding a field
to each entry, which when not NULL contains an RCU-protected pointer to
a cache entry containing the cached string. The cache entries are kept
in a linked list sorted according to how recently they were used. On a
cache miss when the cache is full, the least recently used entry is
removed to make space for the new entry.
The patch migrates security_sid_to_context_core() to use the cache (also
a few other functions where it was possible without too much fuss, but
these mostly use the translation for logging in case of error, which is
rare).
Link: https://bugzilla.redhat.com/show_bug.cgi?id=1733259
Cc: Michal Sekletar <msekleta@xxxxxxxxxx>
Signed-off-by: Ondrej Mosnacek <omosnace@xxxxxxxxxx>
This looks good to me and didn't trigger any issues during testing.
Might want to run it by the RCU wizards, e.g. Paul McKenney, to
confirm that it is correct and optimal usage of RCU. I didn't see
anywhere near the same degree of performance improvement in running
your reproducer above but I also have all the kernel debugging options
enabled so perhaps those are creating noise or perhaps the reproducer
doesn't yield stable results.
Rebuilding with the stock Fedora x86_64 kernel config was closer to your
results albeit still different, ~35% before and ~2% after.
Ah, I can reproduce your ~65% result on the 5.3-based Fedora kernel, but
not with mainline 5.4.0-rc1. Only SELinux change between those two that
seems potentially relevant is your "selinux: avoid atomic_t usage in
sidtab" patch.
Hm... did you use the stock Fedora kernel RPM as the baseline? If so,
this could be because on stable Fedora releases the kernel package is
built with release config and kernel-debug with debug config, while on
Rawhide there is only one kernel package, which is built with debug
config. Under debug Fedora config the numbers are completely different
due to the overhead of various debug checks. I don't remember the
"after" value that I got when testing the patched Rawhide kernel with
the default debug config, but on stock Rawhide I got 43%, and on
Rawhide kernel rebuild with release config I got 65% again.
I first built selinux/next (based on 5.4-rc1) before and after your
patch using a config with many debug options enabled (including KASAN),
which yielded wildly different percentages both before and after - don't
remember the exact values (but still improved by your patch).
Then I built the same sources using the stock Fedora 31 release kernel
config and got the 35% versus 2% figures, likewise an improvement.
Then I ran your reproducer just on the stock Fedora 31 release kernel,
which happened to be 5.3-based, and got the 65% versus 2% figures that
matched your results. So I'm not sure what accounts for the difference
in the before results between the stock Fedora 31 release kernel and
selinux/next using the same config. Could be a 5.3 vs 5.4-rc1 change, a
change in selinux/next on top of 5.4-rc1, or something different in the
build toolchain/environment used for the stock Fedora kernel versus my
build host (which was running Fedora 31).
Regardless, I do see a significant improvement in all cases from the
patch, only the degree of improvement differs. So I'm fine with it.
Might want to get a 2nd opinion from Paul McKenney all the same on the
RCU bits.
