On Tue, Aug 13, 2024 at 7:49 AM Suren Baghdasaryan [off-list ref] wrote:

On Mon, Aug 12, 2024 at 11:07 PM Mateusz Guzik [off-list ref] wrote:

quoted

On Mon, Aug 12, 2024 at 09:29:16PM -0700, Andrii Nakryiko wrote:

quoted

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!

quoted

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.

quoted

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.