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

[Mateusz Guzik]

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 at gmail.com>
> 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.

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.