| Message ID | 20230906154611.31762-4-jsatterfield.linux@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Paul Moore |
| Headers | show |
| Series | avtab hotspot optimizations | expand |
On Wed, Sep 6, 2023 at 11:47 AM Jacob Satterfield <jsatterfield.linux@gmail.com> wrote: > > avtab_read_item() is a hot function called when reading each rule in a > binary policydb. With the current Fedora policy and refpolicy, this > function is called nearly 100,000 times per policy load. > > A single avtab node is only permitted to have a single specifier to > describe the data it holds. As such, a check is performed to make sure > only one specifier is set. Previously this was done via a for-loop. > However, there is already an optimal function for finding the number of > bits set (hamming weight) and on some architectures, dedicated > instructions (popcount) which can be executed much more efficiently. > > Even when using -mcpu=generic on a x86-64 Fedora 38 VM, this commit > results in a modest 2-4% speedup for policy loading due to a substantial > reduction in the number of instructions executed. > > Signed-off-by: Jacob Satterfield <jsatterfield.linux@gmail.com> Reviewed-by: Stephen Smalley <stephen.smalley.work@gmail.com> > --- > security/selinux/ss/avtab.c | 7 ++----- > 1 file changed, 2 insertions(+), 5 deletions(-)
On Sep 6, 2023 Jacob Satterfield <jsatterfield.linux@gmail.com> wrote: > > avtab_read_item() is a hot function called when reading each rule in a > binary policydb. With the current Fedora policy and refpolicy, this > function is called nearly 100,000 times per policy load. > > A single avtab node is only permitted to have a single specifier to > describe the data it holds. As such, a check is performed to make sure > only one specifier is set. Previously this was done via a for-loop. > However, there is already an optimal function for finding the number of > bits set (hamming weight) and on some architectures, dedicated > instructions (popcount) which can be executed much more efficiently. > > Even when using -mcpu=generic on a x86-64 Fedora 38 VM, this commit > results in a modest 2-4% speedup for policy loading due to a substantial > reduction in the number of instructions executed. > > Signed-off-by: Jacob Satterfield <jsatterfield.linux@gmail.com> > Reviewed-by: Stephen Smalley <stephen.smalley.work@gmail.com> > --- > security/selinux/ss/avtab.c | 7 ++----- > 1 file changed, 2 insertions(+), 5 deletions(-) Merged into selinux/next, thanks. -- paul-moore.com
diff --git a/security/selinux/ss/avtab.c b/security/selinux/ss/avtab.c index b7a11f417f0a..b0e455fb395c 100644 --- a/security/selinux/ss/avtab.c +++ b/security/selinux/ss/avtab.c @@ -17,6 +17,7 @@ * Tuned number of hash slots for avtab to reduce memory usage */ +#include <linux/bitops.h> #include <linux/kernel.h> #include <linux/slab.h> #include <linux/errno.h> @@ -516,11 +517,7 @@ int avtab_read_item(struct avtab *a, void *fp, struct policydb *pol, return -EINVAL; } - set = 0; - for (i = 0; i < ARRAY_SIZE(spec_order); i++) { - if (key.specified & spec_order[i]) - set++; - } + set = hweight16(key.specified & (AVTAB_XPERMS | AVTAB_TYPE | AVTAB_AV)); if (!set || set > 1) { pr_err("SELinux: avtab: more than one specifier\n"); return -EINVAL;
avtab_read_item() is a hot function called when reading each rule in a binary policydb. With the current Fedora policy and refpolicy, this function is called nearly 100,000 times per policy load. A single avtab node is only permitted to have a single specifier to describe the data it holds. As such, a check is performed to make sure only one specifier is set. Previously this was done via a for-loop. However, there is already an optimal function for finding the number of bits set (hamming weight) and on some architectures, dedicated instructions (popcount) which can be executed much more efficiently. Even when using -mcpu=generic on a x86-64 Fedora 38 VM, this commit results in a modest 2-4% speedup for policy loading due to a substantial reduction in the number of instructions executed. Signed-off-by: Jacob Satterfield <jsatterfield.linux@gmail.com> --- security/selinux/ss/avtab.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-)