| Message ID | 20240813042917.506057-13-andrii@kernel.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | uprobes: RCU-protected hot path optimizations | expand |
On Mon, Aug 12, 2024 at 09:29:16PM -0700, Andrii Nakryiko wrote: > Add RCU protection for file struct's backing memory by adding > SLAB_TYPESAFE_BY_RCU flag to files_cachep. This will allow to locklessly > access struct file's fields under RCU lock protection without having to > take much more expensive and contended locks. > > This is going to be used for lockless uprobe look up in the next patch. > > Suggested-by: Matthew Wilcox <willy@infradead.org> > Signed-off-by: Andrii Nakryiko <andrii@kernel.org> > --- > kernel/fork.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/kernel/fork.c b/kernel/fork.c > index 76ebafb956a6..91ecc32a491c 100644 > --- a/kernel/fork.c > +++ b/kernel/fork.c > @@ -3157,8 +3157,8 @@ void __init proc_caches_init(void) > NULL); > files_cachep = kmem_cache_create("files_cache", > sizeof(struct files_struct), 0, > - SLAB_HWCACHE_ALIGN|SLAB_PANIC|SLAB_ACCOUNT, > - NULL); > + SLAB_HWCACHE_ALIGN|SLAB_PANIC|SLAB_TYPESAFE_BY_RCU| > + SLAB_ACCOUNT, NULL); > fs_cachep = kmem_cache_create("fs_cache", > sizeof(struct fs_struct), 0, > SLAB_HWCACHE_ALIGN|SLAB_PANIC|SLAB_ACCOUNT, Did you mean to add it to the cache backing 'struct file' allocations? That cache is created in fs/file_table.c and already has the flag: filp_cachep = kmem_cache_create("filp", sizeof(struct file), 0, SLAB_TYPESAFE_BY_RCU | SLAB_HWCACHE_ALIGN | SLAB_PANIC | SLAB_ACCOUNT, NULL); The cache you are modifying in this patch contains the fd array et al and is of no consequence to "uprobes: add speculative lockless VMA to inode resolution". iow this patch needs to be dropped
On Mon, Aug 12, 2024 at 11:07 PM Mateusz Guzik <mjguzik@gmail.com> wrote: > > On Mon, Aug 12, 2024 at 09:29:16PM -0700, Andrii Nakryiko wrote: > > Add RCU protection for file struct's backing memory by adding > > SLAB_TYPESAFE_BY_RCU flag to files_cachep. This will allow to locklessly > > access struct file's fields under RCU lock protection without having to > > take much more expensive and contended locks. > > > > This is going to be used for lockless uprobe look up in the next patch. > > > > Suggested-by: Matthew Wilcox <willy@infradead.org> > > Signed-off-by: Andrii Nakryiko <andrii@kernel.org> > > --- > > kernel/fork.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/kernel/fork.c b/kernel/fork.c > > index 76ebafb956a6..91ecc32a491c 100644 > > --- a/kernel/fork.c > > +++ b/kernel/fork.c > > @@ -3157,8 +3157,8 @@ void __init proc_caches_init(void) > > NULL); > > files_cachep = kmem_cache_create("files_cache", > > sizeof(struct files_struct), 0, > > - SLAB_HWCACHE_ALIGN|SLAB_PANIC|SLAB_ACCOUNT, > > - NULL); > > + SLAB_HWCACHE_ALIGN|SLAB_PANIC|SLAB_TYPESAFE_BY_RCU| > > + SLAB_ACCOUNT, NULL); > > fs_cachep = kmem_cache_create("fs_cache", > > sizeof(struct fs_struct), 0, > > SLAB_HWCACHE_ALIGN|SLAB_PANIC|SLAB_ACCOUNT, > > Did you mean to add it to the cache backing 'struct file' allocations? > > That cache is created in fs/file_table.c and already has the flag: > filp_cachep = kmem_cache_create("filp", sizeof(struct file), 0, > SLAB_TYPESAFE_BY_RCU | SLAB_HWCACHE_ALIGN | > SLAB_PANIC | SLAB_ACCOUNT, NULL); Oh, I completely missed the SLAB_TYPESAFE_BY_RCU for this cache, and here I was telling Andrii that it's RCU unsafe to access vma->vm_file... Mea culpa. > > The cache you are modifying in this patch contains the fd array et al > and is of no consequence to "uprobes: add speculative lockless VMA to > inode resolution". > > iow this patch needs to be dropped I believe you are correct.
On Tue, Aug 13, 2024 at 7:49 AM Suren Baghdasaryan <surenb@google.com> wrote: > > On Mon, Aug 12, 2024 at 11:07 PM Mateusz Guzik <mjguzik@gmail.com> wrote: > > > > On Mon, Aug 12, 2024 at 09:29:16PM -0700, Andrii Nakryiko wrote: > > > Add RCU protection for file struct's backing memory by adding > > > SLAB_TYPESAFE_BY_RCU flag to files_cachep. This will allow to locklessly > > > access struct file's fields under RCU lock protection without having to > > > take much more expensive and contended locks. > > > > > > This is going to be used for lockless uprobe look up in the next patch. > > > > > > Suggested-by: Matthew Wilcox <willy@infradead.org> > > > Signed-off-by: Andrii Nakryiko <andrii@kernel.org> > > > --- > > > kernel/fork.c | 4 ++-- > > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > > > diff --git a/kernel/fork.c b/kernel/fork.c > > > index 76ebafb956a6..91ecc32a491c 100644 > > > --- a/kernel/fork.c > > > +++ b/kernel/fork.c > > > @@ -3157,8 +3157,8 @@ void __init proc_caches_init(void) > > > NULL); > > > files_cachep = kmem_cache_create("files_cache", > > > sizeof(struct files_struct), 0, > > > - SLAB_HWCACHE_ALIGN|SLAB_PANIC|SLAB_ACCOUNT, > > > - NULL); > > > + SLAB_HWCACHE_ALIGN|SLAB_PANIC|SLAB_TYPESAFE_BY_RCU| > > > + SLAB_ACCOUNT, NULL); > > > fs_cachep = kmem_cache_create("fs_cache", > > > sizeof(struct fs_struct), 0, > > > SLAB_HWCACHE_ALIGN|SLAB_PANIC|SLAB_ACCOUNT, > > > > Did you mean to add it to the cache backing 'struct file' allocations? Yep, thanks for catching this! > > > > That cache is created in fs/file_table.c and already has the flag: > > filp_cachep = kmem_cache_create("filp", sizeof(struct file), 0, > > SLAB_TYPESAFE_BY_RCU | SLAB_HWCACHE_ALIGN | > > SLAB_PANIC | SLAB_ACCOUNT, NULL); > > Oh, I completely missed the SLAB_TYPESAFE_BY_RCU for this cache, and > here I was telling Andrii that it's RCU unsafe to access > vma->vm_file... Mea culpa. > Well, my bad for not double-checking and going just by the name. filp_cachep vs files_cachep is easy to mix up. > > > > The cache you are modifying in this patch contains the fd array et al > > and is of no consequence to "uprobes: add speculative lockless VMA to > > inode resolution". > > > > iow this patch needs to be dropped > > I believe you are correct. > I'm happy that we already have SLAB_TYPESAFE_BY_RCU on filp_cachep, I'll just drop this patch.
diff --git a/kernel/fork.c b/kernel/fork.c index 76ebafb956a6..91ecc32a491c 100644 --- a/kernel/fork.c +++ b/kernel/fork.c @@ -3157,8 +3157,8 @@ void __init proc_caches_init(void) NULL); files_cachep = kmem_cache_create("files_cache", sizeof(struct files_struct), 0, - SLAB_HWCACHE_ALIGN|SLAB_PANIC|SLAB_ACCOUNT, - NULL); + SLAB_HWCACHE_ALIGN|SLAB_PANIC|SLAB_TYPESAFE_BY_RCU| + SLAB_ACCOUNT, NULL); fs_cachep = kmem_cache_create("fs_cache", sizeof(struct fs_struct), 0, SLAB_HWCACHE_ALIGN|SLAB_PANIC|SLAB_ACCOUNT,
Add RCU protection for file struct's backing memory by adding SLAB_TYPESAFE_BY_RCU flag to files_cachep. This will allow to locklessly access struct file's fields under RCU lock protection without having to take much more expensive and contended locks. This is going to be used for lockless uprobe look up in the next patch. Suggested-by: Matthew Wilcox <willy@infradead.org> Signed-off-by: Andrii Nakryiko <andrii@kernel.org> --- kernel/fork.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)