On Fri, Feb 28, 2020 at 3:48 AM Ondrej Mosnacek <omosnace@xxxxxxxxxx> wrote: > On Fri, Feb 28, 2020 at 1:27 AM Paul Moore <paul@xxxxxxxxxxxxxx> wrote: > > On Wed, Feb 26, 2020 at 10:55 AM Ondrej Mosnacek <omosnace@xxxxxxxxxx> wrote: > > > > > > Instead allocate hash tables with just the right size based on the > > > actual number of elements (which is almost always known beforehand, we > > > just need to defer the hashtab allocation to the right time). The only > > > case when we don't know the size (with the current policy format) is the > > > new filename transitions hashtable. Here I just left the existing value. > > > > > > After this patch, the time to load Fedora policy on x86_64 decreases > > > from 790 ms to 167 ms. If the unconfined module is removed, it decreases > > > from 750 ms to 122 ms. It is also likely that other operations are going > > > to be faster, mainly string_to_context_struct() or mls_compute_sid(), > > > but I didn't try to quantify that. > > > > > > The memory usage of all hash table arrays increases from ~58 KB to > > > ~163 KB (with Fedora policy on x86_64). > > > > > > Signed-off-by: Ondrej Mosnacek <omosnace@xxxxxxxxxx> > > > --- > > > > > > Changed in v3: > > > - switch to simpler and more logical hash size heuristic > > > - add comment explaining the choice of the heuristic > > > > > > Changed in v2: > > > - guard against h->size == 0 in hashtab_search() and hashtab_insert() > > > > > > security/selinux/ss/hashtab.c | 28 +++++++++++++++--- > > > security/selinux/ss/hashtab.h | 2 +- > > > security/selinux/ss/policydb.c | 53 +++++++++++++--------------------- > > > security/selinux/ss/policydb.h | 2 -- > > > 4 files changed, 45 insertions(+), 40 deletions(-) > > > > Thanks Ondrej, this looks better; merged into selinux/next. > > > > Also, changing the hash heuristic in v3 really shrunk the memory usage > > compared to v2 without much impact on performance - a ~100k increase > > in memory is a small price to pay for the policy load improvement. > > Well done. > > Er... sorry, I forgot to document it clearly in the e-mails, but the > usage didn't actually drop between the last two versions (it actually > increased by ~36 KB). It's just that In the previous version I > "measured" the memory usage just by looking at the total memory usage > reported by top, which is however fluctuating a lot and I was > apparently just measuring noise... For this patch I actually printk'd > the sizes of the tables exactly (since that's the only thing this > patch touches) and this showed these much smaller numbers. So please > disregard the 1-2 MB from the previous patch versions - they were > bogus. Okay, that makes a lot more sense now. I was a little shocked about the difference in memory usage (it honestly didn't make sense), but numbers don't typically lie so I figured I was just missing some small point which was having a big impact :) Regardless, a ~36KB bump in memory usage over the non-patched kernel is still a small price to pay for the performance increase. -- paul moore www.paul-moore.com
