On Thu, Sep 4, 2025 at 1:59 PM Ricardo Robaina <[email protected]> wrote: > > Whenever there's audit context, __audit_inode_child() gets called > numerous times, which can lead to high latency in scenarios that > create too many sysfs/debugfs entries at once, for instance, upon > device_add_disk() invocation. > > # uname -r > 6.17.0-rc3+ > > # auditctl -a always,exit -F path=/tmp -k foo > # time insmod loop max_loop=1000 > real 0m42.753s > user 0m0.000s > sys 0m42.494s > > # perf record -a insmod loop max_loop=1000 > # perf report --stdio |grep __audit_inode_child > 37.95% insmod [kernel.kallsyms] [k] __audit_inode_child > > __audit_inode_child() searches for both the parent and the child > in two different loops that iterate over the same list. This > process can be optimized by merging these into a single loop, > without changing the function behavior or affecting the code's > readability. > > This patch merges the two loops that walk through the list > context->names_list into a single loop. This optimization resulted > in around 54% performance enhancement for the benchmark. > > # uname -r > 6.17.0-rc3+-enhanced > > # auditctl -a always,exit -F path=/tmp -k foo > # time insmod loop max_loop=1000 > real 0m19.388s > user 0m0.000s > sys 0m19.149s > > Signed-off-by: Ricardo Robaina <[email protected]> > --- > kernel/auditsc.c | 40 ++++++++++++++++++---------------------- > 1 file changed, 18 insertions(+), 22 deletions(-) > > diff --git a/kernel/auditsc.c b/kernel/auditsc.c > index eb98cd6fe91f..7abfb68687fb 100644 > --- a/kernel/auditsc.c > +++ b/kernel/auditsc.c > @@ -2437,44 +2437,40 @@ void __audit_inode_child(struct inode *parent, > if (inode) > handle_one(inode); > > - /* look for a parent entry first */ > list_for_each_entry(n, &context->names_list, list) { > - if (!n->name || > - (n->type != AUDIT_TYPE_PARENT && > - n->type != AUDIT_TYPE_UNKNOWN)) > + /* can only match entries that have a name */ > + if (!n->name) > continue; > > - if (n->ino == parent->i_ino && n->dev == parent->i_sb->s_dev > && > - !audit_compare_dname_path(dname, > - n->name->name, n->name_len)) { > + /* look for a parent entry first */ > + if (!found_parent && > + (n->type == AUDIT_TYPE_PARENT || n->type == > AUDIT_TYPE_UNKNOWN) && > + (n->ino == parent->i_ino && n->dev == parent->i_sb->s_dev > && > + !audit_compare_dname_path(dname, n->name->name, > n->name_len))) { > if (n->type == AUDIT_TYPE_UNKNOWN) > n->type = AUDIT_TYPE_PARENT; > found_parent = n; > - break; > } > - } > > - cond_resched(); > - > - /* is there a matching child entry? */ > - list_for_each_entry(n, &context->names_list, list) { > - /* can only match entries that have a name */ > - if (!n->name || > - (n->type != type && n->type != AUDIT_TYPE_UNKNOWN)) > - continue; > - > - if (!strcmp(dname->name, n->name->name) || > - !audit_compare_dname_path(dname, n->name->name, > + /* is there a matching child entry? */ > + if (!found_child && > + (n->type == type || n->type == AUDIT_TYPE_UNKNOWN) && > + (!strcmp(dname->name, n->name->name) || > + !audit_compare_dname_path(dname, n->name->name, > found_parent ? > found_parent->name_len : > - AUDIT_NAME_FULL)) { > + AUDIT_NAME_FULL))) { > if (n->type == AUDIT_TYPE_UNKNOWN) > n->type = type; > found_child = n; > - break; > } > + > + if (found_parent && found_child) > + break; > } > > + cond_resched(); > + > if (!found_parent) { > /* create a new, "anonymous" parent record */ > n = audit_alloc_name(context, AUDIT_TYPE_PARENT); > -- > 2.51.0 > Hi Paul,
I’m curious if you have any thoughts on this one. Please disregard this email if it’s already in your to-do list. It’s not my intention to rush you in any way.
