On Tue, Dec 19, 2023 at 4:07 PM Ankur Arora <[email protected]> wrote: > 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.
Once again, I appreciate the time and effort that you have put into this patchset, but it is not something I want to accept at this point in time. -- paul-moore.com
