From: Mateusz Guzik <mjguzik@gmail.com>
To: John Johansen <john.johansen@canonical.com>
Cc: Vinicius Costa Gomes <vinicius.gomes@intel.com>,
linux-security-module@vger.kernel.org, apparmor@lists.ubuntu.com,
linux-kernel@vger.kernel.org
Subject: Re: [apparmor] use per-cpu refcounts for apparmor labels?
Date: Tue, 26 Sep 2023 08:38:57 +0200 [thread overview]
Message-ID: <20230926063857.h3afce5hagnlkoob@f> (raw)
In-Reply-To: <c6379a39-42f2-b3f9-c835-bbebe516ba3a@canonical.com>
On Mon, Sep 25, 2023 at 11:21:26PM -0700, John Johansen wrote:
> On 9/25/23 16:49, Vinicius Costa Gomes wrote:
> > Hi Mateusz,
> >
> > Mateusz Guzik <mjguzik@gmail.com> writes:
> >
> > > I'm sanity-checking perf in various microbenchmarks and I found
> > > apparmor to be the main bottleneck in some of them.
> > >
> > > For example: will-it-scale open1_processes -t 16, top of the profile:
> > > 20.17% [kernel] [k] apparmor_file_alloc_security
> > > 20.08% [kernel] [k] apparmor_file_open
> > > 20.05% [kernel] [k] apparmor_file_free_security
> > > 18.39% [kernel] [k] apparmor_current_getsecid_subj
> > > [snip]
> > >
> > > This serializes on refing/unrefing apparmor objs, sounds like a great
> > > candidate for per-cpu refcounting instead (I'm assuming they are
> > > expected to be long-lived).
> > >
> > > I would hack it up myself, but I failed to find a clear spot to switch
> > > back from per-cpu to centalized operation and don't want to put
> > > serious effort into it.
> > >
> > > Can you sort this out?
> >
>
> I will add looking into it on the todo list. Its going to have to come
> after some other major cleanups land, and I am not sure we can make
> the semantic work well for some of these. For other we might get away
> with switching to a critical section like Vinicius's patch has done
> for apparmor_current_getsecid_subj.
>
Is there an eta?
I looked at dodging ref round trips myself, but then found that ref
manipulation in apparmor_file_alloc_security and the free counterpart
cannot be avoided. Thus per-cpu refs instead.
Perhaps making the label as stale would be a good enough switching
point? Is it *guaranteed* to get labelled as stale before it gets freed?
btw, __aa_proxy_redirect open-codes setting the flag.
> > I was looking at this same workload, and proposed a patch[1] some time
> > ago, see if it helps:
> >
> > https://lists.ubuntu.com/archives/apparmor/2023-August/012914.html
> >
> > But my idea was different, in many cases, we are looking at the label
> > associated with the current task, and there's no need to take the
> > refcount.
> >
>
> yes, and thanks for that.
>
next prev parent reply other threads:[~2023-09-26 6:39 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-09-22 21:19 use per-cpu refcounts for apparmor labels? Mateusz Guzik
2023-09-25 23:49 ` [apparmor] " Vinicius Costa Gomes
2023-09-26 6:21 ` John Johansen
2023-09-26 6:38 ` Mateusz Guzik [this message]
2023-09-26 12:48 ` John Johansen
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=20230926063857.h3afce5hagnlkoob@f \
--to=mjguzik@gmail.com \
--cc=apparmor@lists.ubuntu.com \
--cc=john.johansen@canonical.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-security-module@vger.kernel.org \
--cc=vinicius.gomes@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