[lkml]   [2022]   [Jul]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
SubjectRe: [PATCH 3/3] 9p: Add mempools for RPCs
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):
> >
> > 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

> > 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.


> 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

Best regards,
Christian Schoenebeck

 \ /
  Last update: 2022-07-04 15:57    [W:0.074 / U:0.744 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site