Paul Moore <[email protected]> writes:
> On Sat, Dec 16, 2023 at 11:25 AM Haakon Bugge <[email protected]> wrote: >> > On 14 Dec 2023, at 00:54, Paul Moore <[email protected]> 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
