On Wed, Jan 6, 2021 at 8:26 AM Ondrej Mosnacek <omosnace@xxxxxxxxxx> wrote: > > This is motivated by a perfomance regression of selinux_xfrm_enabled() > that happened on a RHEL kernel due to false sharing between > selinux_xfrm_refcount and (the late) selinux_ss.policy_rwlock (i.e. the > .bss section memory layout changed such that they happened to share the > same cacheline). Since the policy rwlock's memory region was modified > upon each read-side critical section, the readers of > selinux_xfrm_refcount had frequent cache misses, eventually leading to a > significant performance degradation under a TCP SYN flood on a system > with many cores (32 in this case, but it's detectable on less cores as > well). > > While upstream has since switched to RCU locking, so the same can no > longer happen here, selinux_xfrm_refcount could still share a cacheline > with another frequently written region, thus marking it __read_mostly > still makes sense. __read_mostly helps, because it will put the symbol > in a separate section along with other read-mostly variables, so there > should never be a clash with frequently written data. > > Since selinux_xfrm_refcount is modified only in case of an explicit > action, it should be safe to do this (i.e. it shouldn't disrupt other > read-mostly variables too much). > > Signed-off-by: Ondrej Mosnacek <omosnace@xxxxxxxxxx> > --- > security/selinux/xfrm.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Even on a system that is using labeled IPsec, which is likely very rare, the refcount should really only be changing when the SPD or SAD changes which should be measured in minutes or seconds on a loaded system. If the __read_mostly tag proves to be a problem we can always look into other solutions. Merged into selinux/next, thanks. -- paul moore www.paul-moore.com
