Messages in this thread |  | | From | Christian Schoenebeck <> | Subject | Re: [PATCH 3/3] 9p: Add mempools for RPCs | Date | Mon, 04 Jul 2022 15:56:55 +0200 |
| |
On Montag, 4. Juli 2022 15:06:00 CEST Dominique Martinet wrote: > Christian Schoenebeck wrote on Mon, Jul 04, 2022 at 01:12:51PM +0200: > > On Montag, 4. Juli 2022 05:38:46 CEST Dominique Martinet wrote: [...] > > However that's exactly what I was going to address with my already posted > > patches (relevant patches regarding this issue here being 9..12): > > https://lore.kernel.org/all/cover.1640870037.git.linux_oss@crudebyte.com/ > > And in the cover letter (section "STILL TODO" ... "3.") I was suggesting > > to > > subsequently subdivide kmem_cache_alloc() into e.g. 4 allocation size > > categories? Because that's what my already posted patches do anyway. > > Yes, I hinted at that by asking if it'd be worth a second mempool for 8k > buffers, but I'm not sure it is -- I think kmalloc will just be as fast > for these in practice? That would need checking. > > But I also took a fresh look just now and didn't remember we had so many > different cases there, and that msize is no longer really used -- now > this is just a gut feeling, but I think we'd benefit from rounding up to > some pooled sizes e.g. I assume it'll be faster to allocate 1MB from the > cache three times than try to get 500k/600k/1MB from kmalloc. > > That's a lot of assuming though and this is going to need checking...
Yeah, that's the reason why omitted this aspect so far, because I also thought it deserves actual benchmarking how much cache granularity really makes sense, instead of blindly subdividing them into random separate cache size categories.
> > Hoo, Dominique, please hold your horses. I currently can't keep up with > > reviewing and testing all pending 9p patches right now. > > > > Personally I would hold these patches back for now. They would make sense > > on current situation on master, because ATM basically all 9p requests > > simply allocate exactly 'msize' for any 9p request. > > So I think they're orthogonal really: > what mempool does is that it'll reserve the minimum amount of memory > required for x allocations (whatever min is set during init, so here 4 > parallel RPCs) -- if normal allocation goes through it'll go through > normal slab allocation first, and if that fails we'll get a buffer from > the pool instead, and if there is none left it'll wait for a previous > request to be freed up possibly throttling the number of parallel > requests down but never failing like we currently do.
Understood.
> With this the worst that can happen is some RPCs will be delayed, and > the patch already falls back to allocating a msize buffer from pool even > if less is requrested if that kmalloc failed, so I think it should work > out ok as a first iteration. > > (I appreciate the need for testing, but this feels much less risky than > the iovec series we've had recently... Famous last words?)
Got it, consider my famous last words dropped. ;-)
> For later iterations we might want to optimize with multiple sizes of > pools and pick the closest majoring size or something, but I think > that'll be tricky to get right so I'd rather not rush such an > optimization. > > > How about I address the already discussed issues and post a v5 of those > > patches this week and then we can continue from there? > > I would have been happy to rebase your patches 9..12 on top of Kent's > this weekend but if you want to refresh them this week we can continue > from there, sure.
I'll rebase them on master and address what we discussed so far. Then we'll see.
Best regards, Christian Schoenebeck
|  |