Re: [PATCH 1/2] audit: Vary struct audit_entry alignment

[Ankur Arora <ankur.a.arora@oracle.com>]

Paul Moore <paul@paul-moore.com> writes:

> On Sat, Dec 16, 2023 at 11:25 AM Haakon Bugge <haakon.bugge@oracle.com> wrote:
>> > On 14 Dec 2023, at 00:54, Paul Moore <paul@paul-moore.com> wrote:
>> >
>> > Two things:
>> >
>> > 1. If we are going to create a kmem_cache pool we shouldn't create it
>> > here, it should be in its own audit_filter_init() function which is
>> > called from audit_init().
>>
>> Understood. Will fix.
>>
>> > 2. I'm not sure it makes a lot of sense to create a kmem_cache pool
>> > for audit filter entries, especially given the modest performance
>> > gains.  Is there not some way to request cacheline alignment with
>> > kmalloc() or similar?
>>
>> The problem with today's kzmalloc() is lack of entropy on the lower order address bits, because the audit filter entries are aligned on a 512B boundary. IOW, they are too much aligned. The increased entropy is exactly what we get from using a kmem_cache which yields more L1D cache sets to be used.
>>
>> Although the performance gain is modest, the reduction in L1D cache misses is substantial and that will improve performance on most archs that employ a virtually indexed L1D cache. And, this commit acts as a prerequisite to avoid high variability in performance gain from the second commit in this series.
>
> My hesitation of using a kmem_cache object here remains, given the
> relatively limited and static filter rule configuration I would rather
> use a k*malloc() based approach.

AFAICT, kmalloc() etc only allows fixed alignment. From
Documentation/core-api/memory-allocation.rst:

  The address of a chunk allocated with `kmalloc` is aligned to at least
  ARCH_KMALLOC_MINALIGN bytes.  For sizes which are a power of two, the
  alignment is also guaranteed to be at least the respective size.

I had sent out a patch a while ago reducing the cost of the same
alignment issue. For me the most pernicious part was the fact that
syscall latency was good or poor based on how boot time factors
affected audit allocations.

So while I do agree with your hesitation on kmem_cache not being quite
the right interface for what are static allocations, I think it might
be worth it given the cost.

Thanks
--
ankur