*Problem* ovl_permission() checks the underlying inode with the credential of mounter. The cred, struct ovl_fs::creator_cred, is somewhat global per overlayfs superblock. Performance degrades when concurrency increases on the cred, to be specific, on ovl_fs::creator_cred::usage. This happens when there are massive file access inside container, especially on SoC with many cores. With Linux 6.6.0-rc2, we run a web workload container on Intel 4th Xeon Sapphire Rapids which has 56 cores. Perf reports that 5.7% (2.50% + 1.87% + 1.33%) CPU stall in overlayfs: Self Command Shared Object Symbol 2.50% foo [kernel.vmlinux] [k] override_creds 1.87% foo [kernel.vmlinux] [k] revert_creds 1.33% foo [kernel.vmlinux] [k] generic_permission On Soc with more than 100 cores, we can even observe ~30% CPU stalled! This scalability issue is caused by two factors: 1) Contention on creator_cred::usage creator_cred::usage is atomic_t and is inc/dec atomically during every file access. So HW acquires the corresponding cache line exclusively. This operataiton is expensive and gets worse when contention is heavy. Call chain: ovl_permission() -> ovl_override_creds() -> override_creds() -> get_new_cred() -> atomic_inc(&cred->usage); ovl_permission() -> revert_creds() -> put_cred() -> atomic_dec_and_test(&(cred)->usage)) 2) False sharing `perf c2c` shows false sharing issue between cred::usage and cred::fsuid. This is why generic_permission() stalls 1.33% CPU in above perf report. ovl_permission() updates cred::usage and it also reads cred::fsuid. Unfortunately, they locate in the same cache line and thus false sharing occurs. cred::fsuid is read at: ovl_permission() -> inode_permission() -> generic_permission() -> acl_permission_check() -> current_fsuid() *Mitigations we tried* We tried several mitigations but are not sure if it can be a fix or just workaround / hack. So we report this and want to have some discussions. Our mitigations aims to eliminate the contention on creator_cred->usage. Without contention, the false sharing will be tiny and no need to handle. The mitigations we tested are: 1) Check underlying inode once in its lifetime. OR 2) In ovl_permission(), copy global creator_cred to a local variable to avoid concurrency. With any mitigations above, CPU will not stall on overlayfs. Paste mitigation 1 below. >From 472bd18eaabcde0d41e450f556691151b1bdb64e Mon Sep 17 00:00:00 2001 From: Chen Hu <hu1.chen@xxxxxxxxx> Date: Fri, 1 Sep 2023 15:03:28 +0800 Subject: [RFC PATCH] ovl: check underlying upper inode once in its lifetime ovl_permission() checks the underlying inode with the credential of mounter. The cred, struct ovl_fs::creator_cred, is global per overlayfs superblock. Performance degrades when concurrency increases on the cred, to be specific, on ovl_fs::creator_cred::usage. This patch (or hack to some extent) checks underlying upper inode once in its lifetime, eliminates the cache line contention on creator_cred::usage and gets 40%+ perf improvement on a 128 cores CPU. CAUTION: this may compromise the file permission check. Need to talk with overlayfs experts. Signed-off-by: Chen Hu <hu1.chen@xxxxxxxxx> --- fs/overlayfs/inode.c | 5 ++++- fs/overlayfs/overlayfs.h | 1 + 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c index 83ef66644c21..62ec99316c7a 100644 --- a/fs/overlayfs/inode.c +++ b/fs/overlayfs/inode.c @@ -307,7 +307,7 @@ int ovl_permission(struct mnt_idmap *idmap, * with creds of mounter */ err = generic_permission(&nop_mnt_idmap, inode, mask); - if (err) + if (err || ovl_test_flag(OVL_FASTPERM, inode)) return err; old_cred = ovl_override_creds(inode->i_sb); @@ -318,6 +318,9 @@ int ovl_permission(struct mnt_idmap *idmap, mask |= MAY_READ; } err = inode_permission(mnt_idmap(realpath.mnt), realinode, mask); + if (err == 0 && upperinode) + /* This gets set once for the upper inode lifetime */ + ovl_set_flag(OVL_FASTPERM, inode); revert_creds(old_cred); return err; diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h index 9817b2dcb132..5b71aaa8f77c 100644 --- a/fs/overlayfs/overlayfs.h +++ b/fs/overlayfs/overlayfs.h @@ -53,6 +53,7 @@ enum ovl_inode_flag { OVL_CONST_INO, OVL_HAS_DIGEST, OVL_VERIFIED_DIGEST, + OVL_FASTPERM, }; enum ovl_entry_flag { -- 2.34.1