| Message ID | 20240827-work-kmem_cache-rcu-v2-0-7bc9c90d5eef@kernel.org (mailing list archive) |
|---|---|
| Headers | show |
| Series | fs,mm: add kmem_cache_create_rcu() | expand |
On Tue, Aug 27, 2024 at 05:59:41PM GMT, Christian Brauner wrote: > When a kmem cache is created with SLAB_TYPESAFE_BY_RCU the free pointer > must be located outside of the object because we don't know what part of > the memory can safely be overwritten as it may be needed to prevent > object recycling. > > That has the consequence that SLAB_TYPESAFE_BY_RCU may end up adding a > new cacheline. This is the case for .e.g, struct file. After having it > shrunk down by 40 bytes and having it fit in three cachelines we still > have SLAB_TYPESAFE_BY_RCU adding a fourth cacheline because it needs to > accomodate the free pointer and is hardware cacheline aligned. > > I tried to find ways to rectify this as struct file is pretty much > everywhere and having it use less memory is a good thing. So here's a > proposal. > > I was hoping to get something to this effect into v6.12. > > If we really want to switch to a struct to pass kmem_cache parameters I > can do the preparatory patch to convert all kmem_cache_create() and > kmem_cache_create_usercopy() callers to use a struct for initialization > of course. I can do this as a preparatory work or as follow-up work to > this series. Thoughts? So one thing I can do is to add: struct kmem_cache_args { .freeptr_offset, .useroffset, .flags, .name, }; accompanied by: int kmem_create_cache(struct kmem_cache_args *args); and then switch both the filp cache and Jens' io_kiocb cache over to use these two helpers. Then we can convert other callers one by one. @Vlastimil, @Jens, @Linus what do you think?
On 8/27/24 18:05, Christian Brauner wrote: > On Tue, Aug 27, 2024 at 05:59:41PM GMT, Christian Brauner wrote: >> When a kmem cache is created with SLAB_TYPESAFE_BY_RCU the free pointer >> must be located outside of the object because we don't know what part of >> the memory can safely be overwritten as it may be needed to prevent >> object recycling. >> >> That has the consequence that SLAB_TYPESAFE_BY_RCU may end up adding a >> new cacheline. This is the case for .e.g, struct file. After having it >> shrunk down by 40 bytes and having it fit in three cachelines we still >> have SLAB_TYPESAFE_BY_RCU adding a fourth cacheline because it needs to >> accomodate the free pointer and is hardware cacheline aligned. >> >> I tried to find ways to rectify this as struct file is pretty much >> everywhere and having it use less memory is a good thing. So here's a >> proposal. >> >> I was hoping to get something to this effect into v6.12. >> >> If we really want to switch to a struct to pass kmem_cache parameters I >> can do the preparatory patch to convert all kmem_cache_create() and >> kmem_cache_create_usercopy() callers to use a struct for initialization >> of course. I can do this as a preparatory work or as follow-up work to >> this series. Thoughts? > > So one thing I can do is to add: > > struct kmem_cache_args { > .freeptr_offset, > .useroffset, > .flags, > .name, > }; Hm basically everyone uses name, size and some flags, so how about we leave those as direct parameters and args is for the rest, and in most cases would be NULL. > accompanied by: > > int kmem_create_cache(struct kmem_cache_args *args); I think we can't reuse the name with different parameters as long the old one exists? > and then switch both the filp cache and Jens' io_kiocb cache over to use > these two helpers. Then we can convert other callers one by one. > > @Vlastimil, @Jens, @Linus what do you think? In the other thread you said it's best to leave such refactoring to maintainers and I agree and don't ask you to do the cleanup in order to get what you need (and we don't need to rush it either).
> > @Vlastimil, @Jens, @Linus what do you think? > > In the other thread you said it's best to leave such refactoring to > maintainers and I agree and don't ask you to do the cleanup in order to get > what you need (and we don't need to rush it either). I mainly didn't want to wait for that huge refactor to be done before we get a workable solution for v6.12 since the patch isn't all that huge. But I'm not shying away from cleanup work I'm the cause of. So I'm happy to do it for v6.13 as a follow-up but I also don't want to steal this away from you because it'll probably be a fun task to get to port everything to struct kmem_cache_args and clean that all up.
On 8/28/24 14:18, Christian Brauner wrote: >> > @Vlastimil, @Jens, @Linus what do you think? >> >> In the other thread you said it's best to leave such refactoring to >> maintainers and I agree and don't ask you to do the cleanup in order to get >> what you need (and we don't need to rush it either). > > I mainly didn't want to wait for that huge refactor to be done before we > get a workable solution for v6.12 since the patch isn't all that huge. > > But I'm not shying away from cleanup work I'm the cause of. > > So I'm happy to do it for v6.13 as a follow-up but I also don't want to > steal this away from you because it'll probably be a fun task to get to > port everything to struct kmem_cache_args and clean that all up. So if you want, be my guest :) Yeah could be a fun task, but I'll probably be too busy for a while now.