Re: [PATCH] apparmor: try to avoid refing the label in apparmor_file_open

From: John Johansen
Date: Thu Jun 20 2024 - 13:09:16 EST


On 6/20/24 09:41, Mateusz Guzik wrote:
On Thu, Jun 20, 2024 at 09:26:00AM -0700, John Johansen wrote:
On 6/20/24 06:15, Mateusz Guzik wrote:
It can be done in the common case.
A 24-way open1_processes from will-it-scale (separate file open) shows:
29.37% [kernel] [k] apparmor_file_open
26.84% [kernel] [k] apparmor_file_alloc_security
26.62% [kernel] [k] apparmor_file_free_security
1.32% [kernel] [k] clear_bhb_loop

apparmor_file_open is eliminated from the profile with the patch.

Throughput (ops/s):
before: 6092196
after: 8309726 (+36%)

Signed-off-by: Mateusz Guzik <mjguzik@xxxxxxxxx>
can you cleanup the commit message and I will pull this in


First of all thanks for a timely review.

I thought that's a decent commit message though. ;)

Would something like this work:
<cm>
apparmor: try to avoid refing the label in apparmor_file_open

In the common case it can be avoided, which in turn reduces the
performance impact apparmor on parallel open() invocations.

When benchmarking on 24-core vm using will-it-scale's open1_process
("Separate file open"), the results are (ops/s):
before: 6092196
after: 8309726 (+36%)
</cm>

If this is fine I'll send a v2.

it will do, largely, I was just looking for something that explains
a little more than. "It can be done in the common case"


If you are looking for something fundamentally different I would say it
will be the fastest if you write your own commit message while borrowing
the numbers and denoting all the wording is yours. I'm trying to reduce
back and forth over email here.

Am I missing something which makes the approach below not work to begin
with?

no this will work in the short term. Long term there is work that will
break this. Both replacing unconfined and the object delegation work
will cause a performance regression as I am not sure we will be able
to conditionally get the label but that is something for those patch
series to work out. My biggest concern being people objecting to necessary
changes that regress performance, if it can't be worked out, but
that really isn't a reason to stop this now.


hrm. I was looking at going a step further, now I'm going to have to
poke around.