From: Chen Hu <hu1.chen@intel.com>
To: miklos@szeredi.hu, amir73il@gmail.com
Cc: malini.bhandaru@intel.com, tim.c.chen@intel.com,
mikko.ylinen@intel.com, lizhen.you@intel.com,
vinicius.gomes@intel.com, hu1.chen@intel.com,
linux-unionfs@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: ovl: ovl_fs::creator_cred::usage scalability issues
Date: Wed, 18 Oct 2023 00:45:53 -0700 [thread overview]
Message-ID: <20231018074553.41333-1-hu1.chen@intel.com> (raw)
*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@intel.com>
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@intel.com>
---
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
next reply other threads:[~2023-10-18 7:47 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-10-18 7:45 Chen Hu [this message]
2023-10-18 11:59 ` ovl: ovl_fs::creator_cred::usage scalability issues Amir Goldstein
2023-12-14 22:02 ` [RFC] HACK: overlayfs: Optimize overlay/restore creds Vinicius Costa Gomes
2023-12-15 10:30 ` Amir Goldstein
2023-12-15 20:00 ` Vinicius Costa Gomes
2023-12-16 10:16 ` Amir Goldstein
2023-12-16 11:38 ` Amir Goldstein
2023-12-18 16:30 ` Christian Brauner
2023-12-18 21:57 ` Vinicius Costa Gomes
2023-12-19 7:15 ` Amir Goldstein
2023-12-19 13:35 ` Christian Brauner
2023-12-19 14:33 ` Vinicius Costa Gomes
2024-01-23 15:39 ` Christian Brauner
2024-01-23 16:37 ` Vinicius Costa Gomes
2023-12-16 18:26 ` Linus Torvalds
2023-12-18 15:17 ` Christian Brauner
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20231018074553.41333-1-hu1.chen@intel.com \
--to=hu1.chen@intel.com \
--cc=amir73il@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-unionfs@vger.kernel.org \
--cc=lizhen.you@intel.com \
--cc=malini.bhandaru@intel.com \
--cc=mikko.ylinen@intel.com \
--cc=miklos@szeredi.hu \
--cc=tim.c.chen@intel.com \
--cc=vinicius.gomes@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).