Messages in this thread |  | | From | Christian Schoenebeck <> | Subject | Re: [PATCH 3/3] 9p: Add mempools for RPCs | Date | Mon, 04 Jul 2022 15:39:32 +0200 |
| |
On Montag, 4. Juli 2022 15:06:31 CEST Kent Overstreet wrote: > On Mon, Jul 04, 2022 at 01:12:51PM +0200, Christian Schoenebeck wrote: > > On Montag, 4. Juli 2022 05:38:46 CEST Dominique Martinet wrote: > > > +Christian, sorry I just noticed you weren't in Ccs again -- > > > the patches are currently there if you want a look: > > > https://evilpiepirate.org/git/bcachefs.git/log/?h=9p_mempool > > > > I wonder whether it would make sense to update 9p section in MAINTAINERS > > to > > better reflect current reality, at least in a way such that contributors > > would CC me right away? > > > > Eric, Latchesar, what do you think? > > > > > > @@ -270,10 +276,8 @@ p9_tag_alloc(struct p9_client *c, int8_t type, > > > > unsigned int max_size)> > > > > > > > > if (!req) > > > > > > > > return ERR_PTR(-ENOMEM); > > > > > > > > - if (p9_fcall_init(c, &req->tc, alloc_msize)) > > > > - goto free_req; > > > > - if (p9_fcall_init(c, &req->rc, alloc_msize)) > > > > - goto free; > > > > + p9_fcall_init(c, &req->tc, 0, alloc_msize); > > > > + p9_fcall_init(c, &req->rc, 1, alloc_msize); > > > > > > mempool allocation never fails, correct? > > > > > > (don't think this needs a comment, just making sure here) > > > > > > This all looks good to me, will queue it up in my -next branch after > > > running some tests next weekend and hopefully submit when 5.20 opens > > > with the code making smaller allocs more common. > > > > 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. > > Err, why? > > These patches are pretty simple, and they fix a bug that's affecting users > right now (and has been for ages)
So simple that it already had one obvious bug (at least). But as it seems that Dominique already supports your patch, I refrain from enumerating more reasons.
> > 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. > > Yeah that sounds like you're just reimplementing kmalloc.
Quite exaggerated statement.
Best regards, Christian Schoenebeck
|  |