On Wed, Jan 13, 2021 at 6:03 PM Jakub Kicinski <k...@kernel.org> wrote: > > On Wed, 13 Jan 2021 05:46:05 +0100 Eric Dumazet wrote: > > On Wed, Jan 13, 2021 at 2:02 AM Jakub Kicinski <k...@kernel.org> wrote: > > > > > > On Tue, 12 Jan 2021 13:23:16 +0100 Eric Dumazet wrote: > > > > On Tue, Jan 12, 2021 at 12:08 PM Alexander Lobakin <aloba...@pm.me> > > > > wrote: > > > > > > > > > > From: Edward Cree <ecree.xil...@gmail.com> > > > > > Date: Tue, 12 Jan 2021 09:54:04 +0000 > > > > > > > > > > > Without wishing to weigh in on whether this caching is a good > > > > > > idea... > > > > > > > > > > Well, we already have a cache to bulk flush "consumed" skbs, although > > > > > kmem_cache_free() is generally lighter than kmem_cache_alloc(), and > > > > > a page frag cache to allocate skb->head that is also bulking the > > > > > operations, since it contains a (compound) page with the size of > > > > > min(SZ_32K, PAGE_SIZE). > > > > > If they wouldn't give any visible boosts, I think they wouldn't hit > > > > > mainline. > > > > > > > > > > > Wouldn't it be simpler, rather than having two separate "alloc" and > > > > > > "flush" > > > > > > caches, to have a single larger cache, such that whenever it > > > > > > becomes full > > > > > > we bulk flush the top half, and when it's empty we bulk alloc the > > > > > > bottom > > > > > > half? That should mean fewer branches, fewer instructions etc. > > > > > > than > > > > > > having to decide which cache to act upon every time. > > > > > > > > > > I though about a unified cache, but couldn't decide whether to flush > > > > > or to allocate heads and how much to process. Your suggestion answers > > > > > these questions and generally seems great. I'll try that one, thanks! > > > > > > > > The thing is : kmalloc() is supposed to have batches already, and nice > > > > per-cpu caches. > > > > > > > > This looks like an mm issue, are we sure we want to get over it ? > > > > > > > > I would like a full analysis of why SLAB/SLUB does not work well for > > > > your test workload. > > > > > > +1, it does feel like we're getting into mm territory > > > > I read the existing code, and with Edward Cree idea of reusing the > > existing cache (storage of pointers), > > ths now all makes sense, since there will be not much added code (and > > new storage of 64 pointers) > > > > The remaining issue is to make sure KASAN will still work, we need > > this to detect old and new bugs. > > IDK much about MM, but we already have a kmem_cache for skbs and now > we're building a cache on top of a cache. Shouldn't MM take care of > providing a per-CPU BH-only lockless cache?
I think part of the improvement comes from bulk operations, which are provided by mm layer. I also note Alexander made no provision for NUMA awareness. Probably reusing skb located on a remote node will not be ideal.
